change test names and address other comments

This commit is contained in:
Daniel Vega-Myhre 2023-02-17 23:02:05 +00:00
parent b0b0959b92
commit c63f448451
2 changed files with 30 additions and 39 deletions

View File

@ -568,13 +568,15 @@ func ValidateJobTemplateSpec(spec *batch.JobTemplateSpec, fldPath *field.Path, o
}
func validateCompletions(spec, oldSpec batch.JobSpec, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
if !feature.DefaultFeatureGate.Enabled(features.ElasticIndexedJob) {
return apivalidation.ValidateImmutableField(spec.Completions, oldSpec.Completions, fldPath)
}
// Completions is immutable for non-indexed jobs.
// For Indexed Jobs, if ElasticIndexedJob feature gate is not enabled,
// fall back to validating that spec.Completions is always immutable.
var allErrs field.ErrorList
isIndexedJob := spec.CompletionMode != nil && *spec.CompletionMode == batch.IndexedCompletion
if !isIndexedJob || !feature.DefaultFeatureGate.Enabled(features.ElasticIndexedJob) {
if !isIndexedJob {
return apivalidation.ValidateImmutableField(spec.Completions, oldSpec.Completions, fldPath)
}

View File

@ -1030,12 +1030,9 @@ func TestIndexedJob(t *testing.T) {
}
func TestElasticIndexedJob(t *testing.T) {
const (
noCompletionsUpdate = -1
initialCompletions = 3
)
const initialCompletions int32 = 3
type jobUpdate struct {
completions int
completions *int32
succeedIndexes []int
failIndexes []int
wantSucceededIndexes string
@ -1051,7 +1048,7 @@ func TestElasticIndexedJob(t *testing.T) {
"feature flag off, mutation not allowed": {
jobUpdates: []jobUpdate{
{
completions: 4,
completions: pointer.Int32Ptr(4),
},
},
wantErr: apierrors.NewInvalid(
@ -1060,23 +1057,22 @@ func TestElasticIndexedJob(t *testing.T) {
field.ErrorList{field.Invalid(field.NewPath("spec", "completions"), 4, "field is immutable")},
),
},
"scale up, verify that the range expands, and the job finishes successfully when indexes including the ones in the new range exit successfully": {
"scale up": {
featureGate: true,
jobUpdates: []jobUpdate{
{
// Scale up completions 3->4 then succeed indexes 0-3
completions: 4,
completions: pointer.Int32Ptr(4),
succeedIndexes: []int{0, 1, 2, 3},
wantSucceededIndexes: "0-3",
},
},
},
"scale down, verify that the range shrinks, and the job finishes successfully when indexes only in the smaller range finishes successfully, and verify that failures that happened for indexes that are now outside the range still count": {
"scale down": {
featureGate: true,
jobUpdates: []jobUpdate{
// First succeed index 1 and fail index 2 while completions is still original value (3).
{
completions: noCompletionsUpdate,
succeedIndexes: []int{1},
failIndexes: []int{2},
wantSucceededIndexes: "1",
@ -1087,19 +1083,18 @@ func TestElasticIndexedJob(t *testing.T) {
// Scale down completions 3->1, verify prev failure out of range still counts
// but succeeded out of range does not.
{
completions: 1,
completions: pointer.Int32Ptr(1),
succeedIndexes: []int{0},
wantSucceededIndexes: "0",
wantFailed: 1,
},
},
},
"index finishes successfully, scale down to exclude the index, then scale up to include it back. verify that the index was restarted and job finishes successfully after all indexes complete": {
"index finishes successfully, scale down, scale up": {
featureGate: true,
jobUpdates: []jobUpdate{
// First succeed index 2 while completions is still original value (3).
{
completions: noCompletionsUpdate,
succeedIndexes: []int{2},
wantSucceededIndexes: "2",
wantRemainingIndexes: sets.NewInt(0, 1),
@ -1107,13 +1102,13 @@ func TestElasticIndexedJob(t *testing.T) {
},
// Scale completions down 3->2 to exclude previously succeeded index.
{
completions: 2,
completions: pointer.Int32Ptr(2),
wantRemainingIndexes: sets.NewInt(0, 1),
wantActivePods: 2,
},
// Scale completions back up to include previously succeeded index that was temporarily out of range.
{
completions: 3,
completions: pointer.Int32Ptr(3),
succeedIndexes: []int{0, 1, 2},
wantSucceededIndexes: "0-2",
},
@ -1122,7 +1117,9 @@ func TestElasticIndexedJob(t *testing.T) {
"scale down to 0, verify that the job succeeds": {
featureGate: true,
jobUpdates: []jobUpdate{
{},
{
completions: pointer.Int32Ptr(0),
},
},
},
}
@ -1141,8 +1138,8 @@ func TestElasticIndexedJob(t *testing.T) {
mode := batchv1.IndexedCompletion
jobObj, err := createJobWithDefaults(ctx, clientSet, ns.Name, &batchv1.Job{
Spec: batchv1.JobSpec{
Parallelism: pointer.Int32Ptr(int32(initialCompletions)),
Completions: pointer.Int32Ptr(int32(initialCompletions)),
Parallelism: pointer.Int32Ptr(initialCompletions),
Completions: pointer.Int32Ptr(initialCompletions),
CompletionMode: &mode,
},
})
@ -1168,16 +1165,12 @@ func TestElasticIndexedJob(t *testing.T) {
for _, update := range tc.jobUpdates {
// Update Job spec if necessary.
if update.completions != noCompletionsUpdate {
if update.completions != nil {
if jobObj, err = updateJob(ctx, jobClient, jobObj.Name, func(j *batchv1.Job) {
j.Spec.Completions = pointer.Int32Ptr(int32(update.completions))
j.Spec.Parallelism = pointer.Int32Ptr(int32(update.completions))
j.Spec.Completions = update.completions
j.Spec.Parallelism = update.completions
}); err != nil {
if tc.wantErr == nil {
t.Fatalf("Failed to update Job: %v", err)
}
statusErr := err.(*apierrors.StatusError)
if diff := cmp.Diff(tc.wantErr, statusErr); diff != "" {
if diff := cmp.Diff(tc.wantErr, err); diff != "" {
t.Fatalf("Unexpected or missing errors (-want/+got): %s", diff)
}
return
@ -1185,20 +1178,16 @@ func TestElasticIndexedJob(t *testing.T) {
}
// Succeed specified indexes.
if update.succeedIndexes != nil {
for _, idx := range update.succeedIndexes {
if err := setJobPhaseForIndex(ctx, clientSet, jobObj, v1.PodSucceeded, idx); err != nil {
t.Fatalf("Failed trying to succeed pod with index %d: %v", idx, err)
}
for _, idx := range update.succeedIndexes {
if err := setJobPhaseForIndex(ctx, clientSet, jobObj, v1.PodSucceeded, idx); err != nil {
t.Fatalf("Failed trying to succeed pod with index %d: %v", idx, err)
}
}
// Fail specified indexes.
if update.failIndexes != nil {
for _, idx := range update.failIndexes {
if err := setJobPhaseForIndex(ctx, clientSet, jobObj, v1.PodFailed, idx); err != nil {
t.Fatalf("Failed trying to fail pod with index %d: %v", idx, err)
}
for _, idx := range update.failIndexes {
if err := setJobPhaseForIndex(ctx, clientSet, jobObj, v1.PodFailed, idx); err != nil {
t.Fatalf("Failed trying to fail pod with index %d: %v", idx, err)
}
}