Skip to content

Commit

Permalink
Add tests for requeuing runs
Browse files Browse the repository at this point in the history
  • Loading branch information
@jessesuen
jessesuen committed Sep 27, 2019
1 parent 8cacf66 commit 0eb059f
Show file tree
Hide file tree
Showing 10 changed files with 247 additions and 43 deletions.
18 changes: 12 additions & 6 deletions analysis/analysis.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func generateMetricTasks(run *v1alpha1.AnalysisRun) []metricTask {
continue
}
lastMeasurement := analysisutil.LastMeasurement(run, metric.Name)
if lastMeasurement != nil && lastMeasurement.FinishedAt.IsZero() {
if lastMeasurement != nil && lastMeasurement.FinishedAt == nil {
// last measurement is still in-progress. need to complete it
log.WithField("metric", metric.Name).Infof("resuming in-progress measurement")
tasks = append(tasks, metricTask{
Expand Down Expand Up @@ -217,25 +217,28 @@ func assessMetricStatus(metric *v1alpha1.Metric, result *v1alpha1.MetricResult,
}

// calculateNextReconcileTime calculates the next time that this AnalysisRun should be reconciled,
// based on the earliest time of all metrics intervals and their finishedAt timestamps
// based on the earliest time of all metrics intervals, counts, and their finishedAt timestamps
func calculateNextReconcileTime(run *v1alpha1.AnalysisRun) *time.Time {
log := logutil.WithAnalysisRun(run)
var reconcileTime *time.Time
for _, metric := range run.Spec.AnalysisSpec.Metrics {
if analysisutil.MetricCompleted(run, metric.Name) {
// NOTE: this also coveres the case where metric.Count is reached
continue
}
lastMeasurement := analysisutil.LastMeasurement(run, metric.Name)
if lastMeasurement == nil {
// no measurement was started. we should not get here
// no measurement was started. we should never get here
log.WithField("metric", metric.Name).Warnf("metric never started. not factored into enqueue time")
continue
}
if lastMeasurement.FinishedAt.IsZero() {
if lastMeasurement.FinishedAt == nil {
// unfinished in-flight measurement.
// TODO(jessesuen) perhaps ask provider for an appropriate time to poll?
continue
}
if metric.Interval == nil {
// a measurement was already taken, and reoccurrence was not desired. no need to re-enqueue
// a measurement was already taken, and reoccurrence was not desired
continue
}
// Take the earliest time of all metrics
Expand All @@ -244,5 +247,8 @@ func calculateNextReconcileTime(run *v1alpha1.AnalysisRun) *time.Time {
reconcileTime = &metricReconcileTime
}
}
return reconcileTime
if reconcileTime != nil {
return reconcileTime
}
return nil
}
193 changes: 167 additions & 26 deletions analysis/analysis_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import (
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
)

func timePtr(t metav1.Time) *metav1.Time {
return &t
}

func TestGenerateMetricTasksInterval(t *testing.T) {
run := &v1alpha1.AnalysisRun{
Spec: v1alpha1.AnalysisRunSpec{
Expand All @@ -32,8 +36,8 @@ func TestGenerateMetricTasksInterval(t *testing.T) {
{
Value: "99",
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: metav1.NewTime(time.Now().Add(-50 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-50 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-50 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-50 * time.Second))),
},
},
},
Expand All @@ -48,8 +52,8 @@ func TestGenerateMetricTasksInterval(t *testing.T) {
{
// ensure we do take measurements when outside interval
successRate := run.Status.MetricResults["success-rate"]
successRate.Measurements[0].StartedAt = metav1.NewTime(time.Now().Add(-61 * time.Second))
successRate.Measurements[0].FinishedAt = metav1.NewTime(time.Now().Add(-61 * time.Second))
successRate.Measurements[0].StartedAt = timePtr(metav1.NewTime(time.Now().Add(-61 * time.Second)))
successRate.Measurements[0].FinishedAt = timePtr(metav1.NewTime(time.Now().Add(-61 * time.Second)))
run.Status.MetricResults["success-rate"] = successRate
tasks := generateMetricTasks(run)
assert.Equal(t, 1, len(tasks))
Expand Down Expand Up @@ -108,8 +112,8 @@ func TestGenerateMetricTasksNoInterval(t *testing.T) {
{
Value: "99",
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: metav1.NewTime(time.Now().Add(-50 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-50 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-50 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-50 * time.Second))),
},
},
},
Expand Down Expand Up @@ -151,7 +155,7 @@ func TestGenerateMetricTasksIncomplete(t *testing.T) {
{
Value: "99",
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: metav1.NewTime(time.Now().Add(-50 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-50 * time.Second))),
},
},
},
Expand Down Expand Up @@ -234,13 +238,13 @@ func TestAssessMetricStatusInFlightMeasurement(t *testing.T) {
{
Value: "99",
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: metav1.NewTime(time.Now().Add(-60 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-60 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
},
{
Value: "99",
Status: v1alpha1.AnalysisStatusRunning,
StartedAt: metav1.NewTime(time.Now()),
StartedAt: timePtr(metav1.NewTime(time.Now())),
},
},
}
Expand All @@ -258,8 +262,8 @@ func TestAssessMetricStatusMaxFailures(t *testing.T) { // max failures
{
Value: "99",
Status: v1alpha1.AnalysisStatusFailed,
StartedAt: metav1.NewTime(time.Now().Add(-60 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-60 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
},
},
}
Expand All @@ -274,37 +278,37 @@ func TestAssessMetricStatusConsecutiveErrors(t *testing.T) {
Measurements: []v1alpha1.Measurement{
{
Status: v1alpha1.AnalysisStatusError,
StartedAt: metav1.NewTime(time.Now().Add(-60 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-60 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
},
{
Status: v1alpha1.AnalysisStatusError,
StartedAt: metav1.NewTime(time.Now().Add(-50 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-50 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-50 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-50 * time.Second))),
},
{
Status: v1alpha1.AnalysisStatusError,
StartedAt: metav1.NewTime(time.Now().Add(-40 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-40 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-40 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-40 * time.Second))),
},
{
Status: v1alpha1.AnalysisStatusError,
StartedAt: metav1.NewTime(time.Now().Add(-30 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-30 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-30 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-30 * time.Second))),
},
{
Status: v1alpha1.AnalysisStatusError,
StartedAt: metav1.NewTime(time.Now().Add(-20 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-20 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-20 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-20 * time.Second))),
},
},
}
assert.Equal(t, v1alpha1.AnalysisStatusError, assessMetricStatus(metric, result, false))
assert.Equal(t, v1alpha1.AnalysisStatusError, assessMetricStatus(metric, result, true))
result.Measurements[2] = v1alpha1.Measurement{
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: metav1.NewTime(time.Now().Add(-40 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-40 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-40 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-40 * time.Second))),
}
assert.Equal(t, v1alpha1.AnalysisStatusSuccessful, assessMetricStatus(metric, result, true))
assert.Equal(t, v1alpha1.AnalysisStatusRunning, assessMetricStatus(metric, result, false))
Expand All @@ -322,8 +326,8 @@ func TestAssessMetricStatusCountReached(t *testing.T) {
{
Value: "99",
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: metav1.NewTime(time.Now().Add(-60 * time.Second)),
FinishedAt: metav1.NewTime(time.Now().Add(-60 * time.Second)),
StartedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
},
},
}
Expand All @@ -332,3 +336,140 @@ func TestAssessMetricStatusCountReached(t *testing.T) {
result.Inconclusive = 5
assert.Equal(t, v1alpha1.AnalysisStatusInconclusive, assessMetricStatus(metric, result, false))
}

func TestCalculateNextReconcileTimeInterval(t *testing.T) {
now := metav1.Now()
nowMinus30 := metav1.NewTime(now.Add(time.Second * -30))
run := &v1alpha1.AnalysisRun{
Spec: v1alpha1.AnalysisRunSpec{
AnalysisSpec: v1alpha1.AnalysisTemplateSpec{
Metrics: []v1alpha1.Metric{
{
Name: "success-rate",
Interval: pointer.Int32Ptr(60),
},
},
},
},
Status: &v1alpha1.AnalysisRunStatus{
Status: v1alpha1.AnalysisStatusRunning,
MetricResults: map[string]v1alpha1.MetricResult{
"success-rate": {
Status: v1alpha1.AnalysisStatusRunning,
Measurements: []v1alpha1.Measurement{
{
Value: "99",
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: &nowMinus30,
FinishedAt: &nowMinus30,
},
},
},
},
},
}
// ensure we requeue at correct interval
assert.Equal(t, now.Add(time.Second*30), *calculateNextReconcileTime(run))
// when in-flight is not set, we do not requeue
run.Status.MetricResults["success-rate"].Measurements[0].FinishedAt = nil
run.Status.MetricResults["success-rate"].Measurements[0].Status = v1alpha1.AnalysisStatusRunning
assert.Nil(t, calculateNextReconcileTime(run))
// do not queue completed metrics
nowMinus120 := metav1.NewTime(now.Add(time.Second * -120))
run.Status.MetricResults["success-rate"] = v1alpha1.MetricResult{
Status: v1alpha1.AnalysisStatusSuccessful,
Measurements: []v1alpha1.Measurement{
{
Value: "99",
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: &nowMinus120,
FinishedAt: &nowMinus120,
},
},
}
assert.Nil(t, calculateNextReconcileTime(run))
}

func TestCalculateNextReconcileTimeNoInterval(t *testing.T) {
now := metav1.Now()
run := &v1alpha1.AnalysisRun{
Spec: v1alpha1.AnalysisRunSpec{
AnalysisSpec: v1alpha1.AnalysisTemplateSpec{
Metrics: []v1alpha1.Metric{
{
Name: "success-rate",
Count: 1,
},
},
},
},
Status: &v1alpha1.AnalysisRunStatus{
Status: v1alpha1.AnalysisStatusRunning,
MetricResults: map[string]v1alpha1.MetricResult{
"success-rate": {
Status: v1alpha1.AnalysisStatusSuccessful,
Measurements: []v1alpha1.Measurement{
{
Value: "99",
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: &now,
FinishedAt: &now,
},
},
},
},
},
}
assert.Nil(t, calculateNextReconcileTime(run))
}

func TestCalculateNextReconcileEarliestMetric(t *testing.T) {
now := metav1.Now()
nowMinus30 := metav1.NewTime(now.Add(time.Second * -30))
nowMinus50 := metav1.NewTime(now.Add(time.Second * -50))
run := &v1alpha1.AnalysisRun{
Spec: v1alpha1.AnalysisRunSpec{
AnalysisSpec: v1alpha1.AnalysisTemplateSpec{
Metrics: []v1alpha1.Metric{
{
Name: "success-rate",
Interval: pointer.Int32Ptr(60),
},
{
Name: "latency",
Interval: pointer.Int32Ptr(60),
},
},
},
},
Status: &v1alpha1.AnalysisRunStatus{
Status: v1alpha1.AnalysisStatusRunning,
MetricResults: map[string]v1alpha1.MetricResult{
"success-rate": {
Status: v1alpha1.AnalysisStatusRunning,
Measurements: []v1alpha1.Measurement{
{
Value: "99",
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: &nowMinus30,
FinishedAt: &nowMinus30,
},
},
},
"latency": {
Status: v1alpha1.AnalysisStatusRunning,
Measurements: []v1alpha1.Measurement{
{
Value: "1",
Status: v1alpha1.AnalysisStatusSuccessful,
StartedAt: &nowMinus50,
FinishedAt: &nowMinus50,
},
},
},
},
},
}
// ensure we requeue at correct interval
assert.Equal(t, now.Add(time.Second*10), *calculateNextReconcileTime(run))
}
2 changes: 2 additions & 0 deletions manifests/crds/analysis-run-crd.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ spec:
- status
type: object
type: array
message:
type: string
name:
type: string
status:
Expand Down
2 changes: 2 additions & 0 deletions manifests/install.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ spec:
- status
type: object
type: array
message:
type: string
name:
type: string
status:
Expand Down
2 changes: 2 additions & 0 deletions manifests/namespace-install.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ spec:
- status
type: object
type: array
message:
type: string
name:
type: string
status:
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/rollouts/v1alpha1/analysis_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ type Measurement struct {
// Status is the status of this single measurement
Status AnalysisStatus `json:"status"`
// StartedAt is the timestamp in which this measurement started to be measured
StartedAt metav1.Time `json:"startedAt,omitempty"`
StartedAt *metav1.Time `json:"startedAt,omitempty"`
// FinishedAt is the timestamp in which this measurement completed and value was collected
FinishedAt metav1.Time `json:"finishedAt,omitempty"`
FinishedAt *metav1.Time `json:"finishedAt,omitempty"`
// Value is the measured value of the metric
Value string `json:"value,omitempty"`
// Metadata stores additional metadata about this metric result, used by the different providers
Expand Down
Loading

0 comments on commit 0eb059f

Please sign in to comment.