Retry NGINX provisioning on all errors (#4399)

Problem: Formerly, we would not retry provisioning if we got an error that a resource already exists. However, in certain cases, the timing was just right where back to back calls to CreateOrUpdate would result in the second call initially not finding the resource (as it was being created but didn't exist yet), and then when it decided to call Create, the resource now existed, and it would fail to update the resource to the new change.

Solution: Retry the CreateOrUpdate call no matter what error is returned. This ensures that if we happen to hit this quick succession scenario, the second update call would eventually succeed and not fail immediately, after the CreateOrUpdate function determined that the resource does exist and simply needs an update, not a Create.

Also added a return statement to safeguard against potential panics if an object is nil, due to a similar potential timing issue.

Author: sjberman

internal/controller/provisioner/provisioner.go

@@ -2,6 +2,7 @@ package provisioner
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"slices"
 	"strings"
@@ -246,33 +247,35 @@ func (p *NginxProvisioner) provisionNginx(
 		createCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
 
 		var res controllerutil.OperationResult
+		var upsertErr error
 		if err := wait.PollUntilContextCancel(
 			createCtx,
 			500*time.Millisecond,
 			true, /* poll immediately */
 			func(ctx context.Context) (bool, error) {
-				var upsertErr error
 				res, upsertErr = controllerutil.CreateOrUpdate(ctx, p.k8sClient, obj, objectSpecSetter(obj))
 				if upsertErr != nil {
-					if !apierrors.IsAlreadyExists(upsertErr) && !apierrors.IsConflict(upsertErr) {
-						return false, upsertErr
-					}
-					if apierrors.IsConflict(upsertErr) {
-						return false, nil
-					}
+					p.cfg.Logger.V(1).Info(
+						"Retrying CreateOrUpdate for nginx resource after error",
+						"namespace", gateway.GetNamespace(),
+						"name", resourceName,
+						"error", upsertErr.Error(),
+					)
+					return false, nil //nolint:nilerr // continue retrying
 				}
 				return true, nil
 			},
 		); err != nil {
+			fullErr := errors.Join(err, upsertErr)
 			p.cfg.EventRecorder.Eventf(
 				obj,
 				corev1.EventTypeWarning,
 				"CreateOrUpdateFailed",
 				"Failed to create or update nginx resource: %s",
-				err.Error(),
+				fullErr.Error(),
 			)
 			cancel()
-			return err
+			return fullErr
 		}
 		cancel()
 
@@ -327,6 +330,10 @@ func (p *NginxProvisioner) provisionNginx(
 			object = daemonSetObj
 		}
 
+		if object == nil {
+			return nil
+		}
+
 		p.cfg.Logger.V(1).Info(
 			"Restarting nginx after agent configmap update",
 			"name", object.GetName(),

internal/controller/provisioner/setter.go

@@ -119,9 +119,7 @@ func serviceSpecSetter(
 		}
 
 		// Apply NGF-managed annotations (take precedence)
-		for k, v := range objectMeta.Annotations {
-			mergedAnnotations[k] = v
-		}
+		maps.Copy(mergedAnnotations, objectMeta.Annotations)
 
 		// Store current managed keys for next reconciliation
 		if len(currentManagedKeys) > 0 {