Merge pull request #31518 from gjenkins8/gjenkins/fix_update_create

fix: Use server-side apply for object create during update

pkg/action/install.go

@@ -419,6 +419,7 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st
 		if err != nil {
 			return nil, err
 		}
+
 		if _, err := i.cfg.KubeClient.Create(
 			resourceList,
 			kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false)); err != nil && !apierrors.IsAlreadyExists(err) {

pkg/kube/client.go

@@ -125,6 +125,9 @@ const (
 	FieldValidationDirectiveStrict FieldValidationDirective = "Strict"
 )
 
+type CreateApplyFunc func(target *resource.Info) error
+type UpdateApplyFunc func(original, target *resource.Info) error
+
 func init() {
 	// Add CRDs to the scheme. They are missing by default.
 	if err := apiextv1.AddToScheme(scheme.Scheme); err != nil {
@@ -281,6 +284,36 @@ func ClientCreateOptionFieldValidationDirective(fieldValidationDirective FieldVa
 	}
 }
 
+func (c *Client) makeCreateApplyFunc(serverSideApply, forceConflicts, dryRun bool, fieldValidationDirective FieldValidationDirective) CreateApplyFunc {
+	if serverSideApply {
+		c.Logger().Debug(
+			"using server-side apply for resource creation",
+			slog.Bool("forceConflicts", forceConflicts),
+			slog.Bool("dryRun", dryRun),
+			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 nil
+		}
+	}
+
+	c.Logger().Debug("using client-side apply for resource creation")
+	return createResource
+}
+
 // Create creates Kubernetes resources specified in the resource list.
 func (c *Client) Create(resources ResourceList, options ...ClientCreateOption) (*Result, error) {
 	c.Logger().Debug("creating resource(s)", "resources", len(resources))
@@ -298,32 +331,12 @@ func (c *Client) Create(resources ResourceList, options ...ClientCreateOption) (
 		return nil, fmt.Errorf("invalid client create option(s): %w", err)
 	}
 
-	makeCreateApplyFunc := func() func(target *resource.Info) error {
-		if createOptions.serverSideApply {
-			c.Logger().Debug("using server-side apply for resource creation", slog.Bool("forceConflicts", createOptions.forceConflicts), slog.Bool("dryRun", createOptions.dryRun), slog.String("fieldValidationDirective", string(createOptions.fieldValidationDirective)))
-			return func(target *resource.Info) error {
-				err := patchResourceServerSide(target, createOptions.dryRun, createOptions.forceConflicts, createOptions.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 patching resource", slog.Any("error", err))
-					return err
-				}
-
-				logger.Debug("Patched resource")
-
-				return nil
-			}
-		}
-
-		c.Logger().Debug("using client-side apply for resource creation")
-		return createResource
-	}
-
-	if err := perform(resources, makeCreateApplyFunc()); err != nil {
+	createApplyFunc := c.makeCreateApplyFunc(
+		createOptions.serverSideApply,
+		createOptions.forceConflicts,
+		createOptions.dryRun,
+		createOptions.fieldValidationDirective)
+	if err := perform(resources, createApplyFunc); err != nil {
 		return nil, err
 	}
 	return &Result{Created: resources}, nil
@@ -525,7 +538,7 @@ func (c *Client) BuildTable(reader io.Reader, validate bool) (ResourceList, erro
 		transformRequests)
 }
 
-func (c *Client) update(originals, targets ResourceList, updateApplyFunc UpdateApplyFunc) (*Result, error) {
+func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateApplyFunc, updateApplyFunc UpdateApplyFunc) (*Result, error) {
 	updateErrors := []error{}
 	res := &Result{}
 
@@ -545,7 +558,7 @@ func (c *Client) update(originals, targets ResourceList, updateApplyFunc UpdateA
 			res.Created = append(res.Created, target)
 
 			// Since the resource does not exist, create it.
-			if err := createResource(target); err != nil {
+			if err := createApplyFunc(target); err != nil {
 				return fmt.Errorf("failed to create resource: %w", err)
 			}
 
@@ -699,8 +712,6 @@ func ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManag
 	}
 }
 
-type UpdateApplyFunc func(original, target *resource.Info) error
-
 // Update takes the current list of objects and target list of objects and
 // creates resources that don't already exist, updates resources that have been
 // modified in the target configuration, and deletes resources from the current
@@ -736,6 +747,12 @@ func (c *Client) Update(originals, targets ResourceList, options ...ClientUpdate
 		return &Result{}, fmt.Errorf("invalid operation: cannot use server-side apply and force replace together")
 	}
 
+	createApplyFunc := c.makeCreateApplyFunc(
+		updateOptions.serverSideApply,
+		updateOptions.forceConflicts,
+		updateOptions.dryRun,
+		updateOptions.fieldValidationDirective)
+
 	makeUpdateApplyFunc := func() UpdateApplyFunc {
 		if updateOptions.forceReplace {
 			c.Logger().Debug(
@@ -796,7 +813,7 @@ func (c *Client) Update(originals, targets ResourceList, options ...ClientUpdate
 		}
 	}
 
-	return c.update(originals, targets, makeUpdateApplyFunc())
+	return c.update(originals, targets, createApplyFunc, makeUpdateApplyFunc())
 }
 
 // Delete deletes Kubernetes resources specified in the resources list with

pkg/kube/client_test.go

@@ -350,9 +350,7 @@ func TestUpdate(t *testing.T) {
 		"/namespaces/default/pods/otter:GET",
 		"/namespaces/default/pods/otter:PATCH",
 		"/namespaces/default/pods/dolphin:GET",
-		"/namespaces/default/pods:POST", // create dolphin
-		"/namespaces/default/pods:POST", // retry due to 409
-		"/namespaces/default/pods:POST", // retry due to 409
+		"/namespaces/default/pods/dolphin:PATCH", // create dolphin
 		"/namespaces/default/pods/squid:GET",
 		"/namespaces/default/pods/squid:DELETE",
 		"/namespaces/default/pods/notfound:GET",
@@ -464,6 +462,8 @@ func TestUpdate(t *testing.T) {
 						return newResponseJSON(http.StatusConflict, resourceQuotaConflict)
 					}
 
+					return newResponse(http.StatusOK, &listTarget.Items[1])
+				case p == "/namespaces/default/pods/dolphin" && m == http.MethodPatch:
 					return newResponse(http.StatusOK, &listTarget.Items[1])
 				case p == "/namespaces/default/pods/squid" && m == http.MethodDelete:
 					return newResponse(http.StatusOK, &listTarget.Items[1])
@@ -485,10 +485,9 @@ func TestUpdate(t *testing.T) {
 						Reason:  metav1.StatusReasonForbidden,
 						Code:    http.StatusForbidden,
 					})
-				default:
 				}
 
-				t.Fail()
+				t.FailNow()
 				return nil, nil
 			}