Fix serialization of EnforceNodeAllocatable #44606
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
This change is |
k8s-github-robot
added
kind/api-change
ivan4th
force-pushed
the
fix-serialization-of-enforce-node-allocatable
branch
2 times, most recently
from
2a13305
to
a13a31e
Compare
k8s-github-robot
added
size/S
k8s-github-robot
added
the
needs-rebase
yujuhong
added
release-note
|
Change looks fine to me, but adding @mtaufen since he's the authority of kubelet config. |
|
/lgtm This looks good to me for now. But keep in mind you really shouldn't be using the existing dynamic config implementation for anything other than testing (it's still alpha). It will also change significantly in 1.7: kubernetes/enhancements#281 Sorry for lateness on this review. |
k8s-ci-robot
added
the
lgtm
EnforceNodeAllocatable being `nil` and `[]` are treated in different
ways by kubelet. Namely, `nil` is replaced with `[]string{"pods"}` by
the defaulting mechanism.
E.g. if you run kubelet in Docker-in-Docker environment
you may need to run it with the following options:
`--cgroups-per-qos=false --enforce-node-allocatable=`
(this corresponds to EnforceNodeAllocatable being empty array and not
null) If you then grab kubelet configuration via /configz and try to
reuse it for dynamic kubelet config, kubelet will think that
EnforceNodeAllocatable is null, failing to run in the
Docker-in-Docker environment.
Encountered this while updating Virtlet for Kubernetes 1.6
(the dev environment is based on kubeadm-dind-cluster)
ivan4th
force-pushed
the
fix-serialization-of-enforce-node-allocatable
branch
from
a13a31e
to
ef85747
Compare
k8s-github-robot
added
approved
|
@mtaufen thanks for the comment. I've rebased the PR. We use dynamic kubelet config during the deployment of our CRI implementation (Virtlet), but it'll require some updates anyway to be compatible with forthcoming k8s 1.7, and as of now it looks like we'll be able to use the new dynamic kubelet config mechanism, too. |
|
@k8s-bot kubemark e2e test this |
|
@k8s-bot gce etcd3 e2e test this |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ivan4th, mtaufen
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
@ivan4th The Kubelet's configuration API is still alpha-grade, and likely to undergo violent breaking changes. At present, there is no guarantee that any field you are dynamically updating will still be dynamic in any future version. For this reason, I caution against using dynamic config for anything other than tests (even there, please use it sparingly, as I'm the one who gets to update everyone else's code when the API changes). |
|
@mtaufen thanks! Well, our idea is giving users a simple way to try out Virtlet. It's CRI implementation that itself runs as a DaemonSet and in case of "bootstrap" mode (not intended for production!) it uses dynamic kubelet config to set |
|
(running CRI impl as a DaemonSet is possible thanks to the CRI proxy mechanism that makes it easy to have multiple runtimes on the node) |
|
Automatic merge from submit-queue (batch tested with PRs 44606, 46038) |
EnforceNodeAllocatable being
niland[]are treated in differentways by kubelet. Namely,
nilis replaced with[]string{"pods"}bythe defaulting mechanism.
E.g. if you run kubelet in Docker-in-Docker environment
you may need to run it with the following options:
--cgroups-per-qos=false --enforce-node-allocatable=(this corresponds to EnforceNodeAllocatable being empty array and not
null) If you then grab kubelet configuration via /configz and try to
reuse it for dynamic kubelet config, kubelet will think that
EnforceNodeAllocatable is null, failing to run in the
Docker-in-Docker environment.
Encountered this while updating Virtlet for Kubernetes 1.6
(the dev environment is based on kubeadm-dind-cluster)