[KEP 3836]: Bump to beta for 1.29

[alexanderConstantinescu]

• One-line PR description: 1.29 milestone bump for KEP - 3836

• Issue link: Kube-proxy improved ingress connectivity reliability #3836

• Other comments: none

/sig network
/assign @thockin
/assign @wojtek-t

[alexanderConstantinescu]

FYI: I didn't have time to get to two of the graduation criterias for this KEP to beta. Specifically it's missing:

enhancements/keps/sig-network/3836-kube-proxy-improved-ingress-connectivity-reliability/README.md

Line 258 in 3ef92ad

- E2E tests coded before any feature implementation is made which highlights the

and

enhancements/keps/sig-network/3836-kube-proxy-improved-ingress-connectivity-reliability/README.md

Line 261 in 3ef92ad

- Document written at https://kubernetes.io/docs/concepts/

[wojtek-t]

Few comments about the KEP itself:

As such any vendor doing this would get a heads-up during Alpha.

People are not using Alpha.
Can we make any survey of providers to check who will be hit by it?
@thockin - thoughts?

Implement change in Kube-proxy which will react to changes on the Node object and once the taint ToBeDeletedByClusterAutoscaler is placed on the Node object: start failing it's healthz state.

I started wondering about this one. Namely if we shouldn't do that in two steps...
(a) introduce livez that [reacts to the existence of taint], but doesn't touch healthz
(b) graduate to stable and give people time and chance to use something stable
(c) [potentially] only then change healthz itself

@thockin - thoguhts?

The metric: proxy_healthz_503_count mentioned in [Monitoring requirements]

I don't see that metric in the codebase - was this added?

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

has this happened? Can you update the KEP with the test and findings?

Please fill in the remaining questions in Throubleshooting section.

[alexanderConstantinescu]

Can we make any survey of providers to check who will be hit by it?

Do we have a general procedure for that? I guess we could create a Google Form and send that out to dev@kubernetes.io, but that would still feel inconclusive.

I don't see that metric in the codebase - was this added?

Yes, here: https://github.com/kubernetes/kubernetes/blob/master/pkg/proxy/metrics/metrics.go#L189-L209

The name changed during the implementation however, because we decided to have the HTTP status code be used as a label. I will update the KEP with this

has this happened?

No

Please fill in the remaining questions in Throubleshooting section.

OK, will update that now

[thockin]

People are not using Alpha.
Can we make any survey of providers to check who will be hit by it?

I went back and re-read the whole KEP and scanned the code in question. I agree that signal during alpha is ~0. That said, I can't reason any way this explodes (other than logic bugs, obviously) - but I also know that this is super subtle stuff.

What would we do differently with more information, and as much I know the subtlety, I doubt anyone would look at this plan and see the explosion-path. If this explodes, it's going to be something nobody can see until it's lit up. :(

I think the mitigations in place are as low-risk as we can get with this, short of flipping the polarity entirely, which will almost guarantee low adoption.

Implement change in Kube-proxy which will react to changes on the Node object and once the taint ToBeDeletedByClusterAutoscaler is placed on the Node object: start failing it's healthz state.

I started wondering about this one. Namely if we shouldn't do that in two steps...
(a) introduce livez that [reacts to the existence of taint], but doesn't touch healthz
(b) graduate to stable and give people time and chance to use something stable
(c) [potentially] only then change healthz itself

This is the "flip the polarity" path, but it would not be livez. More like "add /healthz-v2; get people to use it; deprecate old /healthz". That is the safer path, for sure. If we go that way, I don't think we will ever actually get rid of or fix the old healthz, and all of the existing clusters in the world will get no benefits. Providers will be reluctant to adopt the new path because then they have to support the old AND the new, and who wants to do that?

The lack of in-place updates sort of kills the value of this KEP, IMO, and the risk is not proportional to the reaction. That said, this is not such a high-urgency thing that we should take on dramatic risk. If you think I am wrong on the risk level, then we can try to go through a new URL endpoint and see if we can drive adoption. That means figuring ouot, for example, if cloud-providers support updates to healthcheck configs.

[wojtek-t]

Thanks Tim - I guess I convinced by your explanation above. Let's just update the troubleshooting section as I requested and let's proceed with the plan

[thockin]

/approve
/lgtm

Assuming no directional change.

/hold

to confirm

[wojtek-t]

The one described in Risks/Mitigations:

Vendors of Kubernetes which deploy Kube-proxy and specify a livenessProbe targeting /healthz are expected to start seeing a CrashLooping Kube-proxy when the Node gets tainted with ToBeDeletedByClusterAutoscaler. This is because: if we modify /healthz to fail when this taint gets added on the Node, then the livenessProbe will fail, causing the Kubelet to restart the Pod until the Node is deleted. As far as we can tell, no vendor set livenessProbe, nor does kubeadm, so the risk is low.

is actually relevant and we should probably mention it here [with bullet points from the template]

[alexanderConstantinescu]

Updated

[wojtek-t]

Thanks Tim - I guess I convinced by your explanation above. Let's just update the troubleshooting section as I requested and let's proceed with the plan

[thockin]

Regarding the 2 criteria - docs and tests can be written after KEP freeze, if you feel comfortable with the idea of getting to it?

[thockin]

Thanks!

/lgtm
/approve

[wojtek-t]

/lgtm
/approve PRR

Thanks!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexanderConstantinescu, 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