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.

Sign up for GitHub

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 JobReadyPods to beta #107476

Merged
merged 1 commit into from
Mar 29, 2022
Merged

Graduate JobReadyPods to beta #107476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/openapi-spec/swagger.json
View file
Open in desktop

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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@
"type": "integer"
},
"ready": {
"description": "The number of pods which have a Ready condition.\n\nThis field is alpha-level. The job controller populates the field when the feature gate JobReadyPods is enabled (disabled by default).",
"description": "The number of pods which have a Ready condition.\n\nThis field is beta-level. The job controller populates the field when the feature gate JobReadyPods is enabled (enabled by default).",
"format": "int32",
"type": "integer"
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/batch/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ type JobStatus struct {

// The number of active pods which have a Ready condition.
//
// This field is alpha-level. The job controller populates the field when
// the feature gate JobReadyPods is enabled (disabled by default).
// This field is beta-level. The job controller populates the field when
// the feature gate JobReadyPods is enabled (enabled by default).
// +optional
Ready *int32

Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/job/job_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ import (

// podUpdateBatchPeriod is the batch period to hold pod updates before syncing
// a Job. It is used if the feature gate JobReadyPods is enabled.
const podUpdateBatchPeriod = 500 * time.Millisecond
const podUpdateBatchPeriod = time.Second

// controllerKind contains the schema.GroupVersionKind for this controller type.
var controllerKind = batch.SchemeGroupVersion.WithKind("Job")
Expand Down
83 changes: 68 additions & 15 deletions pkg/controller/job/job_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

"github.com/google/go-cmp/cmp"
batch "k8s.io/api/batch/v1"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -2146,6 +2146,8 @@ func TestAddPod(t *testing.T) {
jm, informer := newControllerFromClient(clientset, controller.NoResyncPeriodFunc)
jm.podStoreSynced = alwaysReady
jm.jobStoreSynced = alwaysReady
// Disable batching of pod updates.
jm.podUpdateBatchPeriod = 0

job1 := newJob(1, 1, 6, batch.NonIndexedCompletion)
job1.Name = "job1"
Expand Down Expand Up @@ -2191,6 +2193,8 @@ func TestAddPodOrphan(t *testing.T) {
jm, informer := newControllerFromClient(clientset, controller.NoResyncPeriodFunc)
jm.podStoreSynced = alwaysReady
jm.jobStoreSynced = alwaysReady
// Disable batching of pod updates.
jm.podUpdateBatchPeriod = 0

job1 := newJob(1, 1, 6, batch.NonIndexedCompletion)
job1.Name = "job1"
Expand Down Expand Up @@ -2219,6 +2223,8 @@ func TestUpdatePod(t *testing.T) {
jm, informer := newControllerFromClient(clientset, controller.NoResyncPeriodFunc)
jm.podStoreSynced = alwaysReady
jm.jobStoreSynced = alwaysReady
// Disable batching of pod updates.
jm.podUpdateBatchPeriod = 0

job1 := newJob(1, 1, 6, batch.NonIndexedCompletion)
job1.Name = "job1"
Expand Down Expand Up @@ -2268,6 +2274,8 @@ func TestUpdatePodOrphanWithNewLabels(t *testing.T) {
jm, informer := newControllerFromClient(clientset, controller.NoResyncPeriodFunc)
jm.podStoreSynced = alwaysReady
jm.jobStoreSynced = alwaysReady
// Disable batching of pod updates.
jm.podUpdateBatchPeriod = 0

job1 := newJob(1, 1, 6, batch.NonIndexedCompletion)
job1.Name = "job1"
Expand Down Expand Up @@ -2295,6 +2303,8 @@ func TestUpdatePodChangeControllerRef(t *testing.T) {
jm, informer := newControllerFromClient(clientset, controller.NoResyncPeriodFunc)
jm.podStoreSynced = alwaysReady
jm.jobStoreSynced = alwaysReady
// Disable batching of pod updates.
jm.podUpdateBatchPeriod = 0

job1 := newJob(1, 1, 6, batch.NonIndexedCompletion)
job1.Name = "job1"
Expand All @@ -2321,6 +2331,8 @@ func TestUpdatePodRelease(t *testing.T) {
jm, informer := newControllerFromClient(clientset, controller.NoResyncPeriodFunc)
jm.podStoreSynced = alwaysReady
jm.jobStoreSynced = alwaysReady
// Disable batching of pod updates.
jm.podUpdateBatchPeriod = 0

job1 := newJob(1, 1, 6, batch.NonIndexedCompletion)
job1.Name = "job1"
Expand All @@ -2347,6 +2359,8 @@ func TestDeletePod(t *testing.T) {
jm, informer := newControllerFromClient(clientset, controller.NoResyncPeriodFunc)
jm.podStoreSynced = alwaysReady
jm.jobStoreSynced = alwaysReady
// Disable batching of pod updates.
jm.podUpdateBatchPeriod = 0

job1 := newJob(1, 1, 6, batch.NonIndexedCompletion)
job1.Name = "job1"
Expand Down Expand Up @@ -2392,6 +2406,8 @@ func TestDeletePodOrphan(t *testing.T) {
jm, informer := newControllerFromClient(clientset, controller.NoResyncPeriodFunc)
jm.podStoreSynced = alwaysReady
jm.jobStoreSynced = alwaysReady
// Disable batching of pod updates.
jm.podUpdateBatchPeriod = 0

job1 := newJob(1, 1, 6, batch.NonIndexedCompletion)
job1.Name = "job1"
Expand Down Expand Up @@ -2703,24 +2719,61 @@ func TestJobBackoff(t *testing.T) {
newPod.ResourceVersion = "2"

testCases := map[string]struct {
// inputs
requeues int
phase v1.PodPhase

// expectation
backoff int
requeues int
phase v1.PodPhase
jobReadyPodsEnabled bool
wantBackoff time.Duration
}{
"1st failure": {0, v1.PodFailed, 0},
"2nd failure": {1, v1.PodFailed, 1},
"3rd failure": {2, v1.PodFailed, 2},
"1st success": {0, v1.PodSucceeded, 0},
"2nd success": {1, v1.PodSucceeded, 0},
"1st running": {0, v1.PodSucceeded, 0},
"2nd running": {1, v1.PodSucceeded, 0},
"1st failure": {
requeues: 0,
phase: v1.PodFailed,
wantBackoff: 0,
},
"2nd failure": {
requeues: 1,
phase: v1.PodFailed,
wantBackoff: DefaultJobBackOff,
},
"3rd failure": {
requeues: 2,
phase: v1.PodFailed,
wantBackoff: 2 * DefaultJobBackOff,
},
"1st success": {
requeues: 0,
phase: v1.PodSucceeded,
wantBackoff: 0,
},
"2nd success": {
requeues: 1,
phase: v1.PodSucceeded,
wantBackoff: 0,
},
"1st running": {
requeues: 0,
phase: v1.PodSucceeded,
wantBackoff: 0,
},
"2nd running": {
requeues: 1,
phase: v1.PodSucceeded,
wantBackoff: 0,
},
"1st failure with pod updates batching": {
requeues: 0,
phase: v1.PodFailed,
wantBackoff: podUpdateBatchPeriod,
},
"2nd failure with pod updates batching": {
requeues: 1,
phase: v1.PodFailed,
wantBackoff: DefaultJobBackOff,
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.JobReadyPods, tc.jobReadyPodsEnabled)()
clientset := clientset.NewForConfigOrDie(&restclient.Config{Host: "", ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
manager, sharedInformerFactory := newControllerFromClient(clientset, controller.NoResyncPeriodFunc)
fakePodControl := controller.FakePodControl{}
Expand All @@ -2735,7 +2788,7 @@ func TestJobBackoff(t *testing.T) {
newPod.Status.Phase = tc.phase
manager.updatePod(oldPod, newPod)

if queue.duration.Nanoseconds() != int64(tc.backoff)*DefaultJobBackOff.Nanoseconds() {
if queue.duration.Nanoseconds() != int64(tc.wantBackoff)*DefaultJobBackOff.Nanoseconds() {
t.Errorf("unexpected backoff %v", queue.duration)
}
})
Expand Down
3 changes: 2 additions & 1 deletion pkg/features/kube_features.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ const (

// owner: @alculquicondor
// alpha: v1.23
// beta: v1.24
//
// Track the number of pods with Ready condition in the Job status.
JobReadyPods featuregate.Feature = "JobReadyPods"
Expand Down Expand Up @@ -926,7 +927,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
TTLAfterFinished: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.25
IndexedJob: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.26
JobTrackingWithFinalizers: {Default: true, PreRelease: featuregate.Beta},
JobReadyPods: {Default: false, PreRelease: featuregate.Alpha},
JobReadyPods: {Default: true, PreRelease: featuregate.Beta},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we were no longer defaulting beta features on?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beta endpoints, to start, since that is what causes 99% of user pain.

policy for beta fields hasn't changed, since they have proven far less disruptive.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

KubeletPodResources: {Default: true, PreRelease: featuregate.Beta},
LocalStorageCapacityIsolationFSQuotaMonitoring: {Default: false, PreRelease: featuregate.Alpha},
NonPreemptingPriority: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.25
Expand Down
2 changes: 1 addition & 1 deletion pkg/generated/openapi/zz_generated.openapi.go
View file
Open in desktop

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

4 changes: 2 additions & 2 deletions staging/src/k8s.io/api/batch/v1/generated.proto
View file
Open in desktop

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

4 changes: 2 additions & 2 deletions staging/src/k8s.io/api/batch/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,8 @@ type JobStatus struct {

// The number of pods which have a Ready condition.
//
// This field is alpha-level. The job controller populates the field when
// the feature gate JobReadyPods is enabled (disabled by default).
// This field is beta-level. The job controller populates the field when
// the feature gate JobReadyPods is enabled (enabled by default).
// +optional
Ready *int32 `json:"ready,omitempty" protobuf:"varint,9,opt,name=ready"`
}
Expand Down
2 changes: 1 addition & 1 deletion staging/src/k8s.io/api/batch/v1/types_swagger_doc_generated.go
View file
Open in desktop

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

Loading