refactor: simplify kubelet systemd service #3167
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3167 +/- ##
=======================================
Coverage 71.23% 71.24%
=======================================
Files 147 147
Lines 25745 25751 +6
=======================================
+ Hits 18340 18346 +6
Misses 6268 6268
Partials 1137 1137
Continue to review full report at Codecov.
|
jackfrancis
force-pushed
the
kubelet-systemd-simplify
branch
2 times, most recently
from
15f3c71
to
f413af8
Compare
| {{/* This is a partial workaround to this upstream Kubernetes issue: */}} | ||
| {{/* https://github.com/kubernetes/kubernetes/issues/41916#issuecomment-312428731 */}} | ||
|
|
||
| ExecStartPre=-/sbin/ebtables -t nat --list |
There was a problem hiding this comment.
I vote we get rid of this extra stdout at the beginning of the kubelet startup. There are better ways to accomplish that, if we want, and journalctl -u kubelet reporting that is misleading as the iptables/ebtables rules do not originate from the kubelet runtime.
| ExecStartPre=/bin/bash -c "if [ $(mount | grep \"/var/lib/kubelet\" | wc -l) -le 0 ] ; then /bin/mount --bind /var/lib/kubelet /var/lib/kubelet ; fi" | ||
| ExecStartPre=/bin/mount --make-shared /var/lib/kubelet | ||
| {{/* This is a partial workaround to this upstream Kubernetes issue: */}} | ||
| {{/* https://github.com/kubernetes/kubernetes/issues/41916#issuecomment-312428731 */}} |
There was a problem hiding this comment.
I wasn't able to find what in here relates to the above (closed) issue, so I suspect it's an obsolete comment.
| ExecStart=/usr/local/bin/kubelet \ | ||
| --enable-server \ | ||
| --node-labels="${KUBELET_NODE_LABELS}" \ |
There was a problem hiding this comment.
As far as I can tell we want to keep the --node-labels configuration like this so we can declare the value of KUBELET_NODE_LABELS at ARM deployment runtime. The KUBELET_CONFIG env var is statically defined in the cloud-init template at generation time, so arbitrary ARM variable/parameters injection isn't possible via that vector.
| PRIVATE_IP=$(hostname -i | cut -d" " -f1) | ||
| {{- if IsMasterVirtualMachineScaleSets}} | ||
| PRIVATE_IP=$(hostname -i | cut -d" " -f1) | ||
| sed -i "s|<SERVERIP>|https://$PRIVATE_IP:443|g" "/var/lib/kubelet/kubeconfig" | ||
| {{end}} | ||
| {{- if gt .MasterProfile.Count 1}} | ||
| # Redirect ILB (4443) traffic to port 443 (ELB) in the prerouting chain | ||
| {{- /* Redirect ILB (4443) traffic to port 443 (ELB) in the prerouting chain */}} |
There was a problem hiding this comment.
While I'm in here...
jackfrancis
changed the title
| ExecStart=/usr/local/bin/kubelet \ | ||
| --enable-server \ |
There was a problem hiding this comment.
--enable-server is the default, we don't need to explicitly set it
ref: https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis, mboersma 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 |
jackfrancis
force-pushed
the
kubelet-systemd-simplify
branch
from
7e65e46
to
c185b4d
Compare
|
New changes are detected. LGTM label has been removed. |
Reason for Change:
This PR dusts off the cobwebs from the kubelet systemd implementation. Specifically:
ExecStartPrestrings into the pre-existing/opt/azure/containers/kubelet.shscript (aExecStartPredependency)--container-runtime--runtime-request-timeout--container-runtime-endpointIssue Fixed:
Requirements:
Notes: