⭐️ use Server-Side Apply (SSA) for resource reconciliation#1381
Merged
chris-rock merged 2 commits intomainfrom Feb 13, 2026
Merged
⭐️ use Server-Side Apply (SSA) for resource reconciliation#1381chris-rock merged 2 commits intomainfrom
chris-rock merged 2 commits intomainfrom
Conversation
Contributor
Test Results 5 files ± 0 42 suites ±0 42m 43s ⏱️ + 4m 36s Results for commit 983969f. ± Comparison against base commit 784a806. This pull request removes 55 tests.♻️ This comment has been updated with latest results. |
3e9a932 to
81c5ded
Compare
imilchev
reviewed
Feb 12, 2026
Member
imilchev
left a comment
There was a problem hiding this comment.
I think we should use ctrl.CreateOrUpdate. Here's an example https://github.com/kubernetes-sigs/controller-runtime/blob/2053ba3d414e7cf9b47ae407e5c860556b3113b0/examples/multiclustersync/main.go#L165
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the custom Server-Side Apply implementation with controller-runtime's upstream CreateOrUpdate pattern, which is simpler and avoids issues with server-set defaults being zeroed out on every reconcile. Key changes: - Delete custom apply.go/apply_test.go - Restore CreateOrUpdate wrapper from main branch - Add field-level update helpers (UpdateCronJobFields, UpdateDeploymentFields, UpdateDaemonSetFields) to preserve server-set defaults like DNSPolicy, SchedulerName, TerminationGracePeriodSeconds, etc. - Replace DeleteAllOf with DeleteCompletedJobs to avoid killing running scans - Add TerminationMessagePath/TerminationMessagePolicy to all container specs to prevent unnecessary diffs against server defaults Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
81c5ded to
983969f
Compare
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
This PR replaces the previous custom Server-Side Apply (SSA) implementation with controller-runtime's upstream
CreateOrUpdatepattern.pkg/utils/k8s/create_update.gowrapper aroundctrl.CreateOrUpdatewith controller reference and loggingpkg/utils/k8s/update_fields.gowith field-level update helpers (UpdateCronJobFields,UpdateDeploymentFields,UpdateDaemonSetFields) that copy only managed fields, preserving server-set defaultsCreateOrUpdatewith field-level mutate functionsDeleteAllOfwithDeleteCompletedJobsto avoid killing running scan JobsTerminationMessagePath/TerminationMessagePolicyto all container specs to prevent unnecessary diffs against server defaultsapply.go/apply_test.go(SSA approach)Why the approach changed
The initial SSA implementation caused integration test failures because:
obj.Spec = desired.Specin mutate functions zeroed out server-set defaults (DNSPolicy, SchedulerName, TerminationGracePeriodSeconds, etc.), causingDeepEqualto find differences on every reconcileDeleteAllOfkilled ALL Jobs (including running ones) on every detected "update", preventing scan pods from ever completingThe upstream
CreateOrUpdate+ field-level updates pattern avoids both issues by only mutating fields the operator manages, letting the API server retain its defaults.Closes #687
Test plan
make test)make lint,make lint/actions)🤖 Generated with Claude Code