DNM: Reconcile follow up - Fix reconciliation loop using semantic comparison #467
Conversation
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
Transition from ConfigMap to Config CRD to enable cascading deletion via ownerReferences for namespace and cluster-scoped resources like CSIDriver (ConfigMaps are namespace-scoped and cannot be owners of cluster-scoped resources). Additional benefit of Config CRD is that configuration can be properly validated by the API server. Refactor of reconciliation logic with improved logging and error handling, unit tests. Signed-off-by: Andreas Karis <[email protected]>
Signed-off-by: Andreas Karis <[email protected]>
Signed-off-by: Andreas Karis <[email protected]>
Signed-off-by: Andreas Karis <[email protected]>
Add finalizer to Config CRD to prevent race condition during deletion where operator could recreate resources while Kubernetes garbage collection is trying to delete them via owner references. Changes: - Add BpfmanConfigFinalizer constant - Add finalizer RBAC permissions via controller-gen - Check deletion timestamp before reconciliation - Add finalizer management to prevent race conditions - Add handleDeletion for coordinated cleanup - Update tests for two-phase reconciliation - Fix undeploy targets to handle cluster-scoped Config CRD This ensures safe deletion sequence: 1. User deletes Config 2. Deletion timestamp set, finalizer blocks deletion 3. Operator stops creating/updating resources 4. Operator removes finalizer 5. Kubernetes garbage collection proceeds via owner references Signed-off-by: Andrew McDermott <[email protected]>
Signed-off-by: Andreas Karis <[email protected]>
Add a new run-local Makefile target that allows developers to run the bpfman-operator locally for development and debugging purposes. The target automatically scales down the cluster deployment to avoid conflicts and runs the operator with debug logging enabled. Signed-off-by: Andreas Karis <[email protected]>
Add conditional updates to Config reconciliation using semantic deep equality comparison to prevent unnecessary resource updates. This resolves reconciliation loops where the operator would continuously update resources even when no meaningful changes were present. Additionally, sync the DaemonSet and metrics-proxy YAML manifests with the actual deployed resources by adding missing Kubernetes default fields like terminationMessagePath, apiVersion in fieldRef, and updateStrategy settings. Signed-off-by: Andreas Karis <[email protected]>
andreaskaris
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #467 +/- ##
===========================
===========================
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Follow-up to #461 - to be rebased and merged once #461 lands
Fix reconciliation loop using semantic comparison
This PR resolves a reconciliation loop issue in the bpfman-operator where resources were being
continuously updated even when no meaningful changes were present.
Changes
Controller Logic:
assureResourcefunction to accept aneedsUpdatecallback parameterk8s.io/apimachinery/pkg/api/equalityresources
YAML Manifest Synchronization:
daemonset.yamlandmetrics-proxy-daemonset.yamlto include missing Kubernetes defaultfields
terminationMessagePath,terminationMessagePolicy,apiVersionin fieldRef,updateStrategy, etc.Problem Solved
Previously, the operator would continuously reconcile resources because:
assureResourcefunction always performed updates without checking if changes were necessarystate
Testing
The changes maintain backward compatibility while preventing unnecessary resource churn. The operator
now only updates resources when there are actual semantic differences in the specifications.