Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graduate IndexedJob to stable #107395

Merged
merged 1 commit into from Mar 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/openapi-spec/swagger.json

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
Expand Up @@ -275,7 +275,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
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
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
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
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
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
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
Expand Up @@ -214,6 +214,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 @@ -887,7 +888,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