Config follow-up: Implement Server-Side Apply#472
Open
andreaskaris wants to merge 1 commit intobpfman:mainfrom
Open
Config follow-up: Implement Server-Side Apply#472andreaskaris wants to merge 1 commit intobpfman:mainfrom
andreaskaris wants to merge 1 commit intobpfman:mainfrom
Conversation
c97aab8 to
fa02f77
Compare
frobware
pushed a commit
to frobware/bpfman-operator
that referenced
this pull request
Sep 5, 2025
…s/component-update-ocp-bpfman-agent chore(deps): update ocp-bpfman-agent to cf30ca8
ba3e236 to
4b5edb8
Compare
Replace the Get/Create and Get/Update reconciliation pattern with Server-Side Apply (SSA) using client.Apply patches. This simplifies resource management by handling both creation and updates in a single operation while providing better conflict resolution and field ownership tracking. Changes: - Add TypeMeta fields to all Kubernetes objects for proper SSA support - Change assureResource logic to use r.Patch with client.Apply - Add test interceptor to handle SSA patches in fake client Signed-off-by: Andreas Karis <ak.karis@gmail.com>
4b5edb8 to
c97f5ac
Compare
Contributor
|
I notice in controller-runtime v0.22 that:
Given the “native support”, should we look at the implementation again, particularly if the fake client has better support for unit testing? And on the subject of testing, would it be better to consider envtest in addition to unit tests? How practical is it to test the server-side behaviour with unit testing? (Unlikely.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Server-Side Apply
My investigation into Server-Side Apply:
https://andreaskaris.github.io/blog/coding/server-side-apply/
Server-Side Apply simplifies controller logic by using a single approach for both creating and updating resources. Instead of checking if an object exists and then choosing between Create or Update operations, controllers can use the Patch method with client.Apply for all cases.
This approach works particularly well for reconstructive controllers that want to declare the desired state of resources. The controller builds the complete object definition and lets the Kubernetes API server handle the differences. Field management ensures that conflicts are detected and ownership is tracked properly.
https://kubernetes.io/blog/2022/10/20/advanced-server-side-apply/#reconstructive-controllers: Reconstructive controllers: "This kind of controller wasn't really possible prior to SSA. The idea here is to (whenever something changes etc) reconstruct from scratch the fields of the object as the controller wishes them to be, and then apply the change to the server, letting it figure out the result. I now recommend that new controllers start out this way–it's less fiddly to say what you want an object to look like than it is to say how you want it to change. (...) To get around this downside, why not GET the object and only send your apply if the object needs it? Surprisingly, it doesn't help much – a no-op apply is not very much more work for the API server than an extra GET; and an apply that changes things is cheaper than that same apply with a preceding GET. Worse, since it is a distributed system, something could change between your GET and apply, invalidating your computation. Instead, you can use this optimization on an object retrieved from a cache–then it legitimately will reduce load on the system (at the cost of a delay when a change is needed and the cache is a bit behind)"
Get requests are cached by the controller-runtime, so we still benefit from the caching mechanism by running
r.Getbefore and by comparingexistingtodesired. Ifexistingis not found in the cache, or if the cached version ofexisting!=desired, we build an SSA patch and send that to the server.We continue comparing to and sending our full intent, meaning the full object, via SSA. Partial updates to resources with Server-Side Apply make little sense in my opinion, as we want to declare the full state of each object and we want to catch any deviations.
Status of Server-Side Apply native support in controller-runtime
Native support of Server-Side Apply is still a work in progress, although it seems that they are nearly done.
Up to controller-runtime 0.21.0,
r.Apply()was not available, instead, the recommendation is to user.Patch:https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.21.0/pkg/client#Client
Therefore, users of controller-runtime would use constructs like:
A problem with this are the unit tests which do not support Server-Side Apply, and thus an interceptor for Patch requests is needed:
r.Apply()was introduced with controller-runtime v0.22.0 which has not been included in the operator-sdk at time of this writing:r.Apply()is available (and the mock client should work with that, as well)compare https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.21.0/pkg/client#Writer -> https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.22.0/pkg/client#Writer
Improved reconciliation logic
I decided to maintain reconciliation logic as is, meaning whenever any object is changed, created, deleted we run through the entire reconciliation logic. As we are using the controller-runtime's cache (which in turn uses k8s informers), load on the API server will be fairly minimal and will be limited to the actual
r.Patchrequests, only (gets come from cache and we only patch when we need something new or when something deviates).Each
r.Patchagainst any of the owned resources will always lead to another reconciliation run which checks all owned resources.Especially on initialization with none of the resources existing (which is the worst case scenario), this leads to several reconciliation runs which might not seem as if they were needed.
I believe the above is the correct approach: Not a whole lot of documentation is available about this, but the Java Operator SDK is pretty adamant about the fact that reconciliation should always reconcile all resources:
https://javaoperatorsdk.io/docs/getting-started/patterns-best-practices/
In the same line of thought, the controller-runtime documentation clearly states that controllers should not distinguish between create/update/delete events: https://github.com/kubernetes-sigs/controller-runtime/blob/main/FAQ.md#q-how-do-i-have-different-logic-in-my-reconciler-for-different-types-of-events-eg-create-update-delete
About owned secondary resources: