Fix missing conflict retry with server-side apply (#32088)

* Fix missing conflict retry with server-side apply

Signed-off-by: Jakub Jaruszewski <jjaruszewski@man.poznan.pl>

* Only retry on resourcequota transient conflict

Signed-off-by: Jakub Jaruszewski <jjaruszewski@man.poznan.pl>

* Use isServerSideRetryable + naming nitpick

Signed-off-by: Jakub Jaruszewski <jjaruszewski@man.poznan.pl>

---------

Signed-off-by: Jakub Jaruszewski <jjaruszewski@man.poznan.pl>

pkg/kube/client.go

@@ -321,20 +321,24 @@ 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.OnError(
+				retry.DefaultRetry,
+				isServerSideRetryable,
+				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
+				})
 		}
 	}
 
@@ -952,6 +956,32 @@ func isIncompatibleServerError(err error) bool {
 	return err.(*apierrors.StatusError).Status().Code == http.StatusUnsupportedMediaType
 }
 
+// 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 isResourceQuotaConflict(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 {

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,8 +274,30 @@ 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: 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
+			}(),
+		},
+		"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/dolphin:PATCH",
+				"/namespaces/default/pods/seal:PATCH",
 			},
 		},
 	}