KEP-1669: promote ProxyTerminatingEndpoints to Beta #3505

andrewsykim · 2022-09-12T19:35:11Z

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

One-line PR description:

Promote ProxyTerminatingEndpoints feature to Beta (alpha since v1.24). #1669

Issue link:

Other comments:

andrewsykim · 2022-09-12T19:35:42Z

keps/sig-network/1669-proxy-terminating-endpoints/kep.yaml

@@ -21,16 +21,17 @@ see-also:
  - https://github.com/kubernetes/kubernetes/issues/85643

 # The target maturity stage in the current dev cycle for this KEP.
-stage: alpha
+stage: beta


Actual KEP changes are light cause we added Beta criteria in previous releases:

#### Beta * E2E tests are in place, exercising all permutations of internalTrafficPolicy and externalTrafficPolicy. * Metrics to publish how many Services/Endpoints are routing traffic to terminating endpoints.

andrewsykim · 2022-09-13T15:00:53Z

/assign @thockin @wojtek-t

wojtek-t

Couple comments about the PRR.

wojtek-t · 2022-09-28T07:13:16Z

keps/sig-network/1669-proxy-terminating-endpoints/README.md

@@ -252,7 +253,7 @@ gauge if a rollback is necessary.

 ###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?

-Upgrade->downgrade->upgrade path has not been tested yet. We may want to require this for beta or GA.
+Upgrade->downgrade->upgrade testing (manual or automated) will be required for Beta. If tested manually, the steps will be documented in this KEP.


I'm fine with this for now - but please update the KEP with it before graduating the feature to Beta in k/k (and send the PR doing that to me too).

Yup, will do.

wojtek-t · 2022-09-28T07:24:31Z

keps/sig-network/1669-proxy-terminating-endpoints/README.md

@@ -269,7 +270,7 @@ regardless of their termination state. If this is undesired, workloads should be
 ###### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service?

 - [X] Metrics
-  - Metric name: TBD
+  - Metric name: `sync_proxy_rules_no_endpoints_total`


Hmm - I was looking for this metric definition in k/k and I can't found it. Does this already exist?

Sorry this is a typo. It's sync_proxy_rules_no_local_endpoints_total https://github.com/kubernetes/kubernetes/blob/master/pkg/proxy/metrics/metrics.go#L152-L162

wojtek-t · 2022-09-28T07:25:16Z

keps/sig-network/1669-proxy-terminating-endpoints/README.md

@@ -185,6 +185,7 @@ All existing E2E tests for Services should continue to pass.

 * E2E tests are in place, exercising all permutations of internalTrafficPolicy and externalTrafficPolicy.
 * Metrics to publish how many Services/Endpoints are routing traffic to terminating endpoints.
+* Rollback testing (manual or automated)


Sorry - can't comment on the untouched lines - so commenting here.

L228: were those tests added?

L250: were those metrics added?

L362: Can you fill this in? [I guess it's probably as simple as "update workloads to properly set readiness probe"]

Also - please adjust the Testing section to the new format.

L228: were those tests added?

Yes, some examples:
https://github.com/kubernetes/kubernetes/blob/d436f5d0b7eb87f78eb31c12466e2591c24eef59/pkg/proxy/iptables/proxier_test.go#L6114
https://github.com/kubernetes/kubernetes/blob/6e9845f766e4d34620835aaa1e5f864211471a50/pkg/proxy/ipvs/proxier_test.go#L5493

L250: were those metrics added?

Yes, this should be accomplished with sync_proxy_rules_no_local_endpoints_total on a per-node basis.

L362: Can you fill this in? [I guess it's probably as simple as "update workloads to properly set readiness probe"]

Yeah makes sense, sorry I missed this.

Will include these links in the PRR and use the new format.

Updated, PTAL

Yes, some examples:
https://github.com/kubernetes/kubernetes/blob/d436f5d0b7eb87f78eb31c12466e2591c24eef59/pkg/proxy/iptables/proxier_test.go#L6114
https://github.com/kubernetes/kubernetes/blob/6e9845f766e4d34620835aaa1e5f864211471a50/pkg/proxy/ipvs/proxier_test.go#L5493

Can you please update that in the PR?

Yeah I already included them in the Test Plan section using the new format: https://github.com/kubernetes/enhancements/blob/6ddd543896cf6c201352ba302f4354c0e89e8165/keps/sig-network/1669-proxy-terminating-endpoints/README.md#unit-tests

Heh...
Can you just add here sth like "(linked in Test Plan section)".
?

Sure, updated

Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

…re promotion to Beta Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

…points_total and fix typo Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

wojtek-t

One last minor comment - other than that LGTM.

wojtek-t · 2022-09-29T07:48:14Z

keps/sig-network/1669-proxy-terminating-endpoints/README.md

@@ -185,6 +185,7 @@ All existing E2E tests for Services should continue to pass.

 * E2E tests are in place, exercising all permutations of internalTrafficPolicy and externalTrafficPolicy.
 * Metrics to publish how many Services/Endpoints are routing traffic to terminating endpoints.
+* Rollback testing (manual or automated)


Yes, some examples:
https://github.com/kubernetes/kubernetes/blob/d436f5d0b7eb87f78eb31c12466e2591c24eef59/pkg/proxy/iptables/proxier_test.go#L6114
https://github.com/kubernetes/kubernetes/blob/6e9845f766e4d34620835aaa1e5f864211471a50/pkg/proxy/ipvs/proxier_test.go#L5493

Can you please update that in the PR?

…ests Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

… not being met to determine the problem?' Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

wojtek-t · 2022-09-30T08:15:42Z

/label tide/merge-method-squash
/lgtm
/approve PRR

For SIG approval
/assign @thockin

thockin · 2022-10-02T21:46:52Z

SIG:

/lgtm
/approve

k8s-ci-robot · 2022-10-02T21:46:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewsykim, thockin, wojtek-t

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:

~~keps/prod-readiness/OWNERS~~ [wojtek-t]
~~keps/sig-network/OWNERS~~ [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* KEP-1669: promote ProxyTerminatingEndpoints to Beta Signed-off-by: Andrew Sy Kim <andrewsy@google.com> * KEP-1669: add note that upgrade/downgrade testing should be done before promotion to Beta Signed-off-by: Andrew Sy Kim <andrewsy@google.com> * KEP-1669: add more details about metric sync_proxy_rules_no_local_endpoints_total and fix typo Signed-off-by: Andrew Sy Kim <andrewsy@google.com> * KEP-1669: use the new test plan format, including links to existing tests Signed-off-by: Andrew Sy Kim <andrewsy@google.com> * KEP-1669: answer PRR question 'What steps should be taken if SLOs are not being met to determine the problem?' Signed-off-by: Andrew Sy Kim <andrewsy@google.com> Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 12, 2022

k8s-ci-robot requested review from caseydavenport and dcbw September 12, 2022 19:35

k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/network Categorizes an issue or PR as relevant to SIG Network. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 12, 2022

andrewsykim commented Sep 12, 2022

View reviewed changes

andrewsykim force-pushed the kep-1669 branch 2 times, most recently from 8fdc259 to affb965 Compare September 12, 2022 19:40

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 12, 2022

andrewsykim mentioned this pull request Sep 12, 2022

KEP-1672: promote EndpointSliceTerminatingCondition feature to GA #3504

Merged

k8s-ci-robot assigned thockin and wojtek-t Sep 13, 2022

andrewsykim force-pushed the kep-1669 branch 2 times, most recently from a60704e to a8ede61 Compare September 27, 2022 15:06

wojtek-t reviewed Sep 28, 2022

View reviewed changes

andrewsykim added 2 commits September 28, 2022 06:53

KEP-1669: promote ProxyTerminatingEndpoints to Beta

7da8ec9

Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

KEP-1669: add note that upgrade/downgrade testing should be done befo…

df10814

…re promotion to Beta Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

andrewsykim force-pushed the kep-1669 branch from a8ede61 to 57fb5bb Compare September 28, 2022 19:15

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 28, 2022

KEP-1669: add more details about metric sync_proxy_rules_no_local_end…

9ca69f5

…points_total and fix typo Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

andrewsykim force-pushed the kep-1669 branch from 57fb5bb to 6ddd543 Compare September 28, 2022 19:49

wojtek-t reviewed Sep 29, 2022

View reviewed changes

andrewsykim added 2 commits September 29, 2022 09:09

KEP-1669: use the new test plan format, including links to existing t…

e4f725b

…ests Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

KEP-1669: answer PRR question 'What steps should be taken if SLOs are…

6fedf3b

… not being met to determine the problem?' Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

andrewsykim force-pushed the kep-1669 branch from 6ddd543 to 6fedf3b Compare September 29, 2022 13:09

k8s-ci-robot added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Sep 30, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 2, 2022

k8s-ci-robot merged commit 00b3eaf into kubernetes:master Oct 2, 2022

k8s-ci-robot added this to the v1.26 milestone Oct 2, 2022

andrewsykim mentioned this pull request Oct 4, 2022

Proxy Terminating Endpoints #1669

Closed

andrewsykim mentioned this pull request Oct 26, 2022

promote ProxyTerminatingEndpoints feature to Beta kubernetes/kubernetes#113363

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KEP-1669: promote ProxyTerminatingEndpoints to Beta #3505

KEP-1669: promote ProxyTerminatingEndpoints to Beta #3505

andrewsykim commented Sep 12, 2022 •

edited

Loading

andrewsykim Sep 12, 2022

andrewsykim commented Sep 13, 2022

wojtek-t left a comment

wojtek-t Sep 28, 2022

andrewsykim Sep 28, 2022

wojtek-t Sep 28, 2022

andrewsykim Sep 28, 2022

wojtek-t Sep 28, 2022

wojtek-t Sep 28, 2022

andrewsykim Sep 28, 2022 •

edited

Loading

andrewsykim Sep 28, 2022

wojtek-t Sep 29, 2022

andrewsykim Sep 29, 2022

wojtek-t Sep 29, 2022

andrewsykim Sep 29, 2022

wojtek-t left a comment

wojtek-t Sep 29, 2022

wojtek-t commented Sep 30, 2022

thockin commented Oct 2, 2022

k8s-ci-robot commented Oct 2, 2022

KEP-1669: promote ProxyTerminatingEndpoints to Beta #3505

KEP-1669: promote ProxyTerminatingEndpoints to Beta #3505

Conversation

andrewsykim commented Sep 12, 2022 • edited Loading

Choose a reason for hiding this comment

andrewsykim commented Sep 13, 2022

wojtek-t left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewsykim Sep 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wojtek-t left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wojtek-t commented Sep 30, 2022

thockin commented Oct 2, 2022

k8s-ci-robot commented Oct 2, 2022

andrewsykim commented Sep 12, 2022 •

edited

Loading

andrewsykim Sep 28, 2022 •

edited

Loading