fix(controller): Fix for rollouts getting stuck in loop (#2689)

* possible fix for sutck rollouts Signed-off-by: zachaller <[email protected]> * add comments Signed-off-by: zachaller <[email protected]> * add comments Signed-off-by: zachaller <[email protected]> * update comments Signed-off-by: zachaller <[email protected]> --------- Signed-off-by: zachaller <[email protected]>
argoproj · May 1, 2023 · cb4d6ff · cb4d6ff
1 parent 96f164f
commit cb4d6ff
Showing 1 changed file with 9 additions and 4 deletions.
diff --git 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.