github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-09 20:17:41 +00:00
Code Issues Projects Releases Wiki Activity

cleanup: add defaulting for job manual selector (#120206)

Browse Source 
* cleanup: add defaulting for job manual selector

* cleanup: add assert in job defaults test for manual selector

* cleanup: fix failing job storage test

* cleanup: fix batch fuzzer to handle manual selector default

* cleanup: fix lint issue on checking bool condition in job strategy

* cleanup: remove TODO in generateSelectors in job strategy Validate; inline job manual selector assignment in fuzzer
This commit is contained in:
Dejan Zele Pejchev 2023-10-11 22:51:40 +02:00 committed by GitHub
parent c83e73ba57
commit 921c0d0180
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 52 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
pkg/apis/batch/fuzzer/fuzzer.go
View File

@ -45,11 +45,7 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
j.Completions = &completions j.Completions = &completions
j.Parallelism = &parallelism j.Parallelism = &parallelism
j.BackoffLimit = &backoffLimit j.BackoffLimit = &backoffLimit
if c.Rand.Int31()%2 == 0 { j.ManualSelector = pointer.Bool(c.RandBool())
j.ManualSelector = pointer.Bool(true)
} else {
j.ManualSelector = nil
}
mode := batch.NonIndexedCompletion mode := batch.NonIndexedCompletion
if c.RandBool() { if c.RandBool() {
mode = batch.IndexedCompletion mode = batch.IndexedCompletion

3
pkg/apis/batch/v1/defaults.go
View File

@ -79,6 +79,9 @@ func SetDefaults_Job(obj *batchv1.Job) {
} }
} }
} }
if obj.Spec.ManualSelector == nil {
obj.Spec.ManualSelector = utilpointer.Bool(false)
}
} }
func SetDefaults_CronJob(obj *batchv1.CronJob) { func SetDefaults_CronJob(obj *batchv1.CronJob) {

22
pkg/apis/batch/v1/defaults_test.go
View File

@ -98,6 +98,7 @@ func TestSetDefaultJob(t *testing.T) {
BackoffLimit: pointer.Int32(6), BackoffLimit: pointer.Int32(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
ManualSelector: pointer.Bool(false),
PodFailurePolicy: &batchv1.PodFailurePolicy{ PodFailurePolicy: &batchv1.PodFailurePolicy{
Rules: []batchv1.PodFailurePolicyRule{ Rules: []batchv1.PodFailurePolicyRule{
{ {
@ -166,6 +167,7 @@ func TestSetDefaultJob(t *testing.T) {
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
PodReplacementPolicy: podReplacementPtr(batchv1.Failed), PodReplacementPolicy: podReplacementPtr(batchv1.Failed),
ManualSelector: pointer.Bool(false),
PodFailurePolicy: &batchv1.PodFailurePolicy{ PodFailurePolicy: &batchv1.PodFailurePolicy{
Rules: []batchv1.PodFailurePolicyRule{ Rules: []batchv1.PodFailurePolicyRule{
{ {
@ -198,6 +200,7 @@ func TestSetDefaultJob(t *testing.T) {
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
PodReplacementPolicy: podReplacementPtr(batchv1.TerminatingOrFailed), PodReplacementPolicy: podReplacementPtr(batchv1.TerminatingOrFailed),
ManualSelector: pointer.Bool(false),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -218,6 +221,7 @@ func TestSetDefaultJob(t *testing.T) {
BackoffLimit: pointer.Int32(6), BackoffLimit: pointer.Int32(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
ManualSelector: pointer.Bool(false),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -237,6 +241,7 @@ func TestSetDefaultJob(t *testing.T) {
BackoffLimit: pointer.Int32(6), BackoffLimit: pointer.Int32(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
ManualSelector: pointer.Bool(false),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -257,6 +262,7 @@ func TestSetDefaultJob(t *testing.T) {
BackoffLimit: pointer.Int32(6), BackoffLimit: pointer.Int32(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(true), Suspend: pointer.Bool(true),
ManualSelector: pointer.Bool(false),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -279,6 +285,7 @@ func TestSetDefaultJob(t *testing.T) {
BackoffLimit: pointer.Int32(6), BackoffLimit: pointer.Int32(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
ManualSelector: pointer.Bool(false),
}, },
}, },
}, },
@ -297,6 +304,7 @@ func TestSetDefaultJob(t *testing.T) {
BackoffLimit: pointer.Int32(6), BackoffLimit: pointer.Int32(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
ManualSelector: pointer.Bool(false),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -316,6 +324,7 @@ func TestSetDefaultJob(t *testing.T) {
BackoffLimit: pointer.Int32(6), BackoffLimit: pointer.Int32(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
ManualSelector: pointer.Bool(false),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -336,6 +345,7 @@ func TestSetDefaultJob(t *testing.T) {
BackoffLimit: pointer.Int32(6), BackoffLimit: pointer.Int32(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
ManualSelector: pointer.Bool(false),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -356,6 +366,7 @@ func TestSetDefaultJob(t *testing.T) {
BackoffLimit: pointer.Int32(5), BackoffLimit: pointer.Int32(5),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
ManualSelector: pointer.Bool(false),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -369,6 +380,7 @@ func TestSetDefaultJob(t *testing.T) {
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
PodReplacementPolicy: podReplacementPtr(batchv1.TerminatingOrFailed), PodReplacementPolicy: podReplacementPtr(batchv1.TerminatingOrFailed),
ManualSelector: pointer.Bool(false),
Template: v1.PodTemplateSpec{ Template: v1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels},
}, },
@ -382,6 +394,7 @@ func TestSetDefaultJob(t *testing.T) {
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.Bool(false), Suspend: pointer.Bool(false),
PodReplacementPolicy: podReplacementPtr(batchv1.TerminatingOrFailed), PodReplacementPolicy: podReplacementPtr(batchv1.TerminatingOrFailed),
ManualSelector: pointer.Bool(false),
Template: v1.PodTemplateSpec{ Template: v1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels},
}, },
@ -398,6 +411,7 @@ func TestSetDefaultJob(t *testing.T) {
CompletionMode: completionModePtr(batchv1.IndexedCompletion), CompletionMode: completionModePtr(batchv1.IndexedCompletion),
Suspend: pointer.Bool(true), Suspend: pointer.Bool(true),
PodReplacementPolicy: podReplacementPtr(batchv1.Failed), PodReplacementPolicy: podReplacementPtr(batchv1.Failed),
ManualSelector: pointer.Bool(true),
Template: v1.PodTemplateSpec{ Template: v1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels},
}, },
@ -411,6 +425,7 @@ func TestSetDefaultJob(t *testing.T) {
CompletionMode: completionModePtr(batchv1.IndexedCompletion), CompletionMode: completionModePtr(batchv1.IndexedCompletion),
Suspend: pointer.Bool(true), Suspend: pointer.Bool(true),
PodReplacementPolicy: podReplacementPtr(batchv1.Failed), PodReplacementPolicy: podReplacementPtr(batchv1.Failed),
ManualSelector: pointer.Bool(true),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -424,6 +439,7 @@ func TestSetDefaultJob(t *testing.T) {
CompletionMode: completionModePtr(batchv1.IndexedCompletion), CompletionMode: completionModePtr(batchv1.IndexedCompletion),
Template: validPodTemplateSpec, Template: validPodTemplateSpec,
Suspend: pointer.Bool(true), Suspend: pointer.Bool(true),
ManualSelector: pointer.Bool(false),
}, },
}, },
expected: &batchv1.Job{ expected: &batchv1.Job{
@ -435,6 +451,7 @@ func TestSetDefaultJob(t *testing.T) {
CompletionMode: completionModePtr(batchv1.IndexedCompletion), CompletionMode: completionModePtr(batchv1.IndexedCompletion),
Template: validPodTemplateSpec, Template: validPodTemplateSpec,
Suspend: pointer.Bool(true), Suspend: pointer.Bool(true),
ManualSelector: pointer.Bool(false),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -449,6 +466,7 @@ func TestSetDefaultJob(t *testing.T) {
CompletionMode: completionModePtr(batchv1.IndexedCompletion), CompletionMode: completionModePtr(batchv1.IndexedCompletion),
Template: validPodTemplateSpec, Template: validPodTemplateSpec,
Suspend: pointer.Bool(true), Suspend: pointer.Bool(true),
ManualSelector: pointer.Bool(true),
}, },
}, },
expected: &batchv1.Job{ expected: &batchv1.Job{
@ -460,6 +478,7 @@ func TestSetDefaultJob(t *testing.T) {
CompletionMode: completionModePtr(batchv1.IndexedCompletion), CompletionMode: completionModePtr(batchv1.IndexedCompletion),
Template: validPodTemplateSpec, Template: validPodTemplateSpec,
Suspend: pointer.Bool(true), Suspend: pointer.Bool(true),
ManualSelector: pointer.Bool(true),
}, },
}, },
expectLabels: true, expectLabels: true,
@ -500,6 +519,9 @@ func TestSetDefaultJob(t *testing.T) {
if diff := cmp.Diff(expected.Spec.PodReplacementPolicy, actual.Spec.PodReplacementPolicy); diff != "" { if diff := cmp.Diff(expected.Spec.PodReplacementPolicy, actual.Spec.PodReplacementPolicy); diff != "" {
t.Errorf("Unexpected PodReplacementPolicy (-want,+got):\n%s", diff) t.Errorf("Unexpected PodReplacementPolicy (-want,+got):\n%s", diff)
} }
if diff := cmp.Diff(expected.Spec.ManualSelector, actual.Spec.ManualSelector); diff != "" {
t.Errorf("Unexpected ManualSelector (-want,+got):\n%s", diff)
}
}) })
} }
} }

8
pkg/registry/batch/job/storage/storage_test.go
View File

@ -17,6 +17,7 @@ limitations under the License.
package storage package storage
import ( import (
"k8s.io/utils/pointer"
"testing" "testing"
batchv1 "k8s.io/api/batch/v1" batchv1 "k8s.io/api/batch/v1"
@ -137,9 +138,10 @@ func TestCreate(t *testing.T) {
// invalid (empty selector) // invalid (empty selector)
&batch.Job{ &batch.Job{
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Completions: validJob.Spec.Completions, ManualSelector: pointer.Bool(false),
Selector: &metav1.LabelSelector{}, Completions: validJob.Spec.Completions,
Template: validJob.Spec.Template, Selector: &metav1.LabelSelector{},
Template: validJob.Spec.Template,
}, },
}, },
) )

3
pkg/registry/batch/job/strategy.go
View File

@ -163,8 +163,7 @@ func (jobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object
// Validate validates a new job. // Validate validates a new job.
func (jobStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { func (jobStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
job := obj.(*batch.Job) job := obj.(*batch.Job)
// TODO: move UID generation earlier and do this in defaulting logic? if !*job.Spec.ManualSelector {
if job.Spec.ManualSelector == nil || *job.Spec.ManualSelector == false {
generateSelector(job) generateSelector(job)
} }
opts := validationOptionsForJob(job, nil) opts := validationOptionsForJob(job, nil)

30
pkg/registry/batch/job/strategy_test.go
View File

@ -1320,7 +1320,8 @@ func TestJobStrategy_Validate(t *testing.T) {
job: &batch.Job{ job: &batch.Job{
ObjectMeta: validObjectMeta, ObjectMeta: validObjectMeta,
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: nil, Selector: nil,
ManualSelector: pointer.Bool(false),
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Labels: validSelector.MatchLabels, Labels: validSelector.MatchLabels,
@ -1331,7 +1332,8 @@ func TestJobStrategy_Validate(t *testing.T) {
wantJob: &batch.Job{ wantJob: &batch.Job{
ObjectMeta: validObjectMeta, ObjectMeta: validObjectMeta,
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}},
ManualSelector: pointer.Bool(false),
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Labels: validSelector.MatchLabels, Labels: validSelector.MatchLabels,
@ -1344,7 +1346,8 @@ func TestJobStrategy_Validate(t *testing.T) {
job: &batch.Job{ job: &batch.Job{
ObjectMeta: validObjectMeta, ObjectMeta: validObjectMeta,
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: nil, Selector: nil,
ManualSelector: pointer.Bool(false),
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
Spec: validPodSpec, Spec: validPodSpec,
}}, }},
@ -1352,7 +1355,8 @@ func TestJobStrategy_Validate(t *testing.T) {
wantJob: &batch.Job{ wantJob: &batch.Job{
ObjectMeta: validObjectMeta, ObjectMeta: validObjectMeta,
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}},
ManualSelector: pointer.Bool(false),
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Labels: validLabels, Labels: validLabels,
@ -1365,7 +1369,8 @@ func TestJobStrategy_Validate(t *testing.T) {
job: &batch.Job{ job: &batch.Job{
ObjectMeta: validObjectMeta, ObjectMeta: validObjectMeta,
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: nil, Selector: nil,
ManualSelector: pointer.Bool(false),
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Labels: labelsWithNonBatch, Labels: labelsWithNonBatch,
@ -1376,7 +1381,8 @@ func TestJobStrategy_Validate(t *testing.T) {
wantJob: &batch.Job{ wantJob: &batch.Job{
ObjectMeta: validObjectMeta, ObjectMeta: validObjectMeta,
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}},
ManualSelector: pointer.Bool(false),
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Labels: labelsWithNonBatch, Labels: labelsWithNonBatch,
@ -1419,7 +1425,8 @@ func TestJobStrategy_Validate(t *testing.T) {
job: &batch.Job{ job: &batch.Job{
ObjectMeta: validObjectMeta, ObjectMeta: validObjectMeta,
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: nil, Selector: nil,
ManualSelector: pointer.Bool(false),
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Labels: validSelector.MatchLabels, Labels: validSelector.MatchLabels,
@ -1435,7 +1442,8 @@ func TestJobStrategy_Validate(t *testing.T) {
wantJob: &batch.Job{ wantJob: &batch.Job{
ObjectMeta: validObjectMeta, ObjectMeta: validObjectMeta,
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}},
ManualSelector: pointer.Bool(false),
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Labels: labelsWithNonBatch, Labels: labelsWithNonBatch,
@ -1453,7 +1461,8 @@ func TestJobStrategy_Validate(t *testing.T) {
job: &batch.Job{ job: &batch.Job{
ObjectMeta: validObjectMeta, ObjectMeta: validObjectMeta,
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: nil, Selector: nil,
ManualSelector: pointer.Bool(false),
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Labels: validSelector.MatchLabels, Labels: validSelector.MatchLabels,
@ -1470,7 +1479,8 @@ func TestJobStrategy_Validate(t *testing.T) {
wantJob: &batch.Job{ wantJob: &batch.Job{
ObjectMeta: validObjectMeta, ObjectMeta: validObjectMeta,
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}},
ManualSelector: pointer.Bool(false),
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Labels: labelsWithNonBatch, Labels: labelsWithNonBatch,