move kubelet --system-reserved deprecated flag to kubelet-config file#1947
move kubelet --system-reserved deprecated flag to kubelet-config file#1947ffais wants to merge 2 commits into
Conversation
|
Hi @ffais. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Signed-off-by: ffais <ffais@fbk.eu>
abfb593 to
0c1a5e4
Compare
|
/ok-to-test |
|
@ffais Hi, any news about this ? |
|
Hi @Whisper40, looks like pull-ova-all test failing is not related to the changes made in this PR. I'm waiting for a review from one of the maintainers. I know @drew-viles is very busy these days, @mboersma could take a look? |
…beadm.conf Signed-off-by: ffais <ffais@fbk.eu>
bb4109f to
b6b93c9
Compare
|
/override pull-ova-all Known photon-5 build failure. |
|
@drew-viles: Overrode contexts on behalf of drew-viles: pull-ova-all DetailsIn response to this:
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-sigs/prow repository. |
drew-viles
left a comment
There was a problem hiding this comment.
with the changed been made.
/lgtm
mboersma
left a comment
There was a problem hiding this comment.
One follow-up on the systemReserved-already-set guard:
| . "${KUBELET_SYSCONFIG}" | ||
| # If system-reserved is already set by user, ignore | ||
| if grep -q 'KUBELET_EXTRA_ARGS=.*--system-reserved' "${KUBELET_SYSCONFIG}"; then | ||
| if grep -q 'systemReserved' "${KUBELET_SYSCONFIG}"; then |
There was a problem hiding this comment.
This check no longer does what the comment above it claims. ${KUBELET_SYSCONFIG} is /etc/sysconfig/kubelet or /etc/default/kubelet — a shell-style env file consumed by the kubelet unit, not a KubeletConfiguration. A user who wants to pre-set systemReserved would do it in /var/lib/kubelet/config.yaml or in a drop-in under /var/lib/kubelet/kubelet.conf.d/, so systemReserved will essentially never appear in sysconfig and this guard will never trip.
Since this PR moves system-reserved out of KUBELET_EXTRA_ARGS and into a drop-in, I think the intent should be: "if the user has already provided a systemReserved value via the main kubelet config or any other drop-in, don't overwrite it." Something like:
# If the user has already configured systemReserved (in the main kubelet
# config or any other drop-in), don't overwrite their value.
USER_KUBELET_CONFIGS=( "/var/lib/kubelet/config.yaml" )
if [ -d /var/lib/kubelet/kubelet.conf.d ]; then
while IFS= read -r -d '' f; do
[ "$f" = "$KUBELET_CONFIG" ] && continue
USER_KUBELET_CONFIGS+=( "$f" )
done < <(find /var/lib/kubelet/kubelet.conf.d -maxdepth 1 -type f -print0)
fi
for cfg in "${USER_KUBELET_CONFIGS[@]}"; do
[ -f "$cfg" ] || continue
if grep -Eq '^[[:space:]]*systemReserved[[:space:]]*:' "$cfg" \
|| grep -q '"systemReserved"' "$cfg"; then
exit 0
fi
doneThe outer loop over KUBELET_SYSCONFIG_FILES can probably be dropped entirely now — sourcing it was only useful for the old KUBELET_EXTRA_ARGS path. It's worth a sanity check that nothing downstream still relies on that.
Change description
Kubelet '--system-reserved' has been deprecated. With this PR, the flag has been moved to a specific configuration file in the drop-ins directory. The drop-ins directory specified with '--config-dir' flag allows the user to optionally specify additional configs to overwrite what is provided by default and in the KubeletConfigFile flag.