Skip to content

Commit

Permalink
fix: analysis controller could get into a hotloop with terminated run (
Browse files Browse the repository at this point in the history
  • Loading branch information
jessesuen committed Sep 21, 2020
1 parent 40b21ab commit 92b0598
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 2 deletions.
12 changes: 10 additions & 2 deletions analysis/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,9 @@ func (c *Controller) assessRunStatus(run *v1alpha1.AnalysisRun) (v1alpha1.Analys
now := metav1.Now()
run.Status.StartedAt = &now
}
if run.Spec.Terminate {
worstMessage = "run terminated"
}

// Iterate all metrics and update MetricResult.Phase fields based on lastest measurement(s)
for _, metric := range run.Spec.Metrics {
Expand Down Expand Up @@ -426,10 +429,15 @@ func (c *Controller) assessRunStatus(run *v1alpha1.AnalysisRun) (v1alpha1.Analys
}
}
}
if !everythingCompleted || worstStatus == "" {
if !everythingCompleted {
return v1alpha1.AnalysisPhaseRunning, ""
}
if worstStatus == "" {
if terminating {
return v1alpha1.AnalysisPhaseSuccessful, worstMessage
}
return v1alpha1.AnalysisPhaseRunning, ""
}

return worstStatus, worstMessage
}

Expand Down
39 changes: 39 additions & 0 deletions analysis/analysis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/mock"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/argoproj/argo-rollouts/utils/defaults"
Expand Down Expand Up @@ -1493,3 +1494,41 @@ func TestAssessRunStatusWorstMessageInReconcileAnalysisRun(t *testing.T) {
assert.Equal(t, v1alpha1.AnalysisPhaseFailed, newRun.Status.Phase)
assert.Equal(t, "metric \"run-forever\" assessed Failed due to failed (1) > failureLimit (0)", newRun.Status.Message)
}

func TestTerminateAnalysisRun(t *testing.T) {
f := newFixture(t)
defer f.Close()
c, _, _ := f.newController(noResyncPeriodFunc)

f.provider.On("Run", mock.Anything, mock.Anything, mock.Anything).Return(newMeasurement(v1alpha1.AnalysisPhaseError), nil)

now := metav1.Now()
run := &v1alpha1.AnalysisRun{
Spec: v1alpha1.AnalysisRunSpec{
Terminate: true,
Args: []v1alpha1.Argument{
{
Name: "service",
Value: pointer.StringPtr("rollouts-demo-canary.default.svc.cluster.local"),
},
},
Metrics: []v1alpha1.Metric{{
Name: "success-rate",
InitialDelay: "20s",
Interval: "20s",
SuccessCondition: "result[0] > 0.90",
Provider: v1alpha1.MetricProvider{
Web: &v1alpha1.WebMetric{},
},
}},
},
Status: v1alpha1.AnalysisRunStatus{
StartedAt: &now,
Phase: v1alpha1.AnalysisPhaseRunning,
},
}
newRun := c.reconcileAnalysisRun(run)
assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, newRun.Status.Phase)
assert.Equal(t, "run terminated", newRun.Status.Message)

}

0 comments on commit 92b0598

Please sign in to comment.