From e43d45b10c888ed2c8ec25bbc4e7a50ead2cb1ad Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 4 Dec 2025 10:58:40 -0700 Subject: [PATCH 1/2] Retry NGINX provisioning on all errors 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. --- .../controller/provisioner/provisioner.go | 19 ++++++++++--------- internal/controller/provisioner/setter.go | 4 +--- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/internal/controller/provisioner/provisioner.go b/internal/controller/provisioner/provisioner.go index d2957404f4..f901fcf1f5 100644 --- a/internal/controller/provisioner/provisioner.go +++ b/internal/controller/provisioner/provisioner.go @@ -2,6 +2,7 @@ package provisioner import ( "context" + "errors" "fmt" "slices" "strings" @@ -246,33 +247,29 @@ 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 - } + 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 +324,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(), diff --git a/internal/controller/provisioner/setter.go b/internal/controller/provisioner/setter.go index b7204ade4a..d529e01af9 100644 --- a/internal/controller/provisioner/setter.go +++ b/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 { From 42e660ac481a502e2ff0734aca1f61592f7c4d99 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 4 Dec 2025 12:21:48 -0700 Subject: [PATCH 2/2] Add debug log for error message --- internal/controller/provisioner/provisioner.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/controller/provisioner/provisioner.go b/internal/controller/provisioner/provisioner.go index f901fcf1f5..e489e693be 100644 --- a/internal/controller/provisioner/provisioner.go +++ b/internal/controller/provisioner/provisioner.go @@ -255,6 +255,12 @@ func (p *NginxProvisioner) provisionNginx( func(ctx context.Context) (bool, error) { res, upsertErr = controllerutil.CreateOrUpdate(ctx, p.k8sClient, obj, objectSpecSetter(obj)) if upsertErr != 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