feat: allow recreating a resource when a server-side apply is rejected#642
Merged
Merged
Conversation
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Server-side apply cannot always reconcile a manifest change: if a field was
set by API server defaulting (or is otherwise owned by a different/unknown
field manager) and the new manifest introduces a value that conflicts with
it, the apply is rejected and no patch can clear it. The concrete case that
motivated this: a Deployment originally applied with no `strategy` gets
`spec.strategy.rollingUpdate` defaulted by the API server; a later manifest
that sets `strategy.type: Recreate` (without `rollingUpdate`) is then
rejected with:
```
Deployment.apps "foo" is invalid: spec.strategy.rollingUpdate: Forbidden:
may not be specified when strategy `type` is 'Recreate'
```
`UpdateResource` gains a `forceRecreate` parameter (default caller behavior
unchanged when false). When true, and the apply fails with an error that
only a delete+recreate can resolve, the object is deleted, the code waits
for it to be gone, and the apply is retried as a create. Error
error-text patterns for admission/CEL-enforced immutable fields.
Deliberately excludes `apierrors.IsConflict`. Conflicts are already retried
in place by the existing `retry.RetryOnConflict` wrapper.
This also never applies to CustomResourceDefinitions, since deleting one cascades
to every instance of it.
For pull mode, `ConfigurationBundleSpec` gains a matching `Force` field, and
`pullmode.WithResourceInfo` takes a `force` argument alongside the existing
`tier`/`skipNamespaceCreation`, so the option can ride along on the same
per-PolicyRef/per-KustomizationRef granularity as those two.
This is a library-only change with no new caller opting in yet
(`forceRecreate`/`Force` default to false everywhere). addon-controller and
sveltos-applier will consume this in follow-up PRs to expose it as a
user-facing option on `PolicyRef`/`KustomizationRef`.
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
2 participants
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 cannot always reconcile a manifest change: if a field was set by API server defaulting (or is otherwise owned by a different/unknown field manager) and the new manifest introduces a value that conflicts with it, the apply is rejected and no patch can clear it. The concrete case that motivated this: a Deployment originally applied with no
strategygetsspec.strategy.rollingUpdatedefaulted by the API server; a later manifest that setsstrategy.type: Recreate(withoutrollingUpdate) is then rejected with:UpdateResourcegains aforceRecreateparameter (default caller behavior unchanged when false). When true, and the apply fails with an error that only a delete+recreate can resolve, the object is deleted, the code waits for it to be gone, and the apply is retried as a create. Error error-text patterns for admission/CEL-enforced immutable fields.Deliberately excludes
apierrors.IsConflict. Conflicts are already retried in place by the existingretry.RetryOnConflictwrapper. This also never applies to CustomResourceDefinitions, since deleting one cascades to every instance of it.For pull mode,
ConfigurationBundleSpecgains a matchingForcefield, andpullmode.WithResourceInfotakes aforceargument alongside the existingtier/skipNamespaceCreation, so the option can ride along on the same per-PolicyRef/per-KustomizationRef granularity as those two.This is a library-only change with no new caller opting in yet (
forceRecreate/Forcedefault to false everywhere). addon-controller and sveltos-applier will consume this in follow-up PRs to expose it as a user-facing option onPolicyRef/KustomizationRef.