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

@@ -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).

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
-```
+```

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
 }
 

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))
+			}
+		})
+	}
+}