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

Jump to bottom

Promote EndpointSliceTerminatingCondition to Beta #103596

Merged
merged 2 commits into from Jul 9, 2021
Merged

Promote EndpointSliceTerminatingCondition to Beta #103596

merged 2 commits into from Jul 9, 2021

Conversation

andrewsykim
Copy link
Member

Signed-off-by: Andrew Sy Kim kim.andrewsy@gmail.com

What type of PR is this?

/kind feature

What this PR does / why we need it:

As discussed during SIG Network today, the EndpointSliceTerminatingCondiiton feature gate can be promoted independent of the ProxyTerminatingEndpoints feature. Both feature gates are required for kube-proxy to handle terminating endpoints, but the former can be enabled in advance to enable the API field for other consumers.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Promote EndpointSliceTerminatingCondition to Beta. This enables the 'terminating' and 'serving' conditions for EndpointSlice by default.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@andrewsykim
promote EndpointSliceTerminatingCondition to Beta
826a521 
Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jul 8, 2021
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2021
@andrewsykim
Copy link
Member Author

/sig network
/priority important-soon
/triage accepted

@k8s-ci-robot k8s-ci-robot added sig/network Categorizes an issue or PR as relevant to SIG Network. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 8, 2021
@andrewsykim
test/intergration/endpointslice: add tests for endpointslice terminat…
fd0db61 
…ing condition

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>
andrewsykim
andrewsykim commented Jul 8, 2021
View reviewed changes
test/integration/endpointslice/endpointsliceterminating_test.go
// TestEndpointSliceTerminating tests that terminating pods are NOT included in EndpointSlice when
// the feature gate EndpointSliceTerminatingCondition is off. If the gate is on, it tests that
// terminating endpoints are included but with the correct conditions set for ready, serving and terminating.
func TestEndpointSliceTerminating(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I think these integration tests are enough but happy to add e2es if we think it's worthwhile here. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Agree that these seem sufficiently thorough.

@andrewsykim
Copy link
Member Author

/assign @thockin @robscott

robscott
robscott reviewed Jul 8, 2021
View reviewed changes
Copy link
Member

@robscott robscott left a comment

Choose a reason for hiding this comment

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

Thanks for the work on this!

/lgtm

test/integration/endpointslice/endpointsliceterminating_test.go
}

// Validate that terminating the endpoint will result in the expected endpoints in EndpointSlice.
// Use a stricter timeout value here since we should try to catch regressions in the time it takes to remove terminated endpoints.
Copy link
Member

Choose a reason for hiding this comment

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

Is 10 seconds strict here? Do we think there's any chance this will result in flakes?

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 8, 2021
thockin
thockin reviewed Jul 8, 2021
View reviewed changes
Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Thanks!

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sftim
Copy link
Contributor

sftim commented Jul 8, 2021

If bumping this, remember to update the Kubernetes docs to match.

@sftim sftim mentioned this pull request Jul 8, 2021
Merged
@andrewsykim
Copy link
Member Author

/retest

@puerco
Copy link
Member

puerco commented Jul 9, 2021

/milestone v1.22

@k8s-ci-robot k8s-ci-robot added this to the v1.22 milestone Jul 9, 2021
@k8s-ci-robot k8s-ci-robot merged commit 2965224 into kubernetes:master Jul 9, 2021
tengqm added a commit to tengqm/website that referenced this pull request Jul 11, 2021
@tengqm
Batch update feature gates for 1.22
4a4f8fc 
Here is a second batch for feature gate updates in 1.22.

- CPUManagerPolicyOptions    kubernetes/kubernetes#101432
- ControllerManagerLeaderMigration  kubernetes/kubernetes#103533
- DelegateFSGroupToCSIDriver   kubernetes/kubernetes#103244
- DynamicKubeletConfig    kubernetes/kubernetes#102966
- EndpointSliceProxying  kubernetes/kubernetes#103451
- EndpointSliceTerminatingCondition  kubernetes/kubernetes#103596
- HugePageStorageMediumSize    kubernetes/kubernetes#99144
- JobTrackingWithFinalizers   kubernetes/kubernetes#98817
  (also tracked in kubernetes#28841, can rebase).
- MemoryQoS   kubernetes/kubernetes#102970
- NodeSwap  kubernetes/kubernetes#102823, kubernetes/kubernetes#103553
- ServiceInternalTrafficPolicy  kubernetes/kubernetes#103462
- StatefulSetAutoDeletePVC kubernetes/kubernetes#99378
- WindowsEndpointSliceProxying   kubernetes/kubernetes#103451

Some of these needs more detailed documentation.
@tengqm tengqm mentioned this pull request Jul 11, 2021
Merged
tengqm added a commit to tengqm/website that referenced this pull request Jul 18, 2021
@tengqm
Batch update feature gates for 1.22
bee2491 
Here is a second batch for feature gate updates in 1.22.

- CPUManagerPolicyOptions    kubernetes/kubernetes#101432
- ControllerManagerLeaderMigration  kubernetes/kubernetes#103533
- DelegateFSGroupToCSIDriver   kubernetes/kubernetes#103244
- DynamicKubeletConfig    kubernetes/kubernetes#102966
- EndpointSliceProxying  kubernetes/kubernetes#103451
- EndpointSliceTerminatingCondition  kubernetes/kubernetes#103596
- HugePageStorageMediumSize    kubernetes/kubernetes#99144
- JobTrackingWithFinalizers   kubernetes/kubernetes#98817
  (also tracked in kubernetes#28841, can rebase).
- MemoryQoS   kubernetes/kubernetes#102970
- NodeSwap  kubernetes/kubernetes#102823, kubernetes/kubernetes#103553
- ServiceInternalTrafficPolicy  kubernetes/kubernetes#103462
- StatefulSetAutoDeletePVC kubernetes/kubernetes#99378
- WindowsEndpointSliceProxying   kubernetes/kubernetes#103451

Some of these needs more detailed documentation.
tengqm added a commit to tengqm/website that referenced this pull request Jul 21, 2021
@tengqm
Batch update feature gates for 1.22
fda406e 
Here is a second batch for feature gate updates in 1.22.

- CPUManagerPolicyOptions    kubernetes/kubernetes#101432
- ControllerManagerLeaderMigration  kubernetes/kubernetes#103533
- DelegateFSGroupToCSIDriver   kubernetes/kubernetes#103244
- DynamicKubeletConfig    kubernetes/kubernetes#102966
- EndpointSliceProxying  kubernetes/kubernetes#103451
- EndpointSliceTerminatingCondition  kubernetes/kubernetes#103596
- HugePageStorageMediumSize    kubernetes/kubernetes#99144
- JobTrackingWithFinalizers   kubernetes/kubernetes#98817
  (also tracked in kubernetes#28841, can rebase).
- MemoryQoS   kubernetes/kubernetes#102970
- ServiceInternalTrafficPolicy  kubernetes/kubernetes#103462
- StatefulSetAutoDeletePVC kubernetes/kubernetes#99378
- WindowsEndpointSliceProxying   kubernetes/kubernetes#103451

Some of these needs more detailed documentation.
tengqm added a commit to tengqm/website that referenced this pull request Jul 22, 2021
@tengqm
Batch update feature gates for 1.22
e1e87c2 
Here is a second batch for feature gate updates in 1.22.

- CPUManagerPolicyOptions    kubernetes/kubernetes#101432
- ControllerManagerLeaderMigration  kubernetes/kubernetes#103533
- DynamicKubeletConfig    kubernetes/kubernetes#102966
- EndpointSliceProxying  kubernetes/kubernetes#103451
- EndpointSliceTerminatingCondition  kubernetes/kubernetes#103596
- HugePageStorageMediumSize    kubernetes/kubernetes#99144
- JobTrackingWithFinalizers   kubernetes/kubernetes#98817
  (also tracked in kubernetes#28841, can rebase).
- MemoryQoS   kubernetes/kubernetes#102970
- ServiceInternalTrafficPolicy  kubernetes/kubernetes#103462
- StatefulSetAutoDeletePVC kubernetes/kubernetes#99378
- WindowsEndpointSliceProxying   kubernetes/kubernetes#103451

Some of these needs more detailed documentation.
tengqm added a commit to tengqm/website that referenced this pull request Jul 23, 2021
@tengqm
Batch update feature gates for 1.22
5239adf 
Here is a second batch for feature gate updates in 1.22.

- CPUManagerPolicyOptions    kubernetes/kubernetes#101432
- ControllerManagerLeaderMigration  kubernetes/kubernetes#103533
- DynamicKubeletConfig    kubernetes/kubernetes#102966
- EndpointSliceProxying  kubernetes/kubernetes#103451
- EndpointSliceTerminatingCondition  kubernetes/kubernetes#103596
- HugePageStorageMediumSize    kubernetes/kubernetes#99144
- JobTrackingWithFinalizers   kubernetes/kubernetes#98817
  (also tracked in kubernetes#28841, can rebase).
- ServiceInternalTrafficPolicy  kubernetes/kubernetes#103462
- WindowsEndpointSliceProxying   kubernetes/kubernetes#103451

Some of these needs more detailed documentation.
@andrewsykim andrewsykim mentioned this pull request Oct 26, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robscott robscott robscott left review comments

@thockin thockin thockin left review comments

@brendandburns brendandburns Awaiting requested review from brendandburns

@dchen1107 dchen1107 Awaiting requested review from dchen1107

Assignees

@robscott robscott

@thockin thockin

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.22
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@andrewsykim @k8s-ci-robot @sftim @puerco @robscott @thockin