From 15f11c42e716954fbbd10579c5e559ae529c2c9f Mon Sep 17 00:00:00 2001 From: stinkyfingers Date: Wed, 30 Mar 2022 11:00:49 -0500 Subject: [PATCH] omit default backoffLimit value fix tests & template nil check adds tests add tests --- ci/limits.json | 2 +- cmd/ketch/job_deploy.go | 5 -- cmd/ketch/job_deploy_test.go | 3 +- cmd/ketch/job_export_test.go | 3 +- cmd/ketch/job_list_test.go | 23 +++++++- cmd/ketch/job_remove_test.go | 3 +- config/crd/bases/theketch.io_jobs.yaml | 1 + internal/api/v1beta1/job_types.go | 19 +++---- internal/api/v1beta1/job_types_test.go | 72 ++++++++++++++++++++++++++ internal/chart/chartutils_test.go | 4 +- internal/chart/job_chart_test.go | 3 +- internal/templates/job/yamls/job.yaml | 2 +- 12 files changed, 116 insertions(+), 24 deletions(-) create mode 100644 internal/api/v1beta1/job_types_test.go diff --git a/ci/limits.json b/ci/limits.json index 8c6d316e..81c2126c 100644 --- a/ci/limits.json +++ b/ci/limits.json @@ -1,5 +1,5 @@ { - "github.com/theketchio/ketch/cmd/ketch": 61.4, + "github.com/theketchio/ketch/cmd/ketch": 61.3, "github.com/theketchio/ketch/cmd/ketch/configuration": 0, "github.com/theketchio/ketch/cmd/ketch/output": 68.3, "github.com/theketchio/ketch/cmd/manager": 0, diff --git a/cmd/ketch/job_deploy.go b/cmd/ketch/job_deploy.go index ac678c7f..db813e59 100644 --- a/cmd/ketch/job_deploy.go +++ b/cmd/ketch/job_deploy.go @@ -23,8 +23,6 @@ Deploy a job. const ( defaultJobVersion = "v1" defaultJobParallelism = 1 - defaultJobCompletions = 1 - defaultJobBackoffLimit = 6 defaultJobRestartPolicy = "Never" ) @@ -87,9 +85,6 @@ func setJobSpecDefaults(jobSpec *ketchv1.JobSpec) { if jobSpec.Completions == 0 { jobSpec.Completions = jobSpec.Parallelism } - if jobSpec.BackoffLimit == 0 { - jobSpec.BackoffLimit = defaultJobBackoffLimit - } if jobSpec.Policy.RestartPolicy == "" { jobSpec.Policy.RestartPolicy = defaultJobRestartPolicy } diff --git a/cmd/ketch/job_deploy_test.go b/cmd/ketch/job_deploy_test.go index 0adc9340..fb3c058b 100644 --- a/cmd/ketch/job_deploy_test.go +++ b/cmd/ketch/job_deploy_test.go @@ -12,6 +12,7 @@ import ( ketchv1 "github.com/theketchio/ketch/internal/api/v1beta1" "github.com/theketchio/ketch/internal/mocks" + "github.com/theketchio/ketch/internal/utils/conversions" ) func TestJobDeploy(t *testing.T) { @@ -59,7 +60,7 @@ policy: Parallelism: 1, Completions: 1, Suspend: false, - BackoffLimit: 6, + BackoffLimit: conversions.IntPtr(6), Containers: []ketchv1.Container{ { Name: "lister", diff --git a/cmd/ketch/job_export_test.go b/cmd/ketch/job_export_test.go index 9ec80f5c..75d24349 100644 --- a/cmd/ketch/job_export_test.go +++ b/cmd/ketch/job_export_test.go @@ -11,6 +11,7 @@ import ( ketchv1 "github.com/theketchio/ketch/internal/api/v1beta1" "github.com/theketchio/ketch/internal/mocks" + "github.com/theketchio/ketch/internal/utils/conversions" ) func TestJobExport(t *testing.T) { @@ -24,7 +25,7 @@ func TestJobExport(t *testing.T) { Parallelism: 1, Completions: 1, Suspend: false, - BackoffLimit: 6, + BackoffLimit: conversions.IntPtr(6), Containers: []ketchv1.Container{ { Name: "lister", diff --git a/cmd/ketch/job_list_test.go b/cmd/ketch/job_list_test.go index ebb1a8cd..f2d3cd9e 100644 --- a/cmd/ketch/job_list_test.go +++ b/cmd/ketch/job_list_test.go @@ -11,6 +11,7 @@ import ( ketchv1 "github.com/theketchio/ketch/internal/api/v1beta1" "github.com/theketchio/ketch/internal/mocks" + "github.com/theketchio/ketch/internal/utils/conversions" ) func TestJobList(t *testing.T) { @@ -24,7 +25,7 @@ func TestJobList(t *testing.T) { Parallelism: 1, Completions: 1, Suspend: false, - BackoffLimit: 6, + BackoffLimit: conversions.IntPtr(6), Containers: []ketchv1.Container{ { Name: "lister", @@ -80,7 +81,7 @@ func TestJobListNames(t *testing.T) { Parallelism: 1, Completions: 1, Suspend: false, - BackoffLimit: 6, + BackoffLimit: conversions.IntPtr(6), Containers: []ketchv1.Container{ { Name: "lister", @@ -109,6 +110,24 @@ func TestJobListNames(t *testing.T) { }, wantOut: []string{"hello"}, }, + { + name: "filter", + cfg: &mocks.Configuration{ + CtrlClientObjects: []runtime.Object{mockJob}, + DynamicClientObjects: []runtime.Object{}, + }, + filter: "goodbye", + wantOut: []string{}, + }, + { + name: "filter", + cfg: &mocks.Configuration{ + CtrlClientObjects: []runtime.Object{mockJob}, + DynamicClientObjects: []runtime.Object{}, + }, + filter: "hello", + wantOut: []string{"hello"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/cmd/ketch/job_remove_test.go b/cmd/ketch/job_remove_test.go index d6ddde02..a1f8ad9a 100644 --- a/cmd/ketch/job_remove_test.go +++ b/cmd/ketch/job_remove_test.go @@ -11,6 +11,7 @@ import ( ketchv1 "github.com/theketchio/ketch/internal/api/v1beta1" "github.com/theketchio/ketch/internal/mocks" + "github.com/theketchio/ketch/internal/utils/conversions" ) func TestJobRemove(t *testing.T) { @@ -24,7 +25,7 @@ func TestJobRemove(t *testing.T) { Parallelism: 1, Completions: 1, Suspend: false, - BackoffLimit: 6, + BackoffLimit: conversions.IntPtr(6), Containers: []ketchv1.Container{ { Name: "lister", diff --git a/config/crd/bases/theketch.io_jobs.yaml b/config/crd/bases/theketch.io_jobs.yaml index ec75a8dd..5dff3d65 100644 --- a/config/crd/bases/theketch.io_jobs.yaml +++ b/config/crd/bases/theketch.io_jobs.yaml @@ -41,6 +41,7 @@ spec: description: JobSpec defines the desired state of Job properties: backoffLimit: + minimum: 0 type: integer completions: type: integer diff --git a/internal/api/v1beta1/job_types.go b/internal/api/v1beta1/job_types.go index 9af213e8..e05e1994 100644 --- a/internal/api/v1beta1/job_types.go +++ b/internal/api/v1beta1/job_types.go @@ -22,15 +22,16 @@ import ( // JobSpec defines the desired state of Job type JobSpec struct { - Version string `json:"version,omitempty"` - Type string `json:"type"` - Name string `json:"name"` - Framework string `json:"framework"` - Description string `json:"description,omitempty"` - Parallelism int `json:"parallelism,omitempty"` - Completions int `json:"completions,omitempty"` - Suspend bool `json:"suspend,omitempty"` - BackoffLimit int `json:"backoffLimit,omitempty"` + Version string `json:"version,omitempty"` + Type string `json:"type"` + Name string `json:"name"` + Framework string `json:"framework"` + Description string `json:"description,omitempty"` + Parallelism int `json:"parallelism,omitempty"` + Completions int `json:"completions,omitempty"` + Suspend bool `json:"suspend,omitempty"` + //+kubebuilder:validation:Minimum=0 + BackoffLimit *int `json:"backoffLimit,omitempty"` Containers []Container `json:"containers,omitempty"` Policy Policy `json:"policy,omitempty"` } diff --git a/internal/api/v1beta1/job_types_test.go b/internal/api/v1beta1/job_types_test.go new file mode 100644 index 00000000..2266b155 --- /dev/null +++ b/internal/api/v1beta1/job_types_test.go @@ -0,0 +1,72 @@ +package v1beta1 + +import ( + "testing" + + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestCondition(t *testing.T) { + tests := []struct { + description string + jobStatus *JobStatus + conditionType ConditionType + expected *Condition + }{ + { + description: "found", + jobStatus: &JobStatus{Conditions: []Condition{{Type: "type1", Status: "active"}, {Type: "type2", Status: "failed"}}}, + conditionType: ConditionType("type1"), + expected: &Condition{Type: "type1", Status: "active"}, + }, + { + description: "not found", + jobStatus: &JobStatus{Conditions: []Condition{{Type: "type1", Status: "active"}, {Type: "type2", Status: "failed"}}}, + conditionType: ConditionType("complete"), + expected: nil, + }, + } + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + res := tt.jobStatus.Condition(tt.conditionType) + require.Equal(t, tt.expected, res) + }) + } +} + +func TestSetCondition(t *testing.T) { + now := metav1.Time{} + j := &Job{ + Status: JobStatus{ + Conditions: []Condition{ + { + Type: ConditionType("type1"), + Status: "active", + Message: "message-1", + }, + }, + }, + } + expected := &Job{ + Status: JobStatus{ + Conditions: []Condition{ + { + Type: ConditionType("type1"), + Status: "active", + Message: "message-1", + }, + { + Type: ConditionType("type2"), + Status: "failed", + Message: "message-2", + LastTransitionTime: &now, + }, + }, + }, + } + + j.SetCondition("type2", v1.ConditionStatus("failed"), "message-2", now) + require.Equal(t, expected, j) +} diff --git a/internal/chart/chartutils_test.go b/internal/chart/chartutils_test.go index ddb85969..77372474 100644 --- a/internal/chart/chartutils_test.go +++ b/internal/chart/chartutils_test.go @@ -30,7 +30,7 @@ func TestBufferedFiles(t *testing.T) { Parallelism: 2, Completions: 2, Suspend: false, - BackoffLimit: 4, + BackoffLimit: conversions.IntPtr(4), Containers: []ketchv1.Container{ { Name: "test", @@ -105,7 +105,7 @@ func TestGetValuesMap(t *testing.T) { Parallelism: 2, Completions: 2, Suspend: false, - BackoffLimit: 4, + BackoffLimit: conversions.IntPtr(4), Containers: []ketchv1.Container{ { Name: "test", diff --git a/internal/chart/job_chart_test.go b/internal/chart/job_chart_test.go index 41eb1681..38e89b3b 100644 --- a/internal/chart/job_chart_test.go +++ b/internal/chart/job_chart_test.go @@ -9,6 +9,7 @@ import ( ketchv1 "github.com/theketchio/ketch/internal/api/v1beta1" "github.com/theketchio/ketch/internal/templates" + "github.com/theketchio/ketch/internal/utils/conversions" ) var ( @@ -25,7 +26,7 @@ var ( Parallelism: 2, Completions: 2, Suspend: false, - BackoffLimit: 4, + BackoffLimit: conversions.IntPtr(4), Containers: []ketchv1.Container{ { Name: "test", diff --git a/internal/templates/job/yamls/job.yaml b/internal/templates/job/yamls/job.yaml index 59b4996c..2ccdc1bc 100644 --- a/internal/templates/job/yamls/job.yaml +++ b/internal/templates/job/yamls/job.yaml @@ -12,7 +12,7 @@ spec: {{- if $.Values.job.completions }} completions: {{ $.Values.job.completions }} {{- end }} - {{- if $.Values.job.backoffLimit }} + {{- if not (kindIs "invalid" $.Values.job.backoffLimit) }} backoffLimit: {{ $.Values.job.backoffLimit }} {{- end }} {{- if $.Values.job.suspend }}