Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify tests for job metrics by resetting them #113166

Merged
merged 1 commit into from Oct 19, 2022
Merged

Simplify tests for job metrics by resetting them #113166

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 20 additions & 37 deletions test/integration/job/job_test.go
View file
Open in desktop
Expand Up @@ -83,9 +83,9 @@ func TestMetrics(t *testing.T) {
}()

testCases := map[string]struct {
job *batchv1.Job
wantJobFinishedNumMetricDelta metricLabelsWithValue
wantJobPodsFinishedMetricDelta metricLabelsWithValue
job *batchv1.Job
wantJobFinishedNumMetric metricLabelsWithValue
wantJobPodsFinishedMetric metricLabelsWithValue
}{
"non-indexed job": {
job: &batchv1.Job{
Expand All @@ -95,11 +95,11 @@ func TestMetrics(t *testing.T) {
CompletionMode: &nonIndexedCompletion,
},
},
wantJobFinishedNumMetricDelta: metricLabelsWithValue{
wantJobFinishedNumMetric: metricLabelsWithValue{
Labels: []string{"NonIndexed", "succeeded"},
Value: 1,
},
wantJobPodsFinishedMetricDelta: metricLabelsWithValue{
wantJobPodsFinishedMetric: metricLabelsWithValue{
Labels: []string{"NonIndexed", "succeeded"},
Value: 2,
},
Expand All @@ -112,11 +112,11 @@ func TestMetrics(t *testing.T) {
CompletionMode: &indexedCompletion,
},
},
wantJobFinishedNumMetricDelta: metricLabelsWithValue{
wantJobFinishedNumMetric: metricLabelsWithValue{
Labels: []string{"Indexed", "succeeded"},
Value: 1,
},
wantJobPodsFinishedMetricDelta: metricLabelsWithValue{
wantJobPodsFinishedMetric: metricLabelsWithValue{
Labels: []string{"Indexed", "succeeded"},
Value: 2,
},
Expand All @@ -125,17 +125,7 @@ func TestMetrics(t *testing.T) {
job_index := 0 // job index to avoid collisions between job names created by different test cases
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {

// record the metrics after the job is created
jobFinishedNumBefore, err := getCounterMetricValueForLabels(metrics.JobFinishedNum, tc.wantJobFinishedNumMetricDelta.Labels)
if err != nil {
t.Fatalf("Failed to collect the JobFinishedNum metric before creating the job: %q", err)
}
jobPodsFinishedBefore, err := getCounterMetricValueForLabels(metrics.JobPodsFinished, tc.wantJobPodsFinishedMetricDelta.Labels)
if err != nil {
t.Fatalf("Failed to collect the JobPodsFinished metric before creating the job: %q", err)
}

resetMetrics()
// create a single job and wait for its completion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call the resetMetrics function

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, it was passing locally because the cases use different labels

job := tc.job.DeepCopy()
job.Name = fmt.Sprintf("job-%v", job_index)
Expand All @@ -154,25 +144,23 @@ func TestMetrics(t *testing.T) {
validateJobSucceeded(ctx, t, clientSet, jobObj)

// verify metric values after the job is finished
validateMetricValueDeltas(t, metrics.JobFinishedNum, tc.wantJobFinishedNumMetricDelta, jobFinishedNumBefore)
validateMetricValueDeltas(t, metrics.JobPodsFinished, tc.wantJobPodsFinishedMetricDelta, jobPodsFinishedBefore)
validateMetricValue(t, metrics.JobFinishedNum, tc.wantJobFinishedNumMetric)
validateMetricValue(t, metrics.JobPodsFinished, tc.wantJobPodsFinishedMetric)
})
}
}

func validateMetricValueDeltas(t *testing.T, counterVer *basemetrics.CounterVec, wantMetricDelta metricLabelsWithValue, metricValuesBefore metricLabelsWithValue) {
func validateMetricValue(t *testing.T, counterVec *basemetrics.CounterVec, wantMetric metricLabelsWithValue) {
t.Helper()
var cmpErr error
err := wait.PollImmediate(10*time.Millisecond, 10*time.Second, func() (bool, error) {
cmpErr = nil
metricValuesAfter, err := getCounterMetricValueForLabels(counterVer, wantMetricDelta.Labels)
value, err := testutil.GetCounterMetricValue(counterVec.WithLabelValues(wantMetric.Labels...))
if err != nil {
return true, fmt.Errorf("Failed to collect the %q metric after the job is finished: %q", counterVer.Name, err)
return true, fmt.Errorf("collecting the %q metric: %q", counterVec.Name, err)
}
wantDelta := wantMetricDelta.Value
gotDelta := metricValuesAfter.Value - metricValuesBefore.Value
if wantDelta != gotDelta {
cmpErr = fmt.Errorf("Unexepected metric delta for %q metric with labels %q. want: %v, got: %v", counterVer.Name, wantMetricDelta.Labels, wantDelta, gotDelta)
if wantMetric.Value != int(value) {
cmpErr = fmt.Errorf("Unexpected metric delta for %q metric with labels %q. want: %v, got: %v", counterVec.Name, wantMetric.Labels, wantMetric.Value, int(value))
return false, nil
}
return true, nil
Expand All @@ -185,16 +173,6 @@ func validateMetricValueDeltas(t *testing.T, counterVer *basemetrics.CounterVec,
}
}

func getCounterMetricValueForLabels(counterVec *basemetrics.CounterVec, labels []string) (metricLabelsWithValue, error) {
var result metricLabelsWithValue = metricLabelsWithValue{Labels: labels}
value, err := testutil.GetCounterMetricValue(counterVec.WithLabelValues(labels...))
if err != nil {
return result, err
}
result.Value = int(value)
return result, nil
}

// TestJobPodFailurePolicyWithFailedPodDeletedDuringControllerRestart verifies that the job is properly marked as Failed
// in a scenario when the job controller crashes between removing pod finalizers and marking the job as Failed (based on
// the pod failure policy). After the finalizer for the failed pod is removed we remove the failed pod. This step is
Expand Down Expand Up @@ -1697,6 +1675,11 @@ func startJobControllerAndWaitForCaches(restConfig *restclient.Config) (context.
return ctx, cancel
}

func resetMetrics() {
metrics.JobFinishedNum.Reset()
metrics.JobPodsFinished.Reset()
}

func createJobControllerWithSharedInformers(restConfig *restclient.Config, informerSet informers.SharedInformerFactory) (*jobcontroller.Controller, context.Context, context.CancelFunc) {
clientSet := clientset.NewForConfigOrDie(restclient.AddUserAgent(restConfig, "job-controller"))
ctx, cancel := context.WithCancel(context.Background())
Expand Down