Repository: furiko-io/furiko (go)

Issue:
Change MaxRetryAttempts to MaxAttempts
The current naming `maxRetryAttempts` can be confusing, as users may not know if it includes the first attempt by default. We use the terms "attempt" and "retry" quite loosely throughout, and it causes confusion amongst the developers as well.

We should use `maxAttempts` instead. This includes the first attempt. For example, a `maxAttempts` value of 3 would mean a total of 3 attempts (not 4, as per `maxRetryAttempts`).

If not defined, defaults to 1. It cannot be non-positive.

Some API surfaces that I can think of that need to be updated:

- [ ] `maxRetryAttempts` field -> `maxAttempts`
- [ ] `task.retry_index` context variable -> `task.attempt_index` (no renumbering needed)
- [ ] `job.max_retries` context variable -> `job.max_attempts`

Pull request:
feat(execution): Change maxRetryAttempts to maxAttempts
Closes #45.

- Updates all references of maxRetryAttempts to maxAttempts.
- All logic interfacing with field are updated to count the first task as the first attempt.


Patch:
diff --git a/apis/execution/v1alpha1/job_types.go b/apis/execution/v1alpha1/job_types.go
index 8b1a86f..3c62940 100644
--- a/apis/execution/v1alpha1/job_types.go
+++ b/apis/execution/v1alpha1/job_types.go
@@ -138,21 +138,19 @@ type JobTemplateSpec struct {
 	// Describes the tasks to be created for the Job.
 	Task JobTaskSpec `json:"task"`
 
-	// Specifies maximum number of retry attempts for the Job if the job exceeds its
-	// pending timeout or active deadline. Each retry attempt will create a single
-	// task at a time. The controller will create up to MaxRetryAttempts+1 tasks for
-	// the job, before terminating in RetryLimitExceeded or PendingTimeout. Defaults
-	// to 0, which means no retry. Value must be a non-negative integer.
+	// Specifies maximum number of attempts for the Job. Each attempt will create a
+	// single task at a time, and if the task fails, the controller will wait
+	// retryDelaySeconds before creating the next task attempt. Once maxAttempts is
+	// reached, the Job terminates in RetryLimitExceeded. Value must be a positive
+	// integer. Defaults to 1.
 	//
-	// +kubebuilder:validation:Minimum=0
 	// +optional
-	MaxRetryAttempts *int32 `json:"maxRetryAttempts,omitempty"`
+	MaxAttempts *int32 `json:"maxAttempts,omitempty"`
 
 	// Optional duration in seconds to wait between retries. If left empty or zero,
 	// it means no delay (i.e. retry immediately). Value must be a non-negative
 	// integer.
 	//
-	// +kubebuilder:validation:Minimum=0
 	// +optional
 	RetryDelaySeconds *int64 `json:"retryDelaySeconds,omitempty"`
 }
diff --git a/apis/execution/v1alpha1/zz_generated.deepcopy.go b/apis/execution/v1alpha1/zz_generated.deepcopy.go
index 7d777fc..de69105 100644
--- a/apis/execution/v1alpha1/zz_generated.deepcopy.go
+++ b/apis/execution/v1alpha1/zz_generated.deepcopy.go
@@ -494,8 +494,8 @@ func (in *JobTemplate) DeepCopy() *JobTemplate {
 func (in *JobTemplateSpec) DeepCopyInto(out *JobTemplateSpec) {
 	*out = *in
 	in.Task.DeepCopyInto(&out.Task)
-	if in.MaxRetryAttempts != nil {
-		in, out := &in.MaxRetryAttempts, &out.MaxRetryAttempts
+	if in.MaxAttempts != nil {
+		in, out := &in.MaxAttempts, &out.MaxAttempts
 		*out = new(int32)
 		**out = **in
 	}
diff --git a/config/crd/bases/execution.furiko.io_jobconfigs.yaml b/config/crd/bases/execution.furiko.io_jobconfigs.yaml
index 071c3b8..7568a36 100644
--- a/config/crd/bases/execution.furiko.io_jobconfigs.yaml
+++ b/config/crd/bases/execution.furiko.io_jobconfigs.yaml
@@ -199,15 +199,13 @@ spec:
                     spec:
                       description: Specification of the desired behavior of the job.
                       properties:
-                        maxRetryAttempts:
-                          description: Specifies maximum number of retry attempts for the Job if the job exceeds its pending timeout or active deadline. Each retry attempt will create a single task at a time. The controller will create up to MaxRetryAttempts+1 tasks for the job, before terminating in RetryLimitExceeded or PendingTimeout. Defaults to 0, which means no retry. Value must be a non-negative integer.
+                        maxAttempts:
+                          description: Specifies maximum number of attempts for the Job. Each attempt will create a single task at a time, and if the task fails, the controller will wait retryDelaySeconds before creating the next task attempt. Once maxAttempts is reached, the Job terminates in RetryLimitExceeded.
                           format: int32
-                          minimum: 0
                           type: integer
                         retryDelaySeconds:
                           description: Optional duration in seconds to wait between retries. If left empty or zero, it means no delay (i.e. retry immediately). Value must be a non-negative integer.
                           format: int64
-                          minimum: 0
                           type: integer
                         task:
                           description: Describes the tasks to be created for the Job.
diff --git a/config/crd/bases/execution.furiko.io_jobs.yaml b/config/crd/bases/execution.furiko.io_jobs.yaml
index 5d6f688..0510933 100644
--- a/config/crd/bases/execution.furiko.io_jobs.yaml
+++ b/config/crd/bases/execution.furiko.io_jobs.yaml
@@ -81,15 +81,13 @@ spec:
                 template:
                   description: Template specifies how to create the Job.
                   properties:
-                    maxRetryAttempts:
-                      description: Specifies maximum number of retry attempts for the Job if the job exceeds its pending timeout or active deadline. Each retry attempt will create a single task at a time. The controller will create up to MaxRetryAttempts+1 tasks for the job, before terminating in RetryLimitExceeded or PendingTimeout. Defaults to 0, which means no retry. Value must be a non-negative integer.
+                    maxAttempts:
+                      description: Specifies maximum number of attempts for the Job. Each attempt will create a single task at a time, and if the task fails, the controller will wait retryDelaySeconds before creating the next task attempt. Once maxAttempts is reached, the Job terminates in RetryLimitExceeded.
                       format: int32
-                      minimum: 0
                       type: integer
                     retryDelaySeconds:
                       description: Optional duration in seconds to wait between retries. If left empty or zero, it means no delay (i.e. retry immediately). Value must be a non-negative integer.
                       format: int64
-                      minimum: 0
                       type: integer
                     task:
                       description: Describes the tasks to be created for the Job.
diff --git a/config/samples/execution_v1alpha1_job.yaml b/config/samples/execution_v1alpha1_job.yaml
index cd00545..7baaeee 100644
--- a/config/samples/execution_v1alpha1_job.yaml
+++ b/config/samples/execution_v1alpha1_job.yaml
@@ -40,8 +40,8 @@ spec:
       # Maximum duration that a task can be Pending for.
       pendingTimeoutSeconds: 1800
 
-    # Defines how many additional tasks to retry in case of failure.
-    maxRetryAttempts: 2
+    # Defines how many tasks can be created in case of failure.
+    maxAttempts: 3
 
     # Defines how long to wait between retries.
     retryDelaySeconds: 15
diff --git a/pkg/execution/validation/utils.go b/pkg/core/validation/generic.go
similarity index 50%
rename from pkg/execution/validation/utils.go
rename to pkg/core/validation/generic.go
index d8b4c05..5ac8bda 100644
--- a/pkg/execution/validation/utils.go
+++ b/pkg/core/validation/generic.go
@@ -23,11 +23,14 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
-// ToInternalErrorList converts an error into an ErrorList with a single InternalError.
+// ToInternalErrorList converts an error into an ErrorList with a single
+// InternalError if it is not nil, otherwise returns an empty list.
 func ToInternalErrorList(fldPath *field.Path, err error) field.ErrorList {
-	return field.ErrorList{
-		field.InternalError(fldPath, err),
+	allErrs := field.ErrorList{}
+	if err != nil {
+		allErrs = append(allErrs, field.InternalError(fldPath, err))
 	}
+	return allErrs
 }
 
 // ValidateImmutableField validates the new value and the old value are deeply
@@ -48,3 +51,39 @@ func ValidateMaxLength(val string, maxLen int, fldPath *field.Path) field.ErrorL
 	}
 	return allErrs
 }
+
+// ValidateGTE validates the given value must be greater than or equal to the minimum value.
+func ValidateGTE(value, minimum int64, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	if value < minimum {
+		allErrs = append(allErrs, field.Invalid(fldPath, value, fmt.Sprintf("must be greater than or equal to %v", minimum)))
+	}
+	return allErrs
+}
+
+// ValidateGT validates the given value must be greater than the minimum value.
+func ValidateGT(value, minimum int64, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	if value <= minimum {
+		allErrs = append(allErrs, field.Invalid(fldPath, value, fmt.Sprintf("must be greater than %v", minimum)))
+	}
+	return allErrs
+}
+
+// ValidateLTE validates the given value must be less than or equal to the maximum value.
+func ValidateLTE(value, maximum int64, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	if value > maximum {
+		allErrs = append(allErrs, field.Invalid(fldPath, value, fmt.Sprintf("must be less than or equal to %v", maximum)))
+	}
+	return allErrs
+}
+
+// ValidateLT validates the given value must be less than the maximum value.
+func ValidateLT(value, maximum int64, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	if value >= maximum {
+		allErrs = append(allErrs, field.Invalid(fldPath, value, fmt.Sprintf("must be less than %v", maximum)))
+	}
+	return allErrs
+}
diff --git a/pkg/execution/controllers/jobcontroller/reconciler.go b/pkg/execution/controllers/jobcontroller/reconciler.go
index 0a74247..f5a1cd8 100644
--- a/pkg/execution/controllers/jobcontroller/reconciler.go
+++ b/pkg/execution/controllers/jobcontroller/reconciler.go
@@ -168,7 +168,6 @@ func (w *Reconciler) syncJobTasks(
 		return nil, errors.Wrapf(err, "cannot create task manager")
 	}
 
-	// Note that max number of allowed tasks is maxRetries + 1.
 	maxAllowedTasks := jobutil.GetMaxAllowedTasks(rj)
 
 	// Get all tasks for job from cache.
diff --git a/pkg/execution/mutation/mutation.go b/pkg/execution/mutation/mutation.go
index 0aec797..35c211c 100644
--- a/pkg/execution/mutation/mutation.go
+++ b/pkg/execution/mutation/mutation.go
@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/clock"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/utils/pointer"
 
 	"github.com/furiko-io/furiko/apis/execution"
 	"github.com/furiko-io/furiko/apis/execution/v1alpha1"
@@ -137,6 +138,9 @@ func (m *Mutator) MutateJob(rj *v1alpha1.Job) *webhook.Result {
 	if rj.Spec.TTLSecondsAfterFinished == nil {
 		rj.Spec.TTLSecondsAfterFinished = cfg.DefaultTTLSecondsAfterFinished
 	}
+	if rj.Spec.Template.MaxAttempts == nil {
+		rj.Spec.Template.MaxAttempts = pointer.Int32(1)
+	}
 
 	return result
 }
diff --git a/pkg/execution/validation/corev1.go b/pkg/execution/validation/corev1.go
index 35164c2..45fdfa8 100644
--- a/pkg/execution/validation/corev1.go
+++ b/pkg/execution/validation/corev1.go
@@ -26,6 +26,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/pkg/apis/core"
 	apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
+
+	"github.com/furiko-io/furiko/pkg/core/validation"
 )
 
 var (
@@ -52,7 +54,7 @@ func ValidatePodTemplateSpec(spec *corev1.PodTemplateSpec, fieldPath *field.Path
 		err = corev1.SchemeBuilder.AddToScheme(scheme)
 	})
 	if err != nil {
-		return ToInternalErrorList(fieldPath, errors.Wrapf(err, "cannot add to scheme"))
+		return validation.ToInternalErrorList(fieldPath, errors.Wrapf(err, "cannot add to scheme"))
 	}
 
 	// Wrap into a new PodTemplate and perform defaulting.
@@ -65,7 +67,7 @@ func ValidatePodTemplateSpec(spec *corev1.PodTemplateSpec, fieldPath *field.Path
 	// corev1.PodTemplateSpec to core.PodTemplateSpec.
 	var corePodTemplateSpec core.PodTemplateSpec
 	if err := scheme.Convert(&podTemplate.Template, &corePodTemplateSpec, &conversion.Meta{}); err != nil {
-		return ToInternalErrorList(fieldPath, errors.Wrapf(err, "conversion error"))
+		return validation.ToInternalErrorList(fieldPath, errors.Wrapf(err, "conversion error"))
 	}
 
 	// Validate core.PodTemplateSpec using default Kubernetes validators.
diff --git a/pkg/execution/validation/validation.go b/pkg/execution/validation/validation.go
index ed2be1e..5272f10 100644
--- a/pkg/execution/validation/validation.go
+++ b/pkg/execution/validation/validation.go
@@ -32,6 +32,7 @@ import (
 	"github.com/furiko-io/furiko/apis/execution/v1alpha1"
 	"github.com/furiko-io/furiko/pkg/core/options"
 	"github.com/furiko-io/furiko/pkg/core/tzutils"
+	"github.com/furiko-io/furiko/pkg/core/validation"
 	"github.com/furiko-io/furiko/pkg/execution/util/cronparser"
 	executionlister "github.com/furiko-io/furiko/pkg/generated/listers/execution/v1alpha1"
 	"github.com/furiko-io/furiko/pkg/runtime/controllercontext"
@@ -66,7 +67,7 @@ func NewValidator(ctrlContext controllercontext.Context) *Validator {
 // ValidateJobConfig validates a *v1alpha1.JobConfig.
 func (v *Validator) ValidateJobConfig(rjc *v1alpha1.JobConfig) field.ErrorList {
 	allErrs := field.ErrorList{}
-	allErrs = append(allErrs, ValidateMaxLength(rjc.Name, maxJobConfigNameLen, field.NewPath("metadata").Child("name"))...)
+	allErrs = append(allErrs, validation.ValidateMaxLength(rjc.Name, maxJobConfigNameLen, field.NewPath("metadata").Child("name"))...)
 	allErrs = append(allErrs, v.ValidateJobConfigSpec(&rjc.Spec, field.NewPath("spec"))...)
 	return allErrs
 }
@@ -94,7 +95,7 @@ func (v *Validator) ValidateJob(rj *v1alpha1.Job) field.ErrorList {
 // ValidateJobMetadata validates the metadata of a *v1alpha1.Job.
 func (v *Validator) ValidateJobMetadata(metadata *metav1.ObjectMeta, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
-	allErrs = append(allErrs, ValidateMaxLength(metadata.Name, maxJobNameLen, fldPath.Child("name"))...)
+	allErrs = append(allErrs, validation.ValidateMaxLength(metadata.Name, maxJobNameLen, fldPath.Child("name"))...)
 	return allErrs
 }
 
@@ -160,7 +161,7 @@ func (v *Validator) ValidateJobUpdate(oldRj, rj *v1alpha1.Job) field.ErrorList {
 
 	// Once Job is started, not allowed to update startPolicy.
 	if !rj.Status.StartTime.IsZero() {
-		allErrs = append(allErrs, ValidateImmutableField(rj.Spec.StartPolicy, oldRj.Spec.StartPolicy,
+		allErrs = append(allErrs, validation.ValidateImmutableField(rj.Spec.StartPolicy, oldRj.Spec.StartPolicy,
 			field.NewPath("spec.startPolicy"), "cannot update startPolicy once Job is started")...)
 	}
 
@@ -332,7 +333,7 @@ func (v *Validator) ValidateJobSpecUpdate(oldSpec, spec *v1alpha1.JobSpec, fldPa
 func (v *Validator) ValidateJobTemplateSpecImmutable(oldTemplate, template *v1alpha1.JobTemplateSpec, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 	allErrs = append(allErrs, apivalidation.ValidateImmutableField(template.Task, oldTemplate.Task, fldPath.Child("task"))...)
-	allErrs = append(allErrs, apivalidation.ValidateImmutableField(template.MaxRetryAttempts, oldTemplate.MaxRetryAttempts, fldPath.Child("maxRetryAttempts"))...)
+	allErrs = append(allErrs, apivalidation.ValidateImmutableField(template.MaxAttempts, oldTemplate.MaxAttempts, fldPath.Child("maxAttempts"))...)
 	allErrs = append(allErrs, apivalidation.ValidateImmutableField(template.RetryDelaySeconds, oldTemplate.RetryDelaySeconds, fldPath.Child("retryDelaySeconds"))...)
 	return allErrs
 }
@@ -381,8 +382,8 @@ func (v *Validator) ValidateStartPolicySpec(spec *v1alpha1.StartPolicySpec, fldP
 func (v *Validator) ValidateJobTemplateSpec(template *v1alpha1.JobTemplateSpec, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 	allErrs = append(allErrs, v.ValidateJobTaskSpec(&template.Task, fldPath.Child("task"))...)
-	if template.MaxRetryAttempts != nil {
-		allErrs = append(allErrs, v.ValidateMaxRetryAttempts(*template.MaxRetryAttempts, fldPath.Child("maxRetryAttempts"))...)
+	if template.MaxAttempts != nil {
+		allErrs = append(allErrs, v.ValidateMaxRetryAttempts(*template.MaxAttempts, fldPath.Child("maxAttempts"))...)
 	}
 	if template.RetryDelaySeconds != nil {
 		allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(*template.RetryDelaySeconds, fldPath.Child("retryDelaySeconds"))...)
@@ -392,12 +393,13 @@ func (v *Validator) ValidateJobTemplateSpec(template *v1alpha1.JobTemplateSpec,
 
 func (v *Validator) ValidateMaxRetryAttempts(attempts int32, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
-	allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(attempts), fldPath)...)
 
-	// Set a limit of 49 retries (excluding first attempt) arbitrarily.
-	if attempts > 49 {
-		allErrs = append(allErrs, field.Invalid(fldPath, attempts, "maxRetryAttempts must be at most 49"))
-	}
+	// Must be greater than 0.
+	allErrs = append(allErrs, validation.ValidateGT(int64(attempts), 0, fldPath)...)
+
+	// Set an arbitrary limit of 50 attempts.
+	// TODO(irvinlim): Support configuring this value
+	allErrs = append(allErrs, validation.ValidateLTE(int64(attempts), 50, fldPath)...)
 
 	return allErrs
 }
diff --git a/pkg/execution/variablecontext/provider.go b/pkg/execution/variablecontext/provider.go
index f461075..cd5fdf1 100644
--- a/pkg/execution/variablecontext/provider.go
+++ b/pkg/execution/variablecontext/provider.go
@@ -81,8 +81,8 @@ func (c *defaultProvider) MakeVariablesFromJob(rj *execution.Job) map[string]str
 		"job.type":      string(rj.Spec.Type),
 	}
 
-	if maxRetries := rj.Spec.Template.MaxRetryAttempts; maxRetries != nil {
-		subs["job.max_retries"] = strconv.Itoa(int(*maxRetries))
+	if maxAttempts := rj.Spec.Template.MaxAttempts; maxAttempts != nil {
+		subs["job.max_attempts"] = strconv.Itoa(int(*maxAttempts))
 	}
 
 	return subs
diff --git a/pkg/utils/execution/job/task_utils.go b/pkg/utils/execution/job/task_utils.go
index c959f3b..2af2168 100644
--- a/pkg/utils/execution/job/task_utils.go
+++ b/pkg/utils/execution/job/task_utils.go
@@ -49,13 +49,13 @@ func IsTaskFinished(task jobtasks.Task) bool {
 
 // GetMaxAllowedTasks returns the maximum number of allowed tasks that a Job can have.
 func GetMaxAllowedTasks(rj *execution.Job) int64 {
-	var maxRetries int64
+	var maxAttempts int64 = 1
 	if template := rj.Spec.Template; template != nil {
-		if rj.Spec.Template.MaxRetryAttempts != nil {
-			maxRetries = int64(*rj.Spec.Template.MaxRetryAttempts)
+		if rj.Spec.Template.MaxAttempts != nil {
+			maxAttempts = int64(*rj.Spec.Template.MaxAttempts)
 		}
 	}
-	return maxRetries + 1
+	return maxAttempts
 }
 
 // GetNextAllowedRetry checks if we can create a new task, and if so, returns


Test patch:
diff --git a/pkg/core/validation/generic_test.go b/pkg/core/validation/generic_test.go
new file mode 100644
index 0000000..8bd6c51
--- /dev/null
+++ b/pkg/core/validation/generic_test.go
@@ -0,0 +1,242 @@
+/*
+ * Copyright 2022 The Furiko Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package validation
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+)
+
+var (
+	fldPath = field.NewPath("spec")
+)
+
+func TestToInternalErrorList(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+		want field.ErrorList
+	}{
+		{
+			name: "nil error",
+		},
+		{
+			name: "non-nil error",
+			err:  assert.AnError,
+			want: field.ErrorList{
+				field.InternalError(fldPath, assert.AnError),
+			},
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			got := ToInternalErrorList(fldPath, tt.err)
+			if !cmp.Equal(got, tt.want, cmpopts.EquateEmpty()) {
+				t.Errorf("ToInternalErrorList() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestValidateImmutableField(t *testing.T) {
+	tests := []struct {
+		name   string
+		newVal interface{}
+		oldVal interface{}
+		msg    string
+		want   field.ErrorList
+	}{
+		{
+			name: "nil values",
+		},
+		{
+			name:   "different integer values",
+			newVal: 1,
+			oldVal: 0,
+			msg:    "custom error message",
+			want: field.ErrorList{
+				field.Invalid(fldPath, 1, "custom error message"),
+			},
+		},
+		{
+			name:   "equal integer values",
+			newVal: 1,
+			oldVal: 1,
+			msg:    "custom error message",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ValidateImmutableField(tt.newVal, tt.oldVal, fldPath, tt.msg)
+			if !cmp.Equal(got, tt.want, cmpopts.EquateEmpty()) {
+				t.Errorf("ValidateImmutableField(%v, %v, %v, %v) = %v, want %v",
+					tt.newVal, tt.oldVal, fldPath, tt.msg, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestValidateGTE(t *testing.T) {
+	tests := []struct {
+		name    string
+		value   int64
+		minimum int64
+		want    field.ErrorList
+	}{
+		{
+			name:    "greater than",
+			value:   11,
+			minimum: 10,
+		},
+		{
+			name:    "equal to",
+			value:   10,
+			minimum: 10,
+		},
+		{
+			name:    "less than",
+			value:   9,
+			minimum: 10,
+			want: field.ErrorList{
+				field.Invalid(fldPath, int64(9), "must be greater than or equal to 10"),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ValidateGTE(tt.value, tt.minimum, fldPath); !cmp.Equal(got, tt.want, cmpopts.EquateEmpty()) {
+				t.Errorf("ValidateGTE(%v, %v, %v) = %v, want %v", tt.value, tt.minimum, fldPath, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestValidateGT(t *testing.T) {
+	tests := []struct {
+		name    string
+		value   int64
+		minimum int64
+		want    field.ErrorList
+	}{
+		{
+			name:    "greater than",
+			value:   11,
+			minimum: 10,
+		},
+		{
+			name:    "equal to",
+			value:   10,
+			minimum: 10,
+			want: field.ErrorList{
+				field.Invalid(fldPath, int64(10), "must be greater than 10"),
+			},
+		},
+		{
+			name:    "less than",
+			value:   9,
+			minimum: 10,
+			want: field.ErrorList{
+				field.Invalid(fldPath, int64(9), "must be greater than 10"),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ValidateGT(tt.value, tt.minimum, fldPath); !cmp.Equal(got, tt.want, cmpopts.EquateEmpty()) {
+				t.Errorf("ValidateGT(%v, %v, %v) = %v, want %v", tt.value, tt.minimum, fldPath, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestValidateLTE(t *testing.T) {
+	tests := []struct {
+		name    string
+		value   int64
+		minimum int64
+		want    field.ErrorList
+	}{
+		{
+			name:    "greater than",
+			value:   11,
+			minimum: 10,
+			want: field.ErrorList{
+				field.Invalid(fldPath, int64(11), "must be less than or equal to 10"),
+			},
+		},
+		{
+			name:    "equal to",
+			value:   10,
+			minimum: 10,
+		},
+		{
+			name:    "less than",
+			value:   9,
+			minimum: 10,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ValidateLTE(tt.value, tt.minimum, fldPath); !cmp.Equal(got, tt.want, cmpopts.EquateEmpty()) {
+				t.Errorf("ValidateLTE(%v, %v, %v) = %v, want %v", tt.value, tt.minimum, fldPath, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestValidateLT(t *testing.T) {
+	tests := []struct {
+		name    string
+		value   int64
+		minimum int64
+		want    field.ErrorList
+	}{
+		{
+			name:    "greater than",
+			value:   11,
+			minimum: 10,
+			want: field.ErrorList{
+				field.Invalid(fldPath, int64(11), "must be less than 10"),
+			},
+		},
+		{
+			name:    "equal to",
+			value:   10,
+			minimum: 10,
+			want: field.ErrorList{
+				field.Invalid(fldPath, int64(10), "must be less than 10"),
+			},
+		},
+		{
+			name:    "less than",
+			value:   9,
+			minimum: 10,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ValidateLT(tt.value, tt.minimum, fldPath); !cmp.Equal(got, tt.want, cmpopts.EquateEmpty()) {
+				t.Errorf("ValidateLT(%v, %v, %v) = %v, want %v", tt.value, tt.minimum, fldPath, got, tt.want)
+			}
+		})
+	}
+}
diff --git a/pkg/execution/mutation/mutation_test.go b/pkg/execution/mutation/mutation_test.go
index bee898e..40a9071 100644
--- a/pkg/execution/mutation/mutation_test.go
+++ b/pkg/execution/mutation/mutation_test.go
@@ -595,14 +595,23 @@ func TestMutator_MutateJob(t *testing.T) {
 			rj: &v1alpha1.Job{
 				ObjectMeta: objectMetaJob,
 				Spec: v1alpha1.JobSpec{
-					Template: &jobTemplateSpecBasic.Spec,
+					Template: &v1alpha1.JobTemplateSpec{
+						Task: v1alpha1.JobTaskSpec{
+							Template: podTemplateSpecBasic,
+						},
+					},
 				},
 			},
 			want: &v1alpha1.Job{
 				ObjectMeta: objectMetaJob,
 				Spec: v1alpha1.JobSpec{
-					Type:                    v1alpha1.JobTypeAdhoc,
-					Template:                &jobTemplateSpecBasic.Spec,
+					Type: v1alpha1.JobTypeAdhoc,
+					Template: &v1alpha1.JobTemplateSpec{
+						Task: v1alpha1.JobTaskSpec{
+							Template: podTemplateSpecBasic,
+						},
+						MaxAttempts: pointer.Int32(1),
+					},
 					TTLSecondsAfterFinished: config.DefaultJobExecutionConfig.DefaultTTLSecondsAfterFinished,
 				},
 			},
@@ -612,8 +621,13 @@ func TestMutator_MutateJob(t *testing.T) {
 			rj: &v1alpha1.Job{
 				ObjectMeta: objectMetaJob,
 				Spec: v1alpha1.JobSpec{
-					Type:                    v1alpha1.JobTypeAdhoc,
-					Template:                &jobTemplateSpecBasic.Spec,
+					Type: v1alpha1.JobTypeAdhoc,
+					Template: &v1alpha1.JobTemplateSpec{
+						Task: v1alpha1.JobTaskSpec{
+							Template: podTemplateSpecBasic,
+						},
+						MaxAttempts: pointer.Int32(1),
+					},
 					TTLSecondsAfterFinished: config.DefaultJobExecutionConfig.DefaultTTLSecondsAfterFinished,
 				},
 			},
diff --git a/pkg/execution/validation/validation_test.go b/pkg/execution/validation/validation_test.go
index 883d33a..1c1c5c2 100644
--- a/pkg/execution/validation/validation_test.go
+++ b/pkg/execution/validation/validation_test.go
@@ -95,7 +95,7 @@ var (
 				Template:              podTemplateSpecBasic,
 				PendingTimeoutSeconds: pointer.Int64(1800),
 			},
-			MaxRetryAttempts:  pointer.Int32(5),
+			MaxAttempts:       pointer.Int32(5),
 			RetryDelaySeconds: pointer.Int64(60),
 		},
 	}
@@ -107,7 +107,7 @@ var (
 				Template:              podTemplateSpecBasic,
 				PendingTimeoutSeconds: pointer.Int64(3600),
 			},
-			MaxRetryAttempts:  jobTemplateSpecBasic.Spec.MaxRetryAttempts,
+			MaxAttempts:       jobTemplateSpecBasic.Spec.MaxAttempts,
 			RetryDelaySeconds: jobTemplateSpecBasic.Spec.RetryDelaySeconds,
 		},
 	}
@@ -116,7 +116,7 @@ var (
 		ObjectMeta: jobTemplateSpecBasic.ObjectMeta,
 		Spec: v1alpha1.JobTemplateSpec{
 			Task:              jobTemplateSpecBasic.Spec.Task,
-			MaxRetryAttempts:  pointer.Int32(10),
+			MaxAttempts:       pointer.Int32(10),
 			RetryDelaySeconds: jobTemplateSpecBasic.Spec.RetryDelaySeconds,
 		},
 	}
@@ -125,7 +125,7 @@ var (
 		ObjectMeta: jobTemplateSpecBasic.ObjectMeta,
 		Spec: v1alpha1.JobTemplateSpec{
 			Task:              jobTemplateSpecBasic.Spec.Task,
-			MaxRetryAttempts:  pointer.Int32(100),
+			MaxAttempts:       pointer.Int32(100),
 			RetryDelaySeconds: jobTemplateSpecBasic.Spec.RetryDelaySeconds,
 		},
 	}
@@ -339,7 +339,7 @@ func TestValidateJobConfig(t *testing.T) {
 			wantErr: "spec.schedule.cron.timezone: Invalid value: \"Invalid/Time/Zone\": cannot parse timezone",
 		},
 		{
-			name: "maxRetryAttempts too large",
+			name: "maxAttempts too large",
 			rjc: &v1alpha1.JobConfig{
 				Spec: v1alpha1.JobConfigSpec{
 					Template:    jobTemplateSpecTooManyRetries,
@@ -347,7 +347,7 @@ func TestValidateJobConfig(t *testing.T) {
 					Schedule:    &scheduleSpecBasic,
 				},
 			},
-			wantErr: "spec.template.spec.maxRetryAttempts: Invalid value: 100: maxRetryAttempts must be at most 49",
+			wantErr: "spec.template.spec.maxAttempts: Invalid value: 100: must be less than or equal to 50",
 		},
 		{
 			name: "invalid options",
@@ -519,7 +519,7 @@ func TestValidateJob(t *testing.T) {
 			wantErr: "spec.ttlSecondsAfterFinished: Invalid value: -300",
 		},
 		{
-			name: "maxRetryAttempts too large",
+			name: "maxAttempts too large",
 			rj: &v1alpha1.Job{
 				ObjectMeta: objectMetaJob,
 				Spec: v1alpha1.JobSpec{
@@ -527,7 +527,7 @@ func TestValidateJob(t *testing.T) {
 					Template: &jobTemplateSpecTooManyRetries.Spec,
 				},
 			},
-			wantErr: "spec.template.maxRetryAttempts: Invalid value: 100: maxRetryAttempts must be at most 49",
+			wantErr: "spec.template.maxAttempts: Invalid value: 100: must be less than or equal to 50",
 		},
 		{
 			name: "invalid pod template",
@@ -720,7 +720,7 @@ func TestValidateJobUpdate(t *testing.T) {
 			wantErr: "spec.template.task: Invalid value",
 		},
 		{
-			name: "immutable field maxRetryAttempts",
+			name: "immutable field maxAttempts",
 			oldRj: &v1alpha1.Job{
 				ObjectMeta: objectMetaJob,
 				Spec: v1alpha1.JobSpec{
@@ -735,7 +735,7 @@ func TestValidateJobUpdate(t *testing.T) {
 					Template: &jobTemplateSpecMoreRetries.Spec,
 				},
 			},
-			wantErr: "spec.template.maxRetryAttempts: Invalid value: 10: field is immutable",
+			wantErr: "spec.template.maxAttempts: Invalid value: 10: field is immutable",
 		},
 		{
 			name: "can set KillTimestamp",
diff --git a/pkg/utils/execution/job/condition_test.go b/pkg/utils/execution/job/condition_test.go
index 15e09f0..87f26f5 100644
--- a/pkg/utils/execution/job/condition_test.go
+++ b/pkg/utils/execution/job/condition_test.go
@@ -309,7 +309,7 @@ func TestGetCondition(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1"),
@@ -374,7 +374,7 @@ func TestGetCondition(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1"),
@@ -442,7 +442,7 @@ func TestGetCondition(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1", "task2"),
@@ -482,7 +482,7 @@ func TestGetCondition(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1", "task2"),
@@ -541,7 +541,7 @@ func TestGetCondition(t *testing.T) {
 					Spec: execution.JobSpec{
 						KillTimestamp: &killTime,
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: execution.JobStatus{
@@ -585,7 +585,7 @@ func TestGetCondition(t *testing.T) {
 					Spec: execution.JobSpec{
 						KillTimestamp: &killTime,
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: execution.JobStatus{
@@ -662,7 +662,7 @@ func TestGetCondition(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1"),
@@ -683,7 +683,7 @@ func TestGetCondition(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: execution.JobStatus{
@@ -720,7 +720,7 @@ func TestGetCondition(t *testing.T) {
 					Spec: execution.JobSpec{
 						KillTimestamp: &killTime,
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1"),
diff --git a/pkg/utils/execution/job/job_utils_test.go b/pkg/utils/execution/job/job_utils_test.go
index 2d1ed9e..150e40b 100644
--- a/pkg/utils/execution/job/job_utils_test.go
+++ b/pkg/utils/execution/job/job_utils_test.go
@@ -48,7 +48,7 @@ func TestAllowedToCreateNewTask(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1"),
@@ -70,7 +70,7 @@ func TestAllowedToCreateNewTask(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1"),
@@ -96,7 +96,7 @@ func TestAllowedToCreateNewTask(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1"),
@@ -124,7 +124,7 @@ func TestAllowedToCreateNewTask(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1"),
@@ -176,7 +176,7 @@ func TestAllowedToCreateNewTask(t *testing.T) {
 					Spec: execution.JobSpec{
 						KillTimestamp: &killTime,
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &one,
+							MaxAttempts: &two,
 						},
 					},
 					Status: createTaskRefsStatus("task1"),
@@ -204,7 +204,7 @@ func TestAllowedToCreateNewTask(t *testing.T) {
 				rj: &execution.Job{
 					Spec: execution.JobSpec{
 						Template: &execution.JobTemplateSpec{
-							MaxRetryAttempts: &two,
+							MaxAttempts: &three,
 						},
 					},
 					Status: createTaskRefsStatus("task1", "task2"),
diff --git a/pkg/utils/execution/job/task_test.go b/pkg/utils/execution/job/task_test.go
index 565e12d..53929e2 100644
--- a/pkg/utils/execution/job/task_test.go
+++ b/pkg/utils/execution/job/task_test.go
@@ -43,8 +43,8 @@ var (
 	finishTime       = metav1.NewTime(stdFinishTime)
 	finishTime2      = metav1.NewTime(stdFinishTime.Add(time.Minute))
 	killTime         = metav1.NewTime(stdKillTime)
-	one              = int32(1)
 	two              = int32(2)
+	three            = int32(3)
 )
 
 type stubTask struct {
diff --git a/pkg/utils/execution/job/task_utils_test.go b/pkg/utils/execution/job/task_utils_test.go
index 93dfe12..a078cf8 100644
--- a/pkg/utils/execution/job/task_utils_test.go
+++ b/pkg/utils/execution/job/task_utils_test.go
@@ -38,7 +38,7 @@ func TestGetNextAllowedRetry(t *testing.T) {
 			rj: &execution.Job{
 				Spec: execution.JobSpec{
 					Template: &execution.JobTemplateSpec{
-						MaxRetryAttempts: &two,
+						MaxAttempts: &three,
 					},
 				},
 			},
@@ -49,7 +49,7 @@ func TestGetNextAllowedRetry(t *testing.T) {
 			rj: &execution.Job{
 				Spec: execution.JobSpec{
 					Template: &execution.JobTemplateSpec{
-						MaxRetryAttempts:  &two,
+						MaxAttempts:       &three,
 						RetryDelaySeconds: pointer.Int64(30),
 					},
 				},
@@ -82,7 +82,7 @@ func TestGetNextAllowedRetry(t *testing.T) {
 			rj: &execution.Job{
 				Spec: execution.JobSpec{
 					Template: &execution.JobTemplateSpec{
-						MaxRetryAttempts: &two,
+						MaxAttempts: &three,
 					},
 				},
 				Status: execution.JobStatus{
@@ -107,7 +107,7 @@ func TestGetNextAllowedRetry(t *testing.T) {
 			rj: &execution.Job{
 				Spec: execution.JobSpec{
 					Template: &execution.JobTemplateSpec{
-						MaxRetryAttempts:  &two,
+						MaxAttempts:       &three,
 						RetryDelaySeconds: pointer.Int64(30),
 					},
 				},
@@ -133,7 +133,7 @@ func TestGetNextAllowedRetry(t *testing.T) {
 			rj: &execution.Job{
 				Spec: execution.JobSpec{
 					Template: &execution.JobTemplateSpec{
-						MaxRetryAttempts:  &two,
+						MaxAttempts:       &three,
 						RetryDelaySeconds: pointer.Int64(30),
 					},
 				},
@@ -158,7 +158,7 @@ func TestGetNextAllowedRetry(t *testing.T) {
 			rj: &execution.Job{
 				Spec: execution.JobSpec{
 					Template: &execution.JobTemplateSpec{
-						MaxRetryAttempts:  &two,
+						MaxAttempts:       &three,
 						RetryDelaySeconds: pointer.Int64(30),
 					},
 					KillTimestamp: &killTime,
@@ -185,7 +185,7 @@ func TestGetNextAllowedRetry(t *testing.T) {
 			rj: &execution.Job{
 				Spec: execution.JobSpec{
 					Template: &execution.JobTemplateSpec{
-						MaxRetryAttempts:  &one,
+						MaxAttempts:       &two,
 						RetryDelaySeconds: pointer.Int64(30),
 					},
 				},
@@ -220,7 +220,7 @@ func TestGetNextAllowedRetry(t *testing.T) {
 			rj: &execution.Job{
 				Spec: execution.JobSpec{
 					Template: &execution.JobTemplateSpec{
-						MaxRetryAttempts:  &one,
+						MaxAttempts:       &two,
 						RetryDelaySeconds: pointer.Int64(30),
 					},
 				},