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
Remove GA featuregates IndexedJob and SuspendJob #112589
Remove GA featuregates IndexedJob and SuspendJob #112589
Conversation
@SataQiu: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/sig apps |
@@ -181,7 +181,6 @@ func validateJobSpec(spec *batch.JobSpec, fldPath *field.Path, opts apivalidatio | |||
if spec.TTLSecondsAfterFinished != nil { | |||
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.TTLSecondsAfterFinished), fldPath.Child("ttlSecondsAfterFinished"))...) | |||
} | |||
// CompletionMode might be nil when IndexedJob feature gate is disabled. | |||
if spec.CompletionMode != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although spec.CompletionMode
is expected to be initialized by SetDefaults_Job()
:
kubernetes/pkg/apis/batch/v1/defaults.go
Lines 47 to 50 in 6820a38
if obj.Spec.CompletionMode == nil { | |
mode := batchv1.NonIndexedCompletion | |
obj.Spec.CompletionMode = &mode | |
} |
It is always a good practice to check whether a pointer is nil, just like we did with the other fields.
For example:
kubernetes/pkg/apis/batch/v1/defaults.go
Lines 40 to 42 in 6820a38
if obj.Spec.BackoffLimit == nil { | |
obj.Spec.BackoffLimit = utilpointer.Int32Ptr(6) | |
} |
kubernetes/pkg/apis/batch/validation/validation.go
Lines 178 to 180 in 6820a38
if spec.BackoffLimit != nil { | |
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.BackoffLimit), fldPath.Child("backoffLimit"))...) | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason is that an old Job that was never updated might have a nil CompletionMode.
But I don't think we need a comment for that :)
/assign @liggitt |
/lgtm
The KEP links: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot.
Can you also submit a PR to k/website?
@@ -181,7 +181,6 @@ func validateJobSpec(spec *batch.JobSpec, fldPath *field.Path, opts apivalidatio | |||
if spec.TTLSecondsAfterFinished != nil { | |||
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.TTLSecondsAfterFinished), fldPath.Child("ttlSecondsAfterFinished"))...) | |||
} | |||
// CompletionMode might be nil when IndexedJob feature gate is disabled. | |||
if spec.CompletionMode != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason is that an old Job that was never updated might have a nil CompletionMode.
But I don't think we need a comment for that :)
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, SataQiu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The Feature Gates docs page does not support removing feature gate. And the reference pages are automatically generated, so a manual PR does not seem to be required. |
Ah, that might be new |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Remove GA featuregates IndexedJob and SuspendJob
IndexedJob and SuspendJob were GA since 1.24, and can be removed in 1.26.
IndexedJob GA at: 2c5d0a2
SuspendJob GA at: b2d2ec9
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: