From 8a45247161d4d6c3ec311995a433bfb9a60b9477 Mon Sep 17 00:00:00 2001 From: Jakub Jaruszewski Date: Wed, 29 Apr 2026 16:23:06 +0200 Subject: [PATCH 1/3] Fix missing conflict retry with server-side apply Signed-off-by: Jakub Jaruszewski --- pkg/kube/client.go | 19 +++++++++++-------- pkg/kube/client_test.go | 11 ++++++++--- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index c955e8875..c5bdb8b81 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -318,20 +318,23 @@ func (c *Client) makeCreateApplyFunc(serverSideApply, forceConflicts, dryRun boo slog.String("fieldValidationDirective", string(fieldValidationDirective))) return func(target *resource.Info) error { - err := patchResourceServerSide(target, dryRun, forceConflicts, fieldValidationDirective) - logger := c.Logger().With( slog.String("namespace", target.Namespace), slog.String("name", target.Name), slog.String("gvk", target.Mapping.GroupVersionKind.String())) - if err != nil { - logger.Debug("Error creating resource via patch", slog.Any("error", err)) - return err - } - logger.Debug("Created resource via patch") + return retry.RetryOnConflict( + retry.DefaultRetry, + func() error { + err := patchResourceServerSide(target, dryRun, forceConflicts, fieldValidationDirective) + if err != nil { + logger.Debug("Error creating resource via patch", slog.Any("error", err)) + return err + } - return nil + logger.Debug("Created resource via patch") + return nil + }) } } diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index ed871c05a..f04be354a 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -53,6 +53,7 @@ import ( k8sfake "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest/fake" + "k8s.io/client-go/util/retry" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" ) @@ -273,9 +274,13 @@ func TestCreate(t *testing.T) { }, ExpectedErrorContains: "Operation cannot be fulfilled on resourcequotas \"quota\": the object has been modified; " + "please apply your changes to the latest version and try again", - ExpectedActions: []string{ - "/namespaces/default/pods/dolphin:PATCH", - }, + ExpectedActions: func() []string { // expect helm to retry on conflict, workaround for: https://github.com/kubernetes/kubernetes/issues/67761 + actions := make([]string, retry.DefaultRetry.Steps) + for i := range actions { + actions[i] = "/namespaces/default/pods/dolphin:PATCH" + } + return actions + }(), }, } From c178b0d9eda0b9f51b9f52483b4ede42ce845328 Mon Sep 17 00:00:00 2001 From: Jakub Jaruszewski Date: Mon, 15 Jun 2026 12:14:11 +0200 Subject: [PATCH 2/3] Only retry on resourcequota transient conflict Signed-off-by: Jakub Jaruszewski --- pkg/kube/client.go | 23 ++++++++++++++++++++++- pkg/kube/client_test.go | 18 ++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index c5bdb8b81..7c98a653d 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -323,8 +323,9 @@ func (c *Client) makeCreateApplyFunc(serverSideApply, forceConflicts, dryRun boo slog.String("name", target.Name), slog.String("gvk", target.Mapping.GroupVersionKind.String())) - return retry.RetryOnConflict( + return retry.OnError( retry.DefaultRetry, + isQuotaConflict, func() error { err := patchResourceServerSide(target, dryRun, forceConflicts, fieldValidationDirective) if err != nil { @@ -954,6 +955,26 @@ func isIncompatibleServerError(err error) bool { return err.(*apierrors.StatusError).Status().Code == http.StatusUnsupportedMediaType } +// isQuotaConflict checks if the error is a conflict error specifically caused by +// a ResourceQuota. This is used to determine if a retry should be attempted, +// since quota conflicts are typically transient and can be resolved by retrying. +func isQuotaConflict(err error) bool { + if !apierrors.IsConflict(err) { + return false + } + + // Check the error message for the specific ResourceQuota conflict pattern. + // The error message from the ResourceQuota admission controller contains: + // "Operation cannot be fulfilled on resourcequotas" and "the object has been modified" + errMsg := err.Error() + if strings.Contains(errMsg, "Operation cannot be fulfilled on resourcequotas") && + strings.Contains(errMsg, "the object has been modified") { + return true + } + + return false +} + // getManagedFieldsManager returns the manager string. If one was set it will be returned. // Otherwise, one is calculated based on the name of the binary. func getManagedFieldsManager() string { diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index f04be354a..6a96c915c 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -282,6 +282,24 @@ func TestCreate(t *testing.T) { return actions }(), }, + "Create fail: managed fields conflict (server-side apply)": { + Pods: newPodList("seal"), + ServerSideApply: true, + Callback: func(t *testing.T, _ testCase, _ []RequestResponseAction, req *http.Request) (*http.Response, error) { + t.Helper() + + // Return a generic 409 conflict (not quota-related) + // This simulates a managed fields conflict + return &http.Response{ + StatusCode: http.StatusConflict, + Request: req, + }, nil + }, + ExpectedErrorContains: "the server reported a conflict", + ExpectedActions: []string{ + "/namespaces/default/pods/seal:PATCH", + }, + }, } c := newTestClient(t) From 1f82bffafc3fb54eea041442f17638a5274ca83c Mon Sep 17 00:00:00 2001 From: Jakub Jaruszewski Date: Mon, 22 Jun 2026 16:11:22 +0200 Subject: [PATCH 3/3] Use isServerSideRetryable + naming nitpick Signed-off-by: Jakub Jaruszewski --- pkg/kube/client.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 7c98a653d..0748b1a2e 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -325,7 +325,7 @@ func (c *Client) makeCreateApplyFunc(serverSideApply, forceConflicts, dryRun boo return retry.OnError( retry.DefaultRetry, - isQuotaConflict, + isServerSideRetryable, func() error { err := patchResourceServerSide(target, dryRun, forceConflicts, fieldValidationDirective) if err != nil { @@ -955,10 +955,16 @@ func isIncompatibleServerError(err error) bool { return err.(*apierrors.StatusError).Status().Code == http.StatusUnsupportedMediaType } -// isQuotaConflict checks if the error is a conflict error specifically caused by +// isServerSideRetryable checks if an error encountered during server-side apply +// should be retried. Currently, only ResourceQuota conflicts are considered retryable. +func isServerSideRetryable(err error) bool { + return isResourceQuotaConflict(err) +} + +// isResourceQuotaConflict checks if the error is a conflict error specifically caused by // a ResourceQuota. This is used to determine if a retry should be attempted, // since quota conflicts are typically transient and can be resolved by retrying. -func isQuotaConflict(err error) bool { +func isResourceQuotaConflict(err error) bool { if !apierrors.IsConflict(err) { return false }