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
Promote EndpointSliceTerminatingCondition to GA #113351
Promote EndpointSliceTerminatingCondition to GA #113351
Conversation
57424b2
to
5982f96
Compare
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
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 @andrewsykim!
/triage accepted |
Addresses: []string{"1.2.3.4"}, | ||
Conditions: discovery.EndpointConditions{ | ||
Ready: utilpointer.BoolPtr(true), | ||
Serving: utilpointer.BoolPtr(true), |
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.
If these were not set before, does this change the nature of the tests (defaults to false) ? Do we need to modulate these fields for full test coverage?
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.
Good point -- let me check the existing test coverage and make sure we're adding cases for terminating endpoints in places we previously assumed it was disabled
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.
Reviewed tests coverage again -- I think by addiing the serving/terminating condition checks in all the tests that previously had the gate disabled and keeping the tests for terminating endpoints when the gate was enabled keeps our test coverage more or less the same as before.
I pushed a change to clean up some left over references to the feature gate in the test names though.
@robscott are you LGTM? |
Thanks @andrewsykim! /lgtm |
@thockin needs approval |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewsykim, thockin 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 |
/hold I should follow-up on #113351 (comment) before merging, make sure we're happy with the test coverage. |
Signed-off-by: Andrew Sy Kim <andrewsy@google.com>
…atingCondition feature gate Signed-off-by: Andrew Sy Kim <andrewsy@google.com>
…iceTerminatingCondition feature gate Signed-off-by: Andrew Sy Kim <andrewsy@google.com>
…Condition feature gate when dropping disabled fields Signed-off-by: Andrew Sy Kim <andrewsy@google.com>
…minatingCondition feature gate Signed-off-by: Andrew Sy Kim <andrewsy@google.com>
5982f96
to
9e24680
Compare
/hold cancel |
/retest |
/lgtm |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Per KEP update in kubernetes/enhancements#3504, this PR promote the EndpointSliceTerminatingCondition feature to GA. This feaure has been in Beta since v1.22 and meets GA graduation criterias:
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.: