Skip to content

Commit

Permalink
feat: Change maxRetryAttempts to maxAttempts
Browse files Browse the repository at this point in the history
  • Loading branch information
irvinlim committed Apr 15, 2022
1 parent e24be62 commit 6e0e6c7
Show file tree
Hide file tree
Showing 19 changed files with 377 additions and 82 deletions.
14 changes: 6 additions & 8 deletions apis/execution/v1alpha1/job_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down
4 changes: 2 additions & 2 deletions apis/execution/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 2 additions & 4 deletions config/crd/bases/execution.furiko.io_jobconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 2 additions & 4 deletions config/crd/bases/execution.furiko.io_jobs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions config/samples/execution_v1alpha1_job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
241 changes: 241 additions & 0 deletions pkg/core/validation/generic_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
/*
* 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) {
if got := ValidateImmutableField(tt.newVal, tt.oldVal, fldPath, tt.msg); !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)
}
})
}
}
1 change: 0 additions & 1 deletion pkg/execution/controllers/jobcontroller/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 6e0e6c7

Please sign in to comment.