Skip to content

Commit

Permalink
Merge pull request #107395 from alculquicondor/indexed-job
Browse files Browse the repository at this point in the history 
Graduate IndexedJob to stable
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Mar 24, 2022
2 parents 324ba13 + 2c5d0a2 commit f97825e
Show file tree
Hide file tree
Showing 18 changed files with 72 additions and 141 deletions.
2 changes: 1 addition & 1 deletion api/openapi-spec/swagger.json
View file

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

2 changes: 1 addition & 1 deletion api/openapi-spec/v3/apis__batch__v1_openapi.json
View file
Expand Up @@ -270,7 +270,7 @@
"type": "integer"
},
"completionMode": {
"description": "CompletionMode specifies how Pod completions are tracked. It can be `NonIndexed` (default) or `Indexed`.\n\n`NonIndexed` means that the Job is considered complete when there have been .spec.completions successfully completed Pods. Each Pod completion is homologous to each other.\n\n`Indexed` means that the Pods of a Job get an associated completion index from 0 to (.spec.completions - 1), available in the annotation batch.kubernetes.io/job-completion-index. The Job is considered complete when there is one successfully completed Pod for each index. When value is `Indexed`, .spec.completions must be specified and `.spec.parallelism` must be less than or equal to 10^5. In addition, The Pod name takes the form `$(job-name)-$(index)-$(random-string)`, the Pod hostname takes the form `$(job-name)-$(index)`.\n\nThis field is beta-level. More completion modes can be added in the future. If the Job controller observes a mode that it doesn't recognize, the controller skips updates for the Job.",
"description": "CompletionMode specifies how Pod completions are tracked. It can be `NonIndexed` (default) or `Indexed`.\n\n`NonIndexed` means that the Job is considered complete when there have been .spec.completions successfully completed Pods. Each Pod completion is homologous to each other.\n\n`Indexed` means that the Pods of a Job get an associated completion index from 0 to (.spec.completions - 1), available in the annotation batch.kubernetes.io/job-completion-index. The Job is considered complete when there is one successfully completed Pod for each index. When value is `Indexed`, .spec.completions must be specified and `.spec.parallelism` must be less than or equal to 10^5. In addition, The Pod name takes the form `$(job-name)-$(index)-$(random-string)`, the Pod hostname takes the form `$(job-name)-$(index)`.\n\nMore completion modes can be added in the future. If the Job controller observes a mode that it doesn't recognize, which is possible during upgrades due to version skew, the controller skips updates for the Job.",
"type": "string"
},
"completions": {
Expand Down
2 changes: 1 addition & 1 deletion api/openapi-spec/v3/apis__batch__v1beta1_openapi.json
View file
Expand Up @@ -15,7 +15,7 @@
"type": "integer"
},
"completionMode": {
"description": "CompletionMode specifies how Pod completions are tracked. It can be `NonIndexed` (default) or `Indexed`.\n\n`NonIndexed` means that the Job is considered complete when there have been .spec.completions successfully completed Pods. Each Pod completion is homologous to each other.\n\n`Indexed` means that the Pods of a Job get an associated completion index from 0 to (.spec.completions - 1), available in the annotation batch.kubernetes.io/job-completion-index. The Job is considered complete when there is one successfully completed Pod for each index. When value is `Indexed`, .spec.completions must be specified and `.spec.parallelism` must be less than or equal to 10^5. In addition, The Pod name takes the form `$(job-name)-$(index)-$(random-string)`, the Pod hostname takes the form `$(job-name)-$(index)`.\n\nThis field is beta-level. More completion modes can be added in the future. If the Job controller observes a mode that it doesn't recognize, the controller skips updates for the Job.",
"description": "CompletionMode specifies how Pod completions are tracked. It can be `NonIndexed` (default) or `Indexed`.\n\n`NonIndexed` means that the Job is considered complete when there have been .spec.completions successfully completed Pods. Each Pod completion is homologous to each other.\n\n`Indexed` means that the Pods of a Job get an associated completion index from 0 to (.spec.completions - 1), available in the annotation batch.kubernetes.io/job-completion-index. The Job is considered complete when there is one successfully completed Pod for each index. When value is `Indexed`, .spec.completions must be specified and `.spec.parallelism` must be less than or equal to 10^5. In addition, The Pod name takes the form `$(job-name)-$(index)-$(random-string)`, the Pod hostname takes the form `$(job-name)-$(index)`.\n\nMore completion modes can be added in the future. If the Job controller observes a mode that it doesn't recognize, which is possible during upgrades due to version skew, the controller skips updates for the Job.",
"type": "string"
},
"completions": {
Expand Down
7 changes: 4 additions & 3 deletions pkg/apis/batch/types.go
View file
Expand Up @@ -194,9 +194,10 @@ type JobSpec struct {
// `$(job-name)-$(index)-$(random-string)`,
// the Pod hostname takes the form `$(job-name)-$(index)`.
//
// This field is beta-level. More completion modes can be added in the future.
// If the Job controller observes a mode that it doesn't recognize, the
// controller skips updates for the Job.
// More completion modes can be added in the future.
// If the Job controller observes a mode that it doesn't recognize, which
// is possible during upgrades due to version skew, the controller
// skips updates for the Job.
// +optional
CompletionMode *CompletionMode

Expand Down
4 changes: 1 addition & 3 deletions pkg/apis/batch/v1/defaults.go
View file
Expand Up @@ -19,8 +19,6 @@ package v1
import (
batchv1 "k8s.io/api/batch/v1"
"k8s.io/apimachinery/pkg/runtime"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/features"
utilpointer "k8s.io/utils/pointer"
)

Expand All @@ -45,7 +43,7 @@ func SetDefaults_Job(obj *batchv1.Job) {
if labels != nil && len(obj.Labels) == 0 {
obj.Labels = labels
}
if utilfeature.DefaultFeatureGate.Enabled(features.IndexedJob) && obj.Spec.CompletionMode == nil {
if obj.Spec.CompletionMode == nil {
mode := batchv1.NonIndexedCompletion
obj.Spec.CompletionMode = &mode
}
Expand Down
90 changes: 36 additions & 54 deletions pkg/apis/batch/v1/defaults_test.go
View file
Expand Up @@ -25,12 +25,9 @@ import (
"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/api/legacyscheme"
_ "k8s.io/kubernetes/pkg/apis/batch/install"
_ "k8s.io/kubernetes/pkg/apis/core/install"
"k8s.io/kubernetes/pkg/features"
"k8s.io/utils/pointer"

. "k8s.io/kubernetes/pkg/apis/batch/v1"
Expand All @@ -39,31 +36,11 @@ import (
func TestSetDefaultJob(t *testing.T) {
defaultLabels := map[string]string{"default": "default"}
tests := map[string]struct {
indexedJobEnabled bool
original *batchv1.Job
expected *batchv1.Job
expectLabels bool
original *batchv1.Job
expected *batchv1.Job
expectLabels bool
}{
"All unspecified -> sets all to default values": {
original: &batchv1.Job{
Spec: batchv1.JobSpec{
Template: v1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels},
},
},
},
expected: &batchv1.Job{
Spec: batchv1.JobSpec{
Completions: pointer.Int32Ptr(1),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(6),
Suspend: pointer.BoolPtr(false),
},
},
expectLabels: true,
},
"All unspecified, indexed job enabled -> sets all to default values": {
indexedJobEnabled: true,
original: &batchv1.Job{
Spec: batchv1.JobSpec{
Template: v1.PodTemplateSpec{
Expand Down Expand Up @@ -92,10 +69,11 @@ func TestSetDefaultJob(t *testing.T) {
},
expected: &batchv1.Job{
Spec: batchv1.JobSpec{
Completions: pointer.Int32Ptr(1),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(6),
Suspend: pointer.BoolPtr(false),
Completions: pointer.Int32Ptr(1),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.BoolPtr(false),
},
},
expectLabels: true,
Expand All @@ -111,10 +89,11 @@ func TestSetDefaultJob(t *testing.T) {
},
expected: &batchv1.Job{
Spec: batchv1.JobSpec{
Completions: pointer.Int32Ptr(1),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(6),
Suspend: pointer.BoolPtr(true),
Completions: pointer.Int32Ptr(1),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.BoolPtr(true),
},
},
expectLabels: true,
Expand All @@ -132,10 +111,11 @@ func TestSetDefaultJob(t *testing.T) {
},
expected: &batchv1.Job{
Spec: batchv1.JobSpec{
Completions: pointer.Int32Ptr(1),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(6),
Suspend: pointer.BoolPtr(false),
Completions: pointer.Int32Ptr(1),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.BoolPtr(false),
},
},
},
Expand All @@ -150,9 +130,10 @@ func TestSetDefaultJob(t *testing.T) {
},
expected: &batchv1.Job{
Spec: batchv1.JobSpec{
Parallelism: pointer.Int32Ptr(0),
BackoffLimit: pointer.Int32Ptr(6),
Suspend: pointer.BoolPtr(false),
Parallelism: pointer.Int32Ptr(0),
BackoffLimit: pointer.Int32Ptr(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.BoolPtr(false),
},
},
expectLabels: true,
Expand All @@ -168,9 +149,10 @@ func TestSetDefaultJob(t *testing.T) {
},
expected: &batchv1.Job{
Spec: batchv1.JobSpec{
Parallelism: pointer.Int32Ptr(2),
BackoffLimit: pointer.Int32Ptr(6),
Suspend: pointer.BoolPtr(false),
Parallelism: pointer.Int32Ptr(2),
BackoffLimit: pointer.Int32Ptr(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.BoolPtr(false),
},
},
expectLabels: true,
Expand All @@ -186,10 +168,11 @@ func TestSetDefaultJob(t *testing.T) {
},
expected: &batchv1.Job{
Spec: batchv1.JobSpec{
Completions: pointer.Int32Ptr(2),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(6),
Suspend: pointer.BoolPtr(false),
Completions: pointer.Int32Ptr(2),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(6),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.BoolPtr(false),
},
},
expectLabels: true,
Expand All @@ -205,10 +188,11 @@ func TestSetDefaultJob(t *testing.T) {
},
expected: &batchv1.Job{
Spec: batchv1.JobSpec{
Completions: pointer.Int32Ptr(1),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(5),
Suspend: pointer.BoolPtr(false),
Completions: pointer.Int32Ptr(1),
Parallelism: pointer.Int32Ptr(1),
BackoffLimit: pointer.Int32Ptr(5),
CompletionMode: completionModePtr(batchv1.NonIndexedCompletion),
Suspend: pointer.BoolPtr(false),
},
},
expectLabels: true,
Expand Down Expand Up @@ -268,8 +252,6 @@ func TestSetDefaultJob(t *testing.T) {

for name, test := range tests {
t.Run(name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.IndexedJob, test.indexedJobEnabled)()

original := test.original
expected := test.expected
obj2 := roundTrip(t, runtime.Object(original))
Expand Down
5 changes: 0 additions & 5 deletions pkg/controller/job/job_controller.go
View file
Expand Up @@ -634,11 +634,6 @@ func (jm *Controller) syncJob(ctx context.Context, key string) (forget bool, rEr
return true, nil
}

// Cannot create Pods if this is an Indexed Job and the feature is disabled.
if !feature.DefaultFeatureGate.Enabled(features.IndexedJob) && isIndexedJob(&job) {
jm.recorder.Event(&job, v1.EventTypeWarning, "IndexedJobDisabled", "Skipped Indexed Job sync because feature is disabled.")
return false, nil
}
if job.Spec.CompletionMode != nil && *job.Spec.CompletionMode != batch.NonIndexedCompletion && *job.Spec.CompletionMode != batch.IndexedCompletion {
jm.recorder.Event(&job, v1.EventTypeWarning, "UnknownCompletionMode", "Skipped Job sync because completion mode is unknown")
return false, nil
Expand Down
24 changes: 1 addition & 23 deletions pkg/controller/job/job_controller_test.go
View file
Expand Up @@ -224,7 +224,6 @@ func TestControllerSyncJob(t *testing.T) {
expectedPodPatches int

// features
indexedJobEnabled bool
jobReadyPodsEnabled bool
}{
"job start": {
Expand Down Expand Up @@ -490,7 +489,6 @@ func TestControllerSyncJob(t *testing.T) {
expectedCreations: 2,
expectedActive: 2,
expectedCreatedIndexes: sets.NewInt(0, 1),
indexedJobEnabled: true,
},
"indexed job completed": {
parallelism: 2,
Expand All @@ -510,7 +508,6 @@ func TestControllerSyncJob(t *testing.T) {
expectedCondition: &jobConditionComplete,
expectedConditionStatus: v1.ConditionTrue,
expectedPodPatches: 4,
indexedJobEnabled: true,
},
"indexed job repeated completed index": {
parallelism: 2,
Expand All @@ -529,7 +526,6 @@ func TestControllerSyncJob(t *testing.T) {
expectedCompletedIdxs: "0,1",
expectedCreatedIndexes: sets.NewInt(2),
expectedPodPatches: 3,
indexedJobEnabled: true,
},
"indexed job some running and completed pods": {
parallelism: 8,
Expand All @@ -553,7 +549,6 @@ func TestControllerSyncJob(t *testing.T) {
expectedCompletedIdxs: "2,4,5,7-9",
expectedCreatedIndexes: sets.NewInt(1, 6, 10, 11, 12, 13),
expectedPodPatches: 6,
indexedJobEnabled: true,
},
"indexed job some failed pods": {
parallelism: 3,
Expand All @@ -570,7 +565,6 @@ func TestControllerSyncJob(t *testing.T) {
expectedFailed: 2,
expectedCreatedIndexes: sets.NewInt(0, 2),
expectedPodPatches: 2,
indexedJobEnabled: true,
},
"indexed job some pods without index": {
parallelism: 2,
Expand All @@ -596,7 +590,6 @@ func TestControllerSyncJob(t *testing.T) {
expectedFailed: 0,
expectedCompletedIdxs: "0",
expectedPodPatches: 8,
indexedJobEnabled: true,
},
"indexed job repeated indexes": {
parallelism: 5,
Expand All @@ -619,7 +612,6 @@ func TestControllerSyncJob(t *testing.T) {
expectedSucceeded: 1,
expectedCompletedIdxs: "0",
expectedPodPatches: 5,
indexedJobEnabled: true,
},
"indexed job with indexes outside of range": {
parallelism: 2,
Expand All @@ -641,19 +633,6 @@ func TestControllerSyncJob(t *testing.T) {
expectedActive: 0,
expectedFailed: 0,
expectedPodPatches: 5,
indexedJobEnabled: true,
},
"indexed job feature disabled": {
parallelism: 2,
completions: 3,
backoffLimit: 6,
completionMode: batch.IndexedCompletion,
podsWithIndexes: []indexPhase{
{"0", v1.PodRunning},
{"1", v1.PodSucceeded},
},
// No status updates.
indexedJobEnabled: false,
},
"suspending a job with satisfied expectations": {
// Suspended Job should delete active pods when expectations are
Expand Down Expand Up @@ -727,7 +706,6 @@ func TestControllerSyncJob(t *testing.T) {
if wFinalizers && tc.podControllerError != nil {
t.Skip("Can't track status if finalizers can't be removed")
}
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.IndexedJob, tc.indexedJobEnabled)()
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.JobReadyPods, tc.jobReadyPodsEnabled)()
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.JobTrackingWithFinalizers, wFinalizers)()

Expand Down Expand Up @@ -857,7 +835,7 @@ func TestControllerSyncJob(t *testing.T) {
if actual.Status.StartTime != nil && tc.suspend {
t.Error("Unexpected .status.startTime not nil when suspend is true")
}
if actual.Status.StartTime == nil && tc.indexedJobEnabled && !tc.suspend {
if actual.Status.StartTime == nil && !tc.suspend {
t.Error("Missing .status.startTime")
}
// validate conditions
Expand Down
3 changes: 2 additions & 1 deletion pkg/features/kube_features.go
View file
Expand Up @@ -215,6 +215,7 @@ const (
// owner: @alculquicondor
// alpha: v1.21
// beta: v1.22
// stable: v1.24
//
// Allows Job controller to manage Pod completions per completion index.
IndexedJob featuregate.Feature = "IndexedJob"
Expand Down Expand Up @@ -891,7 +892,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
NetworkPolicyEndPort: {Default: true, PreRelease: featuregate.Beta},
ProcMountType: {Default: false, PreRelease: featuregate.Alpha},
TTLAfterFinished: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.25
IndexedJob: {Default: true, PreRelease: featuregate.Beta},
IndexedJob: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.26
JobTrackingWithFinalizers: {Default: true, PreRelease: featuregate.Beta},
JobReadyPods: {Default: false, PreRelease: featuregate.Alpha},
KubeletPodResources: {Default: true, PreRelease: featuregate.Beta},
Expand Down

0 comments on commit f97825e

Please sign in to comment.