From cb4d6ff77af55847e38846b36e82811be5c810f9 Mon Sep 17 00:00:00 2001 From: Zach Aller Date: Sun, 30 Apr 2023 23:20:46 -0500 Subject: [PATCH] fix(controller): Fix for rollouts getting stuck in loop (#2689) * possible fix for sutck rollouts Signed-off-by: zachaller * add comments Signed-off-by: zachaller * add comments Signed-off-by: zachaller * update comments Signed-off-by: zachaller --------- Signed-off-by: zachaller --- rollout/controller.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/rollout/controller.go b/rollout/controller.go index 1fa0c73ce0..b5d8122ac3 100644 --- a/rollout/controller.go +++ b/rollout/controller.go @@ -413,13 +413,18 @@ func (c *Controller) syncHandler(ctx context.Context, key string) error { } err = roCtx.reconcile() - if roCtx.newRollout != nil { - c.writeBackToInformer(roCtx.newRollout) - } if err != nil { logCtx.Errorf("roCtx.reconcile err %v", err) + // return an err here so that we do not update the informer cache with a "bad" rollout object, for the case when + // we get an error during reconciliation but c.newRollout still gets updated this can happen in syncReplicaSetRevision + // https://github.com/argoproj/argo-rollouts/issues/2522#issuecomment-1492181154 I also believe there are other cases + // that newRollout can get updated while we get an error during reconciliation + return err } - return err + if roCtx.newRollout != nil { + c.writeBackToInformer(roCtx.newRollout) + } + return nil } // writeBackToInformer writes a just recently updated Rollout back into the informer cache.