Skip to content

Commit

Permalink
test: added missing tests for the CronJob analyzer (#1019)
Browse files Browse the repository at this point in the history 
* test: added missing tests for the CronJob analyzer

- Fixed a small bug where pre-analysis was incorrectly appended to the
  results every time at the end of the for loop. This caused the result
  for a single cronjob failure to be appended multiple times in the
  final results.

- Added missing test cases to ensure proper testing of the CronJob
  analyzer. The addition of these missing test cases has increased the
  code coverage of this analyzer to over 96%.

Partially Addresses: #889

Signed-off-by: VaibhavMalik4187 <vaibhavmalik2018@gmail.com>

* test: removed failure strings matching from tests

It is possible that the error or failure strings might change in the
future, causing the tests to fail. This commit addresses that issue by
removing the matching of failure text from various analyzer tests.

Signed-off-by: VaibhavMalik4187 <vaibhavmalik2018@gmail.com>

---------

Signed-off-by: VaibhavMalik4187 <vaibhavmalik2018@gmail.com>
  • Loading branch information
@VaibhavMalik4187
VaibhavMalik4187 committed Mar 21, 2024
1 parent ebfbba9 commit 3c1c055
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 217 deletions.
14 changes: 7 additions & 7 deletions pkg/analyzer/cronjob.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
271 changes: 98 additions & 173 deletions pkg/analyzer/cronjob_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

config := common.Analyzer{
Client: &kubernetes.Client{
Client: clientset,
},
Context: context.Background(),
Namespace: "default",
}
invalidStartingDeadline := new(int64)
*invalidStartingDeadline = -7

analyzer := CronJobAnalyzer{}
analysisResults, err := analyzer.Analyze(config)
if err != nil {
t.Error(err)
}
validStartingDeadline := new(int64)
*validStartingDeadline = 7

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",
config := common.Analyzer{
Client: &kubernetes.Client{
Client: fake.NewSimpleClientset(
&batchv1.CronJob{
ObjectMeta: metav1.ObjectMeta{
Name: "CJ1",
// This CronJob won't be list because of namespace filtering.
Namespace: "test",
},
},
Spec: batchv1.JobSpec{
Template: v1.PodTemplateSpec{
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "example-container",
Image: "nginx",
},
},
RestartPolicy: v1.RestartPolicyOnFailure,
},
&batchv1.CronJob{
ObjectMeta: metav1.ObjectMeta{
Name: "CJ2",
Namespace: "default",
},
// A suspended CronJob will contribute to failures.
Spec: batchv1.CronJobSpec{
Suspend: suspend,
},
},
},
},
})

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")
}
&batchv1.CronJob{
ObjectMeta: metav1.ObjectMeta{
Name: "CJ3",
Namespace: "default",
},
Spec: batchv1.CronJobSpec{
// Valid schedule
Schedule: "*/1 * * * *",

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",
// Negative starting deadline
StartingDeadlineSeconds: invalidStartingDeadline,
},
},
},
Spec: batchv1.CronJobSpec{
Schedule: "*** * * * *",
ConcurrencyPolicy: "Allow",
JobTemplate: batchv1.JobTemplateSpec{
&batchv1.CronJob{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"app": "example-app",
},
Name: "CJ4",
Namespace: "default",
},
Spec: batchv1.JobSpec{
Template: v1.PodTemplateSpec{
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "example-container",
Image: "nginx",
},
},
RestartPolicy: v1.RestartPolicyOnFailure,
},
},
Spec: batchv1.CronJobSpec{
// Invalid schedule
Schedule: "*** * * * *",
},
},
},
},
&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",
&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,
},
},
},
Spec: batchv1.CronJobSpec{
Schedule: "*** * * * *",
ConcurrencyPolicy: "Allow",
JobTemplate: batchv1.JobTemplateSpec{
&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,
},
},
},
},
},
})

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)
}
}
9 changes: 2 additions & 7 deletions pkg/analyzer/pdb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
25 changes: 9 additions & 16 deletions pkg/analyzer/rs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

0 comments on commit 3c1c055

Please sign in to comment.