Skip to content

Commit

Permalink
Honor DeletionTimestamp in rollout and experiment
Browse files Browse the repository at this point in the history
  • Loading branch information
dthomson25 committed Aug 14, 2019
1 parent 893d673 commit 65dced4
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 0 deletions.
5 changes: 5 additions & 0 deletions experiments/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ func (ec *ExperimentController) syncHandler(key string) error {
logCtx.Info("Reconciliation completed")
}()

if experiment.DeletionTimestamp != nil {
logutil.WithExperiment(experiment).Info("No reconciliation as experiment marked for deletion")
return nil
}

//TODO(dthomson) add validation

// List ReplicaSets owned by this Experiment, while reconciling ControllerRef
Expand Down
14 changes: 14 additions & 0 deletions experiments/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,3 +456,17 @@ func (f *fixture) getPatchedExperiment(index int) string {
}
return string(patchAction.GetPatch())
}

func TestNoReconcileForDeletedExperiment(t *testing.T) {
f := newFixture(t)
defer f.Close()

e := newExperiment("foo", nil, int32(10), pointer.BoolPtr(true))
now := metav1.Now()
e.DeletionTimestamp = &now

f.experimentLister = append(f.experimentLister, e)
f.objects = append(f.objects, e)

f.run(getKey(e, t))
}
6 changes: 6 additions & 0 deletions rollout/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,12 @@ func (c *RolloutController) syncHandler(key string) error {
// This also returns a copy of the rollout to prevent mutation of the informer cache.
r := remarshalRollout(rollout)
logCtx := logutil.WithRollout(r)

if r.ObjectMeta.DeletionTimestamp != nil {
logCtx.Info("No reconciliation as rollout marked for deletion")
return nil
}

// In order to work with HPA, the rollout.Spec.Replica field cannot be nil. As a result, the controller will update
// the rollout to have the replicas field set to the default value. see https://github.com/argoproj/argo-rollouts/issues/119
if rollout.Spec.Replicas == nil {
Expand Down
14 changes: 14 additions & 0 deletions rollout/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,20 @@ requests:
}
}

func TestNoReconcileForDeletedRollout(t *testing.T) {
f := newFixture(t)
defer f.Close()

r := newRollout("foo", 1, nil, nil)
now := metav1.Now()
r.DeletionTimestamp = &now

f.rolloutLister = append(f.rolloutLister, r)
f.objects = append(f.objects, r)

f.run(getKey(r, t))
}

// TestComputeHashChangeTolerationBlueGreen verifies that we can tolerate a change in
// controller.ComputeHash() for the blue-green strategy and do not redeploy any replicasets
func TestComputeHashChangeTolerationBlueGreen(t *testing.T) {
Expand Down

0 comments on commit 65dced4

Please sign in to comment.