From 2f1cf4ee6a90e5f0911ac8d8fcee0d751fc97fa8 Mon Sep 17 00:00:00 2001 From: Suraj Deshmukh Date: Fri, 17 Nov 2017 18:13:16 +0530 Subject: [PATCH] fix(jobspec): make restartPolicy as OnFailure (#347) * fix(jobspec): make restartPolicy as OnFailure Job cannot have a pod with `restartPolicy` as `Always`, but it was being defaulted to `Always` in this commit changing it to `OnFailure`. --- docs/examples/jobs/README.md | 18 ++++ docs/file-reference.md | 4 +- pkg/spec/job.go | 8 ++ pkg/spec/job_test.go | 155 +++++++++++++++++++++++++++++++++++ 4 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 docs/examples/jobs/README.md diff --git a/docs/examples/jobs/README.md b/docs/examples/jobs/README.md new file mode 100644 index 000000000..4fe994462 --- /dev/null +++ b/docs/examples/jobs/README.md @@ -0,0 +1,18 @@ +# Job + +Now you can add support for Kubernetes controller type Job. All you need to do is provide a +root level field called `controller` like this: + +```yaml +name: pival +controller: job +containers: +... +``` + +All of the information you provide for a normal controller will be same. By default the value +of the `restartPolicy` will be `OnFailure`. + + +At root level the fields that are available are from `PodSpec` and `JobSpec`. For example of +job definition in kedge look at file [`job.yaml`](job.yaml). \ No newline at end of file diff --git a/docs/file-reference.md b/docs/file-reference.md index a6cc70e1c..0367f7928 100644 --- a/docs/file-reference.md +++ b/docs/file-reference.md @@ -991,6 +991,8 @@ restartPolicy: Never parallelism: 3 ``` +**Note**: If no `restartPolicy` is provided it defaults to `OnFailure`. + ## Deployment Config ```yaml @@ -1005,4 +1007,4 @@ services: ports: - port: 8080 targetPort: 80 -``` +``` \ No newline at end of file diff --git a/pkg/spec/job.go b/pkg/spec/job.go index 3c487a775..998c90442 100644 --- a/pkg/spec/job.go +++ b/pkg/spec/job.go @@ -46,6 +46,10 @@ func (job *JobSpecMod) Fix() error { job.ControllerFields.ObjectMeta.Labels = addKeyValueToMap(appLabelKey, job.ControllerFields.Name, job.ControllerFields.ObjectMeta.Labels) + // if RestartPolicy is not set by user default it to 'OnFailure' + if job.RestartPolicy == "" { + job.RestartPolicy = api_v1.RestartPolicyOnFailure + } return nil } @@ -129,6 +133,10 @@ func (job *JobSpecMod) Validate() error { return errors.Wrap(err, "unable to validate controller fields") } + if job.RestartPolicy == api_v1.RestartPolicyAlways { + return fmt.Errorf("the Job %q is invalid: restartPolicy: unsupported value: \"Always\": supported values: OnFailure, Never", job.Name) + } + return nil } diff --git a/pkg/spec/job_test.go b/pkg/spec/job_test.go index 29c4763b9..55063096a 100644 --- a/pkg/spec/job_test.go +++ b/pkg/spec/job_test.go @@ -127,3 +127,158 @@ func TestJobSpecMod_CreateKubernetesController(t *testing.T) { }) } } + +func TestJobValidate(t *testing.T) { + tests := []struct { + name string + input *JobSpecMod + success bool + }{ + { + name: "Set restart policy as failure", + input: &JobSpecMod{ + ControllerFields: ControllerFields{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "testJob", + }, + Controller: "job", + PodSpecMod: PodSpecMod{ + PodSpec: api_v1.PodSpec{ + Containers: []api_v1.Container{ + { + Name: "testContainer", + Image: "testImage", + }, + }, + RestartPolicy: api_v1.RestartPolicyAlways, + }, + }, + }, + }, + success: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if err := test.input.Validate(); err != nil && test.success == true { + // test failing condition + t.Fatalf("test expected to pass, but failed with error: %v", err) + } else if err != nil && test.success == false { + // test passing condition + t.Logf("test expected to fail, failed with error: %v", err) + } else if err == nil && test.success == true { + // test passing condition + t.Logf("test passed") + } else if err == nil && test.success == false { + // test failing condition + t.Fatalf("test expected to fail, but passed with error") + } + }) + } +} + +func TestJobFix(t *testing.T) { + tests := []struct { + name string + input *JobSpecMod + output *JobSpecMod + success bool + }{ + { + name: "no restartPolicy given", + input: &JobSpecMod{ + ControllerFields: ControllerFields{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "testJob", + }, + Controller: "job", + PodSpecMod: PodSpecMod{ + PodSpec: api_v1.PodSpec{ + Containers: []api_v1.Container{ + { + Name: "testContainer", + Image: "testImage", + }, + }, + }, + }, + }, + }, + output: &JobSpecMod{ + ControllerFields: ControllerFields{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "testJob", + Labels: map[string]string{"app": "testJob"}, + }, + Controller: "job", + PodSpecMod: PodSpecMod{ + PodSpec: api_v1.PodSpec{ + Containers: []api_v1.Container{ + { + Name: "testContainer", + Image: "testImage", + }, + }, + RestartPolicy: "OnFailure", + }, + }, + }, + }, + success: true, + }, + { + name: "fail condition on two containers without name given", + input: &JobSpecMod{ + ControllerFields: ControllerFields{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "testJob", + }, + Controller: "job", + PodSpecMod: PodSpecMod{ + Containers: []Container{ + { + Container: api_v1.Container{ + Image: "testImage", + }, + }, + { + Container: api_v1.Container{ + Image: "testSideCarImage", + }, + }, + }, + }, + }, + }, + success: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + + err := test.input.Fix() + + switch test.success { + case true: + if err != nil { + t.Fatalf("Expected test to pass but got an error: %v", err) + } else { + t.Logf("test passed for input: %s", prettyPrintObjects(test.input)) + } + case false: + if err == nil { + t.Fatalf("For the input -\n%v\nexpected test to fail, but test passed", prettyPrintObjects(test.input)) + } else { + t.Logf("failed with error: %v", err) + return + } + } + + if !reflect.DeepEqual(test.input, test.output) { + t.Fatalf("Expected Validated Kubernetes JobSpecMod to be -\n%v\nBut got -\n%v", prettyPrintObjects(test.output), prettyPrintObjects(test.input)) + } + }) + } +}