diff --git a/pkg/analyzer/cronjob.go b/pkg/analyzer/cronjob.go index e2b4310..866b2ea 100644 --- a/pkg/analyzer/cronjob.go +++ b/pkg/analyzer/cronjob.go @@ -123,15 +123,15 @@ func (analyzer CronJobAnalyzer) Analyze(a common.Analyzer) ([]common.Result, err AnalyzerErrorsMetric.WithLabelValues(kind, cronJob.Name, cronJob.Namespace).Set(float64(len(failures))) } + } - for key, value := range preAnalysis { - currentAnalysis := common.Result{ - Kind: kind, - Name: key, - Error: value.FailureDetails, - } - a.Results = append(a.Results, currentAnalysis) + for key, value := range preAnalysis { + currentAnalysis := common.Result{ + Kind: kind, + Name: key, + Error: value.FailureDetails, } + a.Results = append(a.Results, currentAnalysis) } return a.Results, nil diff --git a/pkg/analyzer/cronjob_test.go b/pkg/analyzer/cronjob_test.go index a18ecf1..639a716 100644 --- a/pkg/analyzer/cronjob_test.go +++ b/pkg/analyzer/cronjob_test.go @@ -15,219 +15,144 @@ package analyzer import ( "context" + "sort" "testing" "github.com/k8sgpt-ai/k8sgpt/pkg/common" "github.com/k8sgpt-ai/k8sgpt/pkg/kubernetes" - "github.com/magiconair/properties/assert" + "github.com/stretchr/testify/require" batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" ) -func TestCronJobSuccess(t *testing.T) { - clientset := fake.NewSimpleClientset(&batchv1.CronJob{ - ObjectMeta: metav1.ObjectMeta{ - Name: "example-cronjob", - Namespace: "default", - Annotations: map[string]string{ - "analysisDate": "2022-04-01", - }, - Labels: map[string]string{ - "app": "example-app", - }, - }, - Spec: batchv1.CronJobSpec{ - Schedule: "*/1 * * * *", - ConcurrencyPolicy: "Allow", - JobTemplate: batchv1.JobTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "app": "example-app", - }, - }, - Spec: batchv1.JobSpec{ - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "example-container", - Image: "nginx", - }, - }, - RestartPolicy: v1.RestartPolicyOnFailure, - }, - }, - }, - }, - }, - }) +func TestCronJobAnalyzer(t *testing.T) { + suspend := new(bool) + *suspend = true + + invalidStartingDeadline := new(int64) + *invalidStartingDeadline = -7 + + validStartingDeadline := new(int64) + *validStartingDeadline = 7 config := common.Analyzer{ Client: &kubernetes.Client{ - Client: clientset, - }, - Context: context.Background(), - Namespace: "default", - } - - analyzer := CronJobAnalyzer{} - analysisResults, err := analyzer.Analyze(config) - if err != nil { - t.Error(err) - } - - assert.Equal(t, len(analysisResults), 0) -} - -func TestCronJobBroken(t *testing.T) { - clientset := fake.NewSimpleClientset(&batchv1.CronJob{ - ObjectMeta: metav1.ObjectMeta{ - Name: "example-cronjob", - Namespace: "default", - Annotations: map[string]string{ - "analysisDate": "2022-04-01", - }, - Labels: map[string]string{ - "app": "example-app", - }, - }, - Spec: batchv1.CronJobSpec{ - Schedule: "*** * * * *", - ConcurrencyPolicy: "Allow", - JobTemplate: batchv1.JobTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "app": "example-app", - }, - }, - Spec: batchv1.JobSpec{ - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "example-container", - Image: "nginx", - }, - }, - RestartPolicy: v1.RestartPolicyOnFailure, - }, - }, - }, - }, - }, - }) - - config := common.Analyzer{ - Client: &kubernetes.Client{ - Client: clientset, - }, - Context: context.Background(), - Namespace: "default", - } - - analyzer := CronJobAnalyzer{} - analysisResults, err := analyzer.Analyze(config) - if err != nil { - t.Error(err) - } - - assert.Equal(t, len(analysisResults), 1) - assert.Equal(t, analysisResults[0].Name, "default/example-cronjob") - assert.Equal(t, analysisResults[0].Kind, "CronJob") -} - -func TestCronJobBrokenMultipleNamespaceFiltering(t *testing.T) { - clientset := fake.NewSimpleClientset( - &batchv1.CronJob{ - ObjectMeta: metav1.ObjectMeta{ - Name: "example-cronjob", - Namespace: "default", - Annotations: map[string]string{ - "analysisDate": "2022-04-01", - }, - Labels: map[string]string{ - "app": "example-app", - }, - }, - Spec: batchv1.CronJobSpec{ - Schedule: "*** * * * *", - ConcurrencyPolicy: "Allow", - JobTemplate: batchv1.JobTemplateSpec{ + Client: fake.NewSimpleClientset( + &batchv1.CronJob{ ObjectMeta: metav1.ObjectMeta{ + Name: "CJ1", + // This CronJob won't be list because of namespace filtering. + Namespace: "test", + }, + }, + &batchv1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: "CJ2", + Namespace: "default", + }, + // A suspended CronJob will contribute to failures. + Spec: batchv1.CronJobSpec{ + Suspend: suspend, + }, + }, + &batchv1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: "CJ3", + Namespace: "default", + }, + Spec: batchv1.CronJobSpec{ + // Valid schedule + Schedule: "*/1 * * * *", + + // Negative starting deadline + StartingDeadlineSeconds: invalidStartingDeadline, + }, + }, + &batchv1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: "CJ4", + Namespace: "default", + }, + Spec: batchv1.CronJobSpec{ + // Invalid schedule + Schedule: "*** * * * *", + }, + }, + &batchv1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: "CJ5", + Namespace: "default", + }, + Spec: batchv1.CronJobSpec{ + // Valid schedule + Schedule: "*/1 * * * *", + + // Positive starting deadline shouldn't be any problem. + StartingDeadlineSeconds: validStartingDeadline, + }, + }, + &batchv1.CronJob{ + // This cronjob shouldn't contribute to any failures. + ObjectMeta: metav1.ObjectMeta{ + Name: "successful-cronjob", + Namespace: "default", + Annotations: map[string]string{ + "analysisDate": "2022-04-01", + }, Labels: map[string]string{ "app": "example-app", }, }, - Spec: batchv1.JobSpec{ - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "example-container", - Image: "nginx", + Spec: batchv1.CronJobSpec{ + Schedule: "*/1 * * * *", + ConcurrencyPolicy: "Allow", + JobTemplate: batchv1.JobTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "example-app", + }, + }, + Spec: batchv1.JobSpec{ + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "example-container", + Image: "nginx", + }, + }, + RestartPolicy: v1.RestartPolicyOnFailure, }, }, - RestartPolicy: v1.RestartPolicyOnFailure, }, }, }, }, - }, - }, - &batchv1.CronJob{ - ObjectMeta: metav1.ObjectMeta{ - Name: "example-cronjob", - Namespace: "other-namespace", - Annotations: map[string]string{ - "analysisDate": "2022-04-01", - }, - Labels: map[string]string{ - "app": "example-app", - }, - }, - Spec: batchv1.CronJobSpec{ - Schedule: "*** * * * *", - ConcurrencyPolicy: "Allow", - JobTemplate: batchv1.JobTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "app": "example-app", - }, - }, - Spec: batchv1.JobSpec{ - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "example-container", - Image: "nginx", - }, - }, - RestartPolicy: v1.RestartPolicyOnFailure, - }, - }, - }, - }, - }, - }) - - config := common.Analyzer{ - Client: &kubernetes.Client{ - Client: clientset, + ), }, Context: context.Background(), Namespace: "default", } - analyzer := CronJobAnalyzer{} - analysisResults, err := analyzer.Analyze(config) - if err != nil { - t.Error(err) + cjAnalyzer := CronJobAnalyzer{} + results, err := cjAnalyzer.Analyze(config) + require.NoError(t, err) + + sort.Slice(results, func(i, j int) bool { + return results[i].Name < results[j].Name + }) + + expectations := []string{ + "default/CJ2", + "default/CJ3", + "default/CJ4", } - assert.Equal(t, len(analysisResults), 1) - assert.Equal(t, analysisResults[0].Name, "default/example-cronjob") - assert.Equal(t, analysisResults[0].Kind, "CronJob") + require.Equal(t, len(expectations), len(results)) + + for i, result := range results { + require.Equal(t, expectations[i], result.Name) + } } diff --git a/pkg/analyzer/pdb_test.go b/pkg/analyzer/pdb_test.go index 321ff5b..a530494 100644 --- a/pkg/analyzer/pdb_test.go +++ b/pkg/analyzer/pdb_test.go @@ -112,11 +112,6 @@ func TestPodDisruptionBudgetAnalyzer(t *testing.T) { pdbAnalyzer := PdbAnalyzer{} results, err := pdbAnalyzer.Analyze(config) require.NoError(t, err) - - for _, result := range results { - require.Equal(t, "test/PDB3", result.Name) - for _, failure := range result.Error { - require.Contains(t, failure.Text, "expected pdb pod label") - } - } + require.Equal(t, 1, len(results)) + require.Equal(t, "test/PDB3", results[0].Name) } diff --git a/pkg/analyzer/rs_test.go b/pkg/analyzer/rs_test.go index 467c19d..11eb871 100644 --- a/pkg/analyzer/rs_test.go +++ b/pkg/analyzer/rs_test.go @@ -124,30 +124,23 @@ func TestReplicaSetAnalyzer(t *testing.T) { }) expectations := []struct { - name string - failuresText []string + name string + failuresCount int }{ { - name: "default/ReplicaSet1", - failuresText: []string{ - "failed to create test replica set 1", - }, + name: "default/ReplicaSet1", + failuresCount: 1, }, { - name: "default/ReplicaSet4", - failuresText: []string{ - "failed to create test replica set 4 condition 1", - "failed to create test replica set 4 condition 3", - }, + name: "default/ReplicaSet4", + failuresCount: 2, }, } require.Equal(t, len(expectations), len(results)) - for i, expectation := range expectations { - require.Equal(t, expectation.name, results[i].Name) - for j, failure := range results[i].Error { - require.Equal(t, expectation.failuresText[j], failure.Text) - } + for i, result := range results { + require.Equal(t, expectations[i].name, result.Name) + require.Equal(t, expectations[i].failuresCount, len(result.Error)) } } diff --git a/pkg/analyzer/service_test.go b/pkg/analyzer/service_test.go index 4f1c148..d2abdca 100644 --- a/pkg/analyzer/service_test.go +++ b/pkg/analyzer/service_test.go @@ -145,21 +145,16 @@ func TestServiceAnalyzer(t *testing.T) { }) expectations := []struct { - name string - failuresText []string + name string + failuresCount int }{ { - name: "test/Endpoint1", - failuresText: []string{ - "Service has not ready endpoints, pods", - }, + name: "test/Endpoint1", + failuresCount: 1, }, { - name: "test/Service1", - failuresText: []string{ - "Service has no endpoints, expected label", - "Service has no endpoints, expected label", - }, + name: "test/Service1", + failuresCount: 2, }, } @@ -167,8 +162,6 @@ func TestServiceAnalyzer(t *testing.T) { for i, result := range results { require.Equal(t, expectations[i].name, result.Name) - for j, failure := range result.Error { - require.Contains(t, failure.Text, expectations[i].failuresText[j]) - } + require.Equal(t, expectations[i].failuresCount, len(result.Error)) } }