remove tracking annotation from validation and webhooks #117633
Conversation
|
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 |
5442468
to
a832083
Compare
a832083
to
6a4cf35
Compare
|
https://github.com/kubernetes/enhancements/tree/master/keps/sig-apps/2307-job-tracking-without-lingering-pods#deprecation states that we are removing the feature toggle. I also think we should remove the code around validating these annotation or have test cases for it. |
|
/retest |
|
@alculquicondor we discussed this offline. Maybe let’s see if this is a good idea to get into this release? |
There was a problem hiding this comment.
For a change like this, we need to think about it across 2 versions:
- If a job is created in k8s v1.28, would it pass update validations in v1.27?
In 1.28, it would be possible to create a job with the annotation, because we completely ignore it from validation. As a result, this annotation can have any value.
The validation in 1.27 allows the annotation on update if it already has it. The value of the annotation is irrelevant.
- If a job is created in k8s v1.28, would it behave the same in v1.27?
In v1.27, jobs are created with the annotation (for backwards compatibility with v1.26), but the job controller actually ignores it.
So a job created in v1.28 without the annotation behaves the same in v1.27. We don't care about n-2 skew.
In conclusion, this PR is safe.
/lgtm
/approve
|
LGTM label has been added. Git tree hash: 0e93836954c5a25da054dc7bafefa21f24bddbde
|
|
/label api-review |
|
thanks for the detailed reasoning in #117633 (review) /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor, kannon92, liggitt 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 |
|
@kannon92 could you update the KEP to reflect that all the deprecation steps where completed? https://github.com/kubernetes/enhancements/tree/master/keps/sig-apps/2307-job-tracking-without-lingering-pods |
Sure thing. |
What type of PR is this?
/kind cleanup
/kind deprecation
What this PR does / why we need it:
In 1.27, we removed the legacy tracking job in the job controller. We no longer check the annotations so we want to now remove validation in 1.28.
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.: