Update CHANGELOG.md to note node readiness after #43474 #44000
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
This change is |
k8s-github-robot
added
size/XS
calebamiles
added
release-note-none
|
cc @mikedanese |
CHANGELOG.md
Outdated
| [This change](https://github.com/kubernetes/kubernetes/pull/43474) ensures | ||
| kubelet does not start pods that require networking before | ||
| [networking is ready](https://github.com/kubernetes/kubernetes/issues/43014). | ||
| This change may require changes to clients if the client gates pod scheduling |
There was a problem hiding this comment.
This sentence assumes users are using the go client. I would instead say something like "Previously pods requiring just host networking could be scheduled and run successfully. This is no longer possible. Note that currently DaemonSets will be scheduled regardless of node readiness."
There was a problem hiding this comment.
I agree that it's useful to call out that DaemonSets shouldn't be affected
There was a problem hiding this comment.
I would combine Joe's text and mine.
There was a problem hiding this comment.
@jbeda I don't think that statement is accurate. Previously, all pods would be scheduled, some (hostNetwork:true) would run successfully, and the rest would fail over and over and over in a loop forever because networking wasn't ready.
Kubelet still allows hostNetwork:true pods to be run on the node even if the node isn't ready (AFAIK) so "This is no longer possible" isn't correct.
It's simply that clients that assume node readiness means no pods can be scheduled can no longer assume that.
There was a problem hiding this comment.
I think @bgrant0607 has a better summary than I could write here, so I'll take his.
There was a problem hiding this comment.
I'm okay with the text as is, but right now no pods that go through the regular scheduling path (i.e. all except DaemonSet) will land on the machine. This is a break in behavior and is the reason we had to remove the "test deployment" check from kubeadm.
So -- if someone relied on pods being scheduled and working (with host networking) that is now broken and is no longer possible. The scheduler doesn't have a carve out for host networking pods on nodes with broken networking.
Previously non host-networking pods wouldn't launch but would be scheduled. Now they won't be scheduled at all. So they are now a different type of broken.
This is all pretty subtle and I doubt that many folks actually were relying on host networking pods working with broken CNI, but you never know.
CHANGELOG.md
Outdated
| @@ -642,6 +642,15 @@ Anyway, the cluster should get back to the proper size after 10 min. | |||
| the new CRI + CNI code path. | |||
| * If you are using plugins other than `bridge`, make sure you have | |||
| updated custom plugins to the latest version that is compatible. | |||
| * **CNI plugins now affect node readiness** | |||
| * Kubelet will now block node readiness until a CNI configuration file is | |||
| present in /etc/cni/net.d or a given custom CNI configuration path. | |||
There was a problem hiding this comment.
Use code highlighting (`) for /etc/cni/net.d?
CHANGELOG.md
Outdated
| [This change](https://github.com/kubernetes/kubernetes/pull/43474) ensures | ||
| kubelet does not start pods that require networking before | ||
| [networking is ready](https://github.com/kubernetes/kubernetes/issues/43014). | ||
| This change may require changes to clients if the client gates pod scheduling |
There was a problem hiding this comment.
I would write:
This change may require changes to clients that gate pod creation and/or scheduling on the node condition type Ready status being True for pods that need to run prior to the network being ready.
so that the description is in terms of the API rather than our own internal client code, and more clearly differentiates problematic scenarios from the intended behavior.
There was a problem hiding this comment.
Updated with your text, thanks!
dcbw
force-pushed
the
node-readiness-changelog
branch
from
bc6de76
to
75ad746
Compare
|
@bgrant0607 @jbeda PTAL, thanks! |
|
@dcbw: The following test(s) failed:
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
|
/lgtm We can add more text later if further clarification is needed. |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bgrant0607, dcbw
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
the
approved
|
Automatic merge from submit-queue |
No description provided.