-
Notifications
You must be signed in to change notification settings - Fork 636
Move server-side apply function to a shared package #4297
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
Conversation
Kubernetes 1.22 and OpenShift 4.8 have been out of commission for some time now.
| if !equality.Semantic.DeepEqual(actual.Selector, intent.Selector) { | ||
| patch.Replace(append(path, "selector")...)(intent.Selector) | ||
| } | ||
| return runtime.Apply(ctx, r.Writer, object) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you thinking about a separate PR to remove this file and change the many applys in internal/controller/postgrescluster?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't planning on it, but that seems reasonable. I skipped it here because it is called a lot.
| // | ||
| // The "metadata.finalizers" field is also okay. | ||
| // - https://book.kubebuilder.io/reference/using-finalizers.html | ||
| // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Highly recommend turning off whitespace differences to see what's actually going on here for anyone reviewing this
|
|
||
| var suite struct { | ||
| Client client.Client | ||
| Config *rest.Config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, the only use of ServerVersion was in the test in internal/controller/postgrescluster/controller_test.go where we were checking for older versions of k8s than we care about, ah
Multiple controllers had a method for this, but the implementations differed slightly. This combines their fixes and tests them in a single place.
bebf797 to
02a345e
Compare
Multiple controllers had a method for this, but the implementations differed slightly. This combines their fixes and tests them in a single place.
Checklist:
Type of Changes: