fix the misleading error msg when updating job.status.startTime

Signed-off-by: zhzhuang-zju <m17799853869@163.com>
This commit is contained in:
zhzhuang-zju 2026-03-04 10:52:59 +08:00
parent 028015267e
commit 8cd55303b1
3 changed files with 103 additions and 4 deletions

View file

@ -741,7 +741,7 @@ func ValidateJobStatusUpdate(job, oldJob *batch.Job, opts JobStatusValidationOpt
// Note that we check `oldJob.Status.StartTime != nil` to allow transitioning from
// startTime = nil to startTime != nil for unsuspended jobs, which is a desired transition.
if oldJob.Status.StartTime != nil && !ptr.Equal(oldJob.Status.StartTime, job.Status.StartTime) && !ptr.Deref(job.Spec.Suspend, false) {
allErrs = append(allErrs, field.Required(statusFld.Child("startTime"), "startTime cannot be removed for unsuspended job"))
allErrs = append(allErrs, field.Invalid(statusFld.Child("startTime"), job.Status.StartTime, "field is immutable for unsuspended job once set"))
}
}
if isJobSuccessCriteriaMet(oldJob) && !isJobSuccessCriteriaMet(job) {

View file

@ -21,6 +21,7 @@ import (
"fmt"
"strings"
"testing"
"time"
_ "time/tzdata"
"github.com/google/go-cmp/cmp"
@ -2583,12 +2584,16 @@ func TestValidateJobUpdate(t *testing.T) {
}
func TestValidateJobUpdateStatus(t *testing.T) {
now := time.Now()
cases := map[string]struct {
opts JobStatusValidationOptions
old batch.Job
update batch.Job
wantErrs field.ErrorList
cmpopts cmp.Options
}{
"valid": {
old: batch.Job{
@ -2680,6 +2685,7 @@ func TestValidateJobUpdateStatus(t *testing.T) {
{Type: field.ErrorTypeInvalid, Field: "status.ready"},
{Type: field.ErrorTypeInvalid, Field: "status.terminating"},
},
cmpopts: cmp.Options{ignoreErrValueDetail},
},
"empty and duplicated uncounted pods": {
old: batch.Job{
@ -2709,12 +2715,105 @@ func TestValidateJobUpdateStatus(t *testing.T) {
{Type: field.ErrorTypeDuplicate, Field: "status.uncountedTerminatedPods.failed[3]"},
{Type: field.ErrorTypeInvalid, Field: "status.uncountedTerminatedPods.failed[4]"},
},
cmpopts: cmp.Options{ignoreErrValueDetail},
},
"immutable startTime for unsuspended job: with non-nil startTime": {
opts: JobStatusValidationOptions{
RejectStartTimeUpdateForUnsuspendedJob: true,
},
old: batch.Job{
ObjectMeta: metav1.ObjectMeta{
Name: "abc",
Namespace: metav1.NamespaceDefault,
ResourceVersion: "1",
},
Spec: batch.JobSpec{
Suspend: ptr.To(false),
},
Status: batch.JobStatus{
StartTime: &metav1.Time{
Time: now,
},
Active: 1,
},
},
update: batch.Job{
ObjectMeta: metav1.ObjectMeta{
Name: "abc",
Namespace: metav1.NamespaceDefault,
ResourceVersion: "1",
},
Spec: batch.JobSpec{
Suspend: ptr.To(false),
},
Status: batch.JobStatus{
StartTime: &metav1.Time{
Time: now.Add(time.Second), // Attempt to change startTime
},
Active: 1,
},
},
wantErrs: field.ErrorList{
{
Type: field.ErrorTypeInvalid,
Field: "status.startTime",
BadValue: &metav1.Time{
Time: now.Add(time.Second),
},
Detail: "field is immutable for unsuspended job once set",
},
},
cmpopts: cmp.Options{cmpopts.IgnoreFields(field.Error{}, "Origin")},
},
"immutable startTime for unsuspended job: with nil startTime": {
opts: JobStatusValidationOptions{
RejectStartTimeUpdateForUnsuspendedJob: true,
},
old: batch.Job{
ObjectMeta: metav1.ObjectMeta{
Name: "abc",
Namespace: metav1.NamespaceDefault,
ResourceVersion: "1",
},
Spec: batch.JobSpec{
Suspend: ptr.To(false),
},
Status: batch.JobStatus{
StartTime: &metav1.Time{
Time: now,
},
Active: 1,
},
},
update: batch.Job{
ObjectMeta: metav1.ObjectMeta{
Name: "abc",
Namespace: metav1.NamespaceDefault,
ResourceVersion: "1",
},
Spec: batch.JobSpec{
Suspend: ptr.To(false),
},
Status: batch.JobStatus{
StartTime: nil,
Active: 1,
},
},
wantErrs: field.ErrorList{
{
Type: field.ErrorTypeInvalid,
Field: "status.startTime",
BadValue: (*metav1.Time)(nil),
Detail: "field is immutable for unsuspended job once set",
},
},
cmpopts: cmp.Options{cmpopts.IgnoreFields(field.Error{}, "Origin")},
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
errs := ValidateJobUpdateStatus(&tc.update, &tc.old, tc.opts)
if diff := cmp.Diff(tc.wantErrs, errs, ignoreErrValueDetail); diff != "" {
if diff := cmp.Diff(tc.wantErrs, errs, tc.cmpopts); diff != "" {
t.Errorf("Unexpected errors (-want,+got):\n%s", diff)
}
})

View file

@ -2844,7 +2844,7 @@ func TestStatusStrategy_ValidateUpdate(t *testing.T) {
},
},
wantErrs: field.ErrorList{
{Type: field.ErrorTypeRequired, Field: "status.startTime"},
{Type: field.ErrorTypeInvalid, Field: "status.startTime"},
},
},
"verify startTime cannot be updated for unsuspended job": {
@ -2862,7 +2862,7 @@ func TestStatusStrategy_ValidateUpdate(t *testing.T) {
},
},
wantErrs: field.ErrorList{
{Type: field.ErrorTypeRequired, Field: "status.startTime"},
{Type: field.ErrorTypeInvalid, Field: "status.startTime"},
},
},
"verify startTime can be updated when resuming job (JobSuspended: True -> False)": {