From 217605341571ab0b90f9b8c0aa92fbe6054628fb Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Mon, 26 Jun 2023 19:53:14 +0000 Subject: [PATCH 01/12] add completion index as pod label --- pkg/controller/job/indexed_job_utils.go | 8 ++++++++ pkg/controller/job/job_controller.go | 1 + pkg/controller/job/job_controller_test.go | 8 ++++++++ 3 files changed, 17 insertions(+) diff --git a/pkg/controller/job/indexed_job_utils.go b/pkg/controller/job/indexed_job_utils.go index 6a8ba5787fa..3eb30bb7d63 100644 --- a/pkg/controller/job/indexed_job_utils.go +++ b/pkg/controller/job/indexed_job_utils.go @@ -311,6 +311,14 @@ func addCompletionIndexAnnotation(template *v1.PodTemplateSpec, index int) { template.Annotations[batch.JobCompletionIndexAnnotation] = strconv.Itoa(index) } +func addCompletionIndexLabel(template *v1.PodTemplateSpec, index int) { + if template.Labels == nil { + template.Labels = make(map[string]string, 1) + } + // Use completion index annotation as label as well for consistency. + template.Labels[batch.JobCompletionIndexAnnotation] = strconv.Itoa(index) +} + func podGenerateNameWithIndex(jobName string, index int) string { appendIndex := "-" + strconv.Itoa(index) + "-" generateNamePrefix := jobName + appendIndex diff --git a/pkg/controller/job/job_controller.go b/pkg/controller/job/job_controller.go index 888cb7de6b1..cce20c08b5f 100644 --- a/pkg/controller/job/job_controller.go +++ b/pkg/controller/job/job_controller.go @@ -1482,6 +1482,7 @@ func (jm *Controller) manageJob(ctx context.Context, job *batch.Job, activePods if completionIndex != unknownCompletionIndex { template = podTemplate.DeepCopy() addCompletionIndexAnnotation(template, completionIndex) + addCompletionIndexLabel(template, completionIndex) template.Spec.Hostname = fmt.Sprintf("%s-%d", job.Name, completionIndex) generateName = podGenerateNameWithIndex(job.Name, completionIndex) } diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go index e583e466563..8087c16e8d2 100644 --- a/pkg/controller/job/job_controller_test.go +++ b/pkg/controller/job/job_controller_test.go @@ -962,6 +962,7 @@ func checkIndexedJobPods(t *testing.T, control *controller.FakePodControl, wantI gotIndexes := sets.New[int]() for _, p := range control.Templates { checkJobCompletionEnvVariable(t, &p.Spec) + checkJobCompletionLabel(t, &p) ix := getCompletionIndex(p.Annotations) if ix == -1 { t.Errorf("Created pod %s didn't have completion index", p.Name) @@ -4394,6 +4395,13 @@ func TestFinalizersRemovedExpectations(t *testing.T) { t.Errorf("Timeout waiting for expectations (-want, +got):\n%s", diff) } } +func checkJobCompletionLabel(t *testing.T, p *v1.PodTemplateSpec) { + t.Helper() + labels := p.GetLabels() + if labels == nil || labels[batch.JobCompletionIndexAnnotation] == "" { + t.Errorf("missing expected pod label %s", batch.JobCompletionIndexAnnotation) + } +} func checkJobCompletionEnvVariable(t *testing.T, spec *v1.PodSpec) { t.Helper() From a9afaa1eee95e4b6f1b224372a52981c20573e48 Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Tue, 27 Jun 2023 18:06:09 +0000 Subject: [PATCH 02/12] add feature gate --- pkg/controller/job/job_controller.go | 5 ++++- pkg/controller/job/job_controller_test.go | 5 ++++- pkg/features/kube_features.go | 9 +++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/pkg/controller/job/job_controller.go b/pkg/controller/job/job_controller.go index cce20c08b5f..fa76c7ccff5 100644 --- a/pkg/controller/job/job_controller.go +++ b/pkg/controller/job/job_controller.go @@ -1482,7 +1482,10 @@ func (jm *Controller) manageJob(ctx context.Context, job *batch.Job, activePods if completionIndex != unknownCompletionIndex { template = podTemplate.DeepCopy() addCompletionIndexAnnotation(template, completionIndex) - addCompletionIndexLabel(template, completionIndex) + + if feature.DefaultFeatureGate.Enabled(features.PodIndexLabel) { + addCompletionIndexLabel(template, completionIndex) + } template.Spec.Hostname = fmt.Sprintf("%s-%d", job.Name, completionIndex) generateName = podGenerateNameWithIndex(job.Name, completionIndex) } diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go index 8087c16e8d2..a0608acbad3 100644 --- a/pkg/controller/job/job_controller_test.go +++ b/pkg/controller/job/job_controller_test.go @@ -962,7 +962,9 @@ func checkIndexedJobPods(t *testing.T, control *controller.FakePodControl, wantI gotIndexes := sets.New[int]() for _, p := range control.Templates { checkJobCompletionEnvVariable(t, &p.Spec) - checkJobCompletionLabel(t, &p) + if feature.DefaultFeatureGate.Enabled(features.PodIndexLabel) { + checkJobCompletionLabel(t, &p) + } ix := getCompletionIndex(p.Annotations) if ix == -1 { t.Errorf("Created pod %s didn't have completion index", p.Name) @@ -4395,6 +4397,7 @@ func TestFinalizersRemovedExpectations(t *testing.T) { t.Errorf("Timeout waiting for expectations (-want, +got):\n%s", diff) } } + func checkJobCompletionLabel(t *testing.T, p *v1.PodTemplateSpec) { t.Helper() labels := p.GetLabels() diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 3827a55e388..26ea0409fe3 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -843,6 +843,13 @@ const ( // // Enables In-Place Pod Vertical Scaling InPlacePodVerticalScaling featuregate.Feature = "InPlacePodVerticalScaling" + + // owner: @danielvegamyhre + // kep: https://kep.k8s.io/4017 + // beta: v1.28 + // + // Set pod completion index as a pod label for Indexed Jobs and StatefulSets. + PodIndexLabel featuregate.Feature = "PodIndexLabel" ) func init() { @@ -1072,6 +1079,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS InPlacePodVerticalScaling: {Default: false, PreRelease: featuregate.Alpha}, + PodIndexLabel: {Default: true, PreRelease: featuregate.Beta}, + // inherited features from generic apiserver, relisted here to get a conflict if it is changed // unintentionally on either side: From cfa2fa6d7741d024adca7f1b1bd635c2df7eb93d Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Tue, 27 Jun 2023 22:20:17 +0000 Subject: [PATCH 03/12] put feature gate in alphabetical order --- pkg/features/kube_features.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 26ea0409fe3..c2190fd2193 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -606,6 +606,13 @@ const ( // the pod is being deleted due to a disruption. PodDisruptionConditions featuregate.Feature = "PodDisruptionConditions" + // owner: @danielvegamyhre + // kep: https://kep.k8s.io/4017 + // beta: v1.28 + // + // Set pod completion index as a pod label for Indexed Jobs and StatefulSets. + PodIndexLabel featuregate.Feature = "PodIndexLabel" + // owner: @ddebroy // alpha: v1.25 // @@ -843,13 +850,6 @@ const ( // // Enables In-Place Pod Vertical Scaling InPlacePodVerticalScaling featuregate.Feature = "InPlacePodVerticalScaling" - - // owner: @danielvegamyhre - // kep: https://kep.k8s.io/4017 - // beta: v1.28 - // - // Set pod completion index as a pod label for Indexed Jobs and StatefulSets. - PodIndexLabel featuregate.Feature = "PodIndexLabel" ) func init() { From e0af0a5a4557cfc0440561edb885def6ad5171de Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Thu, 29 Jun 2023 21:51:15 +0000 Subject: [PATCH 04/12] add test case param for feature flag --- pkg/controller/job/job_controller_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go index a0608acbad3..94c2b35519f 100644 --- a/pkg/controller/job/job_controller_test.go +++ b/pkg/controller/job/job_controller_test.go @@ -274,7 +274,8 @@ func TestControllerSyncJob(t *testing.T) { expectedPodPatches int // features - jobReadyPodsEnabled bool + jobReadyPodsEnabled bool + podIndexLabelEnabled bool }{ "job start": { parallelism: 2, @@ -781,11 +782,22 @@ func TestControllerSyncJob(t *testing.T) { expectedActive: 2, expectedPodPatches: 2, }, + "indexed job with podIndexLabel feature enabled": { + parallelism: 2, + completions: 5, + backoffLimit: 6, + completionMode: batch.IndexedCompletion, + expectedCreations: 2, + expectedActive: 2, + expectedCreatedIndexes: sets.New(0, 1), + podIndexLabelEnabled: true, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.JobReadyPods, tc.jobReadyPodsEnabled)() + defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.PodIndexLabel, tc.podIndexLabelEnabled)() // job manager setup clientSet := clientset.NewForConfigOrDie(&restclient.Config{Host: "", ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}}) From a647f9febbfba8ea7880e78b6e76b2ee1e08ab21 Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Wed, 5 Jul 2023 18:47:45 +0000 Subject: [PATCH 05/12] default enabled pod index for test cases, add test case disabling it --- pkg/controller/job/job_controller_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go index 94c2b35519f..4f47846c0ea 100644 --- a/pkg/controller/job/job_controller_test.go +++ b/pkg/controller/job/job_controller_test.go @@ -274,8 +274,8 @@ func TestControllerSyncJob(t *testing.T) { expectedPodPatches int // features - jobReadyPodsEnabled bool - podIndexLabelEnabled bool + jobReadyPodsEnabled bool + podIndexLabelDisabled bool }{ "job start": { parallelism: 2, @@ -782,7 +782,7 @@ func TestControllerSyncJob(t *testing.T) { expectedActive: 2, expectedPodPatches: 2, }, - "indexed job with podIndexLabel feature enabled": { + "indexed job with podIndexLabel feature disabled": { parallelism: 2, completions: 5, backoffLimit: 6, @@ -790,14 +790,14 @@ func TestControllerSyncJob(t *testing.T) { expectedCreations: 2, expectedActive: 2, expectedCreatedIndexes: sets.New(0, 1), - podIndexLabelEnabled: true, + podIndexLabelDisabled: true, }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.JobReadyPods, tc.jobReadyPodsEnabled)() - defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.PodIndexLabel, tc.podIndexLabelEnabled)() + defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.PodIndexLabel, !tc.podIndexLabelDisabled)() // job manager setup clientSet := clientset.NewForConfigOrDie(&restclient.Config{Host: "", ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}}) From 3a02ecb3418daebe155485928dc23af18be7537f Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Thu, 6 Jul 2023 17:30:40 +0000 Subject: [PATCH 06/12] check test case param instead of feature flag in unit test code --- pkg/controller/job/job_controller_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go index 4f47846c0ea..c9c147c6e41 100644 --- a/pkg/controller/job/job_controller_test.go +++ b/pkg/controller/job/job_controller_test.go @@ -882,7 +882,7 @@ func TestControllerSyncJob(t *testing.T) { t.Errorf("Unexpected number of creates. Expected %d, saw %d\n", tc.expectedCreations, len(fakePodControl.Templates)) } if tc.completionMode == batch.IndexedCompletion { - checkIndexedJobPods(t, &fakePodControl, tc.expectedCreatedIndexes, job.Name) + checkIndexedJobPods(t, &fakePodControl, tc.expectedCreatedIndexes, job.Name, tc.podIndexLabelDisabled) } else { for _, p := range fakePodControl.Templates { // Fake pod control doesn't add generate name from the owner reference. @@ -969,12 +969,12 @@ func TestControllerSyncJob(t *testing.T) { } } -func checkIndexedJobPods(t *testing.T, control *controller.FakePodControl, wantIndexes sets.Set[int], jobName string) { +func checkIndexedJobPods(t *testing.T, control *controller.FakePodControl, wantIndexes sets.Set[int], jobName string, podIndexLabelDisabled bool) { t.Helper() gotIndexes := sets.New[int]() for _, p := range control.Templates { checkJobCompletionEnvVariable(t, &p.Spec) - if feature.DefaultFeatureGate.Enabled(features.PodIndexLabel) { + if !podIndexLabelDisabled { checkJobCompletionLabel(t, &p) } ix := getCompletionIndex(p.Annotations) From ecf0cee91ce68fccfecf8567e785f3ed0b4528fc Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Thu, 6 Jul 2023 17:33:11 +0000 Subject: [PATCH 07/12] update comment on feature flag --- pkg/features/kube_features.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index c2190fd2193..171192eb34b 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -610,7 +610,7 @@ const ( // kep: https://kep.k8s.io/4017 // beta: v1.28 // - // Set pod completion index as a pod label for Indexed Jobs and StatefulSets. + // Set pod completion index as a pod label for Indexed Jobs. PodIndexLabel featuregate.Feature = "PodIndexLabel" // owner: @ddebroy From 98c6e25c37128c10e238bf24a31356c9b4540d55 Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Mon, 10 Jul 2023 18:19:47 +0000 Subject: [PATCH 08/12] update name of pod index label --- pkg/controller/job/indexed_job_utils.go | 3 +-- pkg/controller/job/job_controller_test.go | 4 ++-- staging/src/k8s.io/api/batch/v1/types.go | 2 ++ 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/controller/job/indexed_job_utils.go b/pkg/controller/job/indexed_job_utils.go index 3eb30bb7d63..9ec8f8535b2 100644 --- a/pkg/controller/job/indexed_job_utils.go +++ b/pkg/controller/job/indexed_job_utils.go @@ -315,8 +315,7 @@ func addCompletionIndexLabel(template *v1.PodTemplateSpec, index int) { if template.Labels == nil { template.Labels = make(map[string]string, 1) } - // Use completion index annotation as label as well for consistency. - template.Labels[batch.JobCompletionIndexAnnotation] = strconv.Itoa(index) + template.Labels[batch.JobCompletionIndexLabel] = strconv.Itoa(index) } func podGenerateNameWithIndex(jobName string, index int) string { diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go index c9c147c6e41..e3b71aa68d9 100644 --- a/pkg/controller/job/job_controller_test.go +++ b/pkg/controller/job/job_controller_test.go @@ -4413,8 +4413,8 @@ func TestFinalizersRemovedExpectations(t *testing.T) { func checkJobCompletionLabel(t *testing.T, p *v1.PodTemplateSpec) { t.Helper() labels := p.GetLabels() - if labels == nil || labels[batch.JobCompletionIndexAnnotation] == "" { - t.Errorf("missing expected pod label %s", batch.JobCompletionIndexAnnotation) + if labels == nil || labels[batch.JobCompletionIndexLabel] == "" { + t.Errorf("missing expected pod label %s", batch.JobCompletionIndexLabel) } } diff --git a/staging/src/k8s.io/api/batch/v1/types.go b/staging/src/k8s.io/api/batch/v1/types.go index 22cf9ee9cb6..15cc874ff21 100644 --- a/staging/src/k8s.io/api/batch/v1/types.go +++ b/staging/src/k8s.io/api/batch/v1/types.go @@ -28,6 +28,8 @@ const ( labelPrefix = "batch.kubernetes.io/" JobCompletionIndexAnnotation = labelPrefix + "job-completion-index" + // JobCompletionIndexLabel is defined following the format .kubernetes.io/pod-index + JobCompletionIndexLabel = "job.kubernetes.io/pod-index" // JobTrackingFinalizer is a finalizer for Job's pods. It prevents them from // being deleted before being accounted in the Job status. // From 1ae60c0ed1636947bbc1c1899e2ca4b8f1c9d96f Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Thu, 13 Jul 2023 21:03:18 +0000 Subject: [PATCH 09/12] use job completion index annotation as label --- pkg/controller/job/indexed_job_utils.go | 3 ++- pkg/controller/job/job_controller_test.go | 4 ++-- staging/src/k8s.io/api/apps/v1/types.go | 3 ++- staging/src/k8s.io/api/batch/v1/types.go | 2 -- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/controller/job/indexed_job_utils.go b/pkg/controller/job/indexed_job_utils.go index 9ec8f8535b2..0778e60ba58 100644 --- a/pkg/controller/job/indexed_job_utils.go +++ b/pkg/controller/job/indexed_job_utils.go @@ -315,7 +315,8 @@ func addCompletionIndexLabel(template *v1.PodTemplateSpec, index int) { if template.Labels == nil { template.Labels = make(map[string]string, 1) } - template.Labels[batch.JobCompletionIndexLabel] = strconv.Itoa(index) + // For consistency, we use the annotation batch.kubernetes.io/job-completion-index for the corresponding label as well. + template.Labels[batch.JobCompletionIndexAnnotation] = strconv.Itoa(index) } func podGenerateNameWithIndex(jobName string, index int) string { diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go index e3b71aa68d9..c9c147c6e41 100644 --- a/pkg/controller/job/job_controller_test.go +++ b/pkg/controller/job/job_controller_test.go @@ -4413,8 +4413,8 @@ func TestFinalizersRemovedExpectations(t *testing.T) { func checkJobCompletionLabel(t *testing.T, p *v1.PodTemplateSpec) { t.Helper() labels := p.GetLabels() - if labels == nil || labels[batch.JobCompletionIndexLabel] == "" { - t.Errorf("missing expected pod label %s", batch.JobCompletionIndexLabel) + if labels == nil || labels[batch.JobCompletionIndexAnnotation] == "" { + t.Errorf("missing expected pod label %s", batch.JobCompletionIndexAnnotation) } } diff --git a/staging/src/k8s.io/api/apps/v1/types.go b/staging/src/k8s.io/api/apps/v1/types.go index 15dc3150a63..c3f5a6e21c3 100644 --- a/staging/src/k8s.io/api/apps/v1/types.go +++ b/staging/src/k8s.io/api/apps/v1/types.go @@ -17,7 +17,7 @@ limitations under the License. package v1 import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" @@ -29,6 +29,7 @@ const ( DeprecatedRollbackTo = "deprecated.deployment.rollback.to" DeprecatedTemplateGeneration = "deprecated.daemonset.template.generation" StatefulSetPodNameLabel = "statefulset.kubernetes.io/pod-name" + StatefulSetPodIndexLabel = "statefulset.kubernetes.io/pod-index" ) // +genclient diff --git a/staging/src/k8s.io/api/batch/v1/types.go b/staging/src/k8s.io/api/batch/v1/types.go index 15cc874ff21..22cf9ee9cb6 100644 --- a/staging/src/k8s.io/api/batch/v1/types.go +++ b/staging/src/k8s.io/api/batch/v1/types.go @@ -28,8 +28,6 @@ const ( labelPrefix = "batch.kubernetes.io/" JobCompletionIndexAnnotation = labelPrefix + "job-completion-index" - // JobCompletionIndexLabel is defined following the format .kubernetes.io/pod-index - JobCompletionIndexLabel = "job.kubernetes.io/pod-index" // JobTrackingFinalizer is a finalizer for Job's pods. It prevents them from // being deleted before being accounted in the Job status. // From a1a5f49bb9cb2f9e76fcf2c50aaf8093df1856d4 Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Thu, 13 Jul 2023 21:07:17 +0000 Subject: [PATCH 10/12] remove statefulset label added to wrong branch --- pkg/controller/job/indexed_job_utils.go | 2 +- staging/src/k8s.io/api/apps/v1/types.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/controller/job/indexed_job_utils.go b/pkg/controller/job/indexed_job_utils.go index 0778e60ba58..ef6b4e23426 100644 --- a/pkg/controller/job/indexed_job_utils.go +++ b/pkg/controller/job/indexed_job_utils.go @@ -298,7 +298,7 @@ func addCompletionIndexEnvVariable(container *v1.Container) { Name: completionIndexEnvName, ValueFrom: &v1.EnvVarSource{ FieldRef: &v1.ObjectFieldSelector{ - FieldPath: fmt.Sprintf("metadata.annotations['%s']", batch.JobCompletionIndexAnnotation), + FieldPath: fmt.Sprintf("metadata.labels['%s']", batch.JobCompletionIndexAnnotation), }, }, }) diff --git a/staging/src/k8s.io/api/apps/v1/types.go b/staging/src/k8s.io/api/apps/v1/types.go index c3f5a6e21c3..5ca27cc7eee 100644 --- a/staging/src/k8s.io/api/apps/v1/types.go +++ b/staging/src/k8s.io/api/apps/v1/types.go @@ -29,7 +29,6 @@ const ( DeprecatedRollbackTo = "deprecated.deployment.rollback.to" DeprecatedTemplateGeneration = "deprecated.daemonset.template.generation" StatefulSetPodNameLabel = "statefulset.kubernetes.io/pod-name" - StatefulSetPodIndexLabel = "statefulset.kubernetes.io/pod-index" ) // +genclient From 037091284e21741bb0427c489694e84774fff8cc Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Thu, 13 Jul 2023 22:38:21 +0000 Subject: [PATCH 11/12] fix unit test bug --- pkg/controller/job/indexed_job_utils.go | 10 +++++++++- pkg/controller/job/job_controller_test.go | 12 +++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/pkg/controller/job/indexed_job_utils.go b/pkg/controller/job/indexed_job_utils.go index ef6b4e23426..43822a95e9a 100644 --- a/pkg/controller/job/indexed_job_utils.go +++ b/pkg/controller/job/indexed_job_utils.go @@ -26,8 +26,10 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/storage/names" + "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/controller" + "k8s.io/kubernetes/pkg/features" ) const ( @@ -294,11 +296,17 @@ func addCompletionIndexEnvVariable(container *v1.Container) { return } } + var fieldPath string + if feature.DefaultFeatureGate.Enabled(features.PodIndexLabel) { + fieldPath = fmt.Sprintf("metadata.labels['%s']", batch.JobCompletionIndexAnnotation) + } else { + fieldPath = fmt.Sprintf("metadata.annotations['%s']", batch.JobCompletionIndexAnnotation) + } container.Env = append(container.Env, v1.EnvVar{ Name: completionIndexEnvName, ValueFrom: &v1.EnvVarSource{ FieldRef: &v1.ObjectFieldSelector{ - FieldPath: fmt.Sprintf("metadata.labels['%s']", batch.JobCompletionIndexAnnotation), + FieldPath: fieldPath, }, }, }) diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go index c9c147c6e41..a830a4c7e7b 100644 --- a/pkg/controller/job/job_controller_test.go +++ b/pkg/controller/job/job_controller_test.go @@ -973,7 +973,7 @@ func checkIndexedJobPods(t *testing.T, control *controller.FakePodControl, wantI t.Helper() gotIndexes := sets.New[int]() for _, p := range control.Templates { - checkJobCompletionEnvVariable(t, &p.Spec) + checkJobCompletionEnvVariable(t, &p.Spec, podIndexLabelDisabled) if !podIndexLabelDisabled { checkJobCompletionLabel(t, &p) } @@ -4418,14 +4418,20 @@ func checkJobCompletionLabel(t *testing.T, p *v1.PodTemplateSpec) { } } -func checkJobCompletionEnvVariable(t *testing.T, spec *v1.PodSpec) { +func checkJobCompletionEnvVariable(t *testing.T, spec *v1.PodSpec, podIndexLabelDisabled bool) { t.Helper() + var fieldPath string + if podIndexLabelDisabled { + fieldPath = fmt.Sprintf("metadata.annotations['%s']", batch.JobCompletionIndexAnnotation) + } else { + fieldPath = fmt.Sprintf("metadata.labels['%s']", batch.JobCompletionIndexAnnotation) + } want := []v1.EnvVar{ { Name: "JOB_COMPLETION_INDEX", ValueFrom: &v1.EnvVarSource{ FieldRef: &v1.ObjectFieldSelector{ - FieldPath: fmt.Sprintf("metadata.annotations['%s']", batch.JobCompletionIndexAnnotation), + FieldPath: fieldPath, }, }, }, From 5f37c102cb5aa0e76ab6d6401077afa1d954d727 Mon Sep 17 00:00:00 2001 From: Daniel Vega-Myhre Date: Fri, 14 Jul 2023 16:48:13 +0000 Subject: [PATCH 12/12] revert changes in staging --- staging/src/k8s.io/api/apps/v1/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staging/src/k8s.io/api/apps/v1/types.go b/staging/src/k8s.io/api/apps/v1/types.go index 5ca27cc7eee..15dc3150a63 100644 --- a/staging/src/k8s.io/api/apps/v1/types.go +++ b/staging/src/k8s.io/api/apps/v1/types.go @@ -17,7 +17,7 @@ limitations under the License. package v1 import ( - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr"