-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
go1.22 and k8s1.29 support #145
Conversation
Signed-off-by: Furkan <[email protected]>
Thanks for trying to get wave up to date with k8s!
This error I have figured out: The call to update the k8s resource is made here https://github.com/Dentrax/wave/blob/go1.22-k8s1.29/pkg/core/handler.go#L110. The problem is that the variable copy isn't of a recognized type, since it's of the type core.daemonset, which is internal to wave. It has a "real" DaemonSet as a member though. So one way of getting rid of that error is with this patch: Index: pkg/core/handler.go
===================================================================
diff --git a/pkg/core/handler.go b/pkg/core/handler.go
--- a/pkg/core/handler.go (revision 2fd44cfd8e8ceaf09d91bd9d8230c22707cc6b11)
+++ b/pkg/core/handler.go (date 1713436150304)
@@ -107,7 +107,7 @@
if !reflect.DeepEqual(instance, copy) {
log.V(0).Info("Updating instance hash", "namespace", instance.GetNamespace(), "name", instance.GetName(), "hash", hash)
h.recorder.Eventf(copy, corev1.EventTypeNormal, "ConfigChanged", "Configuration hash updated to %s", hash)
- err := h.Update(context.TODO(), copy)
+ err := h.Update(context.TODO(), copy.GetObject())
if err != nil {
return reconcile.Result{}, fmt.Errorf("error updating instance %s/%s: %v", instance.GetNamespace(), instance.GetName(), err)
}
Index: pkg/core/types.go
===================================================================
diff --git a/pkg/core/types.go b/pkg/core/types.go
--- a/pkg/core/types.go (revision 2fd44cfd8e8ceaf09d91bd9d8230c22707cc6b11)
+++ b/pkg/core/types.go (date 1713439124160)
@@ -45,6 +45,7 @@
type podController interface {
client.Object
metav1.Object
+ GetObject() client.Object
GetPodTemplate() *corev1.PodTemplateSpec
SetPodTemplate(*corev1.PodTemplateSpec)
DeepCopyPodController() podController
@@ -55,6 +56,10 @@
*appsv1.Deployment
}
+func (d *deployment) GetObject() client.Object {
+ return d.Deployment
+}
+
func (d *deployment) GetPodTemplate() *corev1.PodTemplateSpec {
return &d.Spec.Template
}
@@ -72,6 +77,10 @@
*appsv1.StatefulSet
}
+func (s *statefulset) GetObject() client.Object {
+ return s.StatefulSet
+}
+
func (s *statefulset) GetPodTemplate() *corev1.PodTemplateSpec {
return &s.Spec.Template
}
@@ -89,6 +98,10 @@
*appsv1.DaemonSet
}
+func (d *daemonset) GetObject() client.Object {
+ return d.DaemonSet
+}
+
func (d *daemonset) GetPodTemplate() *corev1.PodTemplateSpec {
return &d.Spec.Template
}
Unfortunately that doesn't make any more tests succeed... |
Cool. With that change, the e2e tests complete (see #147). The errors from |
Now the tests are running, but into timeout issues. I will pick this up tomorrow (if nobody else has any bright ideas :-D ). |
OK, this seems to be (at least) an API mismatch between kubebuilder v1 and v2; migrating 1->2 or even 2->3 seems to be best done by doing a complete re-creation of the project; I am inclined to try that. |
Thank you for your efforts here @toelke! I just checkout your branch and tried to increase timeouts to |
Yeah, looking at the logs, it appears that the controller started for the tests does not, in fact, listen for changes to |
I will create a PR against your branch later which should fix a few things:
In later PRs we should also update some other dependencies but one step after another. |
Here is the PR: Dentrax#1 |
All ready from my side. Let me know if you or @toelke want any other change. This should enable us to build a release and close a lot of issues caused by the old kubernetes client api version (topology-spread, startup-probes etc). We are very eager to test and deploy this. |
I can create another PR for the linter timeout later. Guess my local github action runner has just been much faster. |
Signed-off-by: Furkan <[email protected]>
Fixed CI issue by increasing golangci-lint timeout + used v1.57. |
Judging from the CI Run on the PR against your repo I might have messed up the indent in this line: https://github.com/Dentrax/wave/pull/1/files#diff-faff1af3d8ff408964a57b2e475f69a6b7c7b71c9978cccc8f471798caac2c88R20. Maybe my local CI runner just ignored that setting? See: https://github.com/Dentrax/wave/actions/runs/8823091364 |
Something else seems to be wrong... https://github.com/wave-k8s/wave/actions/runs/8837527207/job/24266584757?pr=145 |
Thanks for fixing the indent.
Looks like one of the tests is flaky. They historically have been flaky so this probably not new. I will take care to make it more robust nonetheless. At least all other tests passed :-). |
This should address the particular case: Dentrax#2. Unfortunately, there are a few more but I have actually seen this case quite a few times so it should go first. One step after the other. Tests also passed: https://github.com/Dentrax/wave/actions/runs/8840886549/job/24277018560?pr=2 |
Thank you! It's green again. Awaiting maintainer approval to run the CI. |
Thank you so much folks for great assistance here! And merging this PR! 🚀 |
Great work all, amazing to see the collab |
CARRY: #105 and #107
Ref: #138
This is an experimental PR to add some additional works to @mamccorm's work: #138 (comment)
go.mod
, deprecatecontroller-gen
manifests
install-kubebuilder-tools
go build cmd
go test -c ./...
make test
fails with some errors that I couldn't understand:So I'm stumped on this. Waiting your feedback on this.
/cc @starkers @toelke