Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect list splitting in kubelet config template #9693

Closed
Tristan971 opened this issue Jan 20, 2023 · 0 comments · Fixed by #9694
Closed

Incorrect list splitting in kubelet config template #9693

Tristan971 opened this issue Jan 20, 2023 · 0 comments · Fixed by #9694
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@Tristan971
Copy link
Contributor

Tristan971 commented Jan 20, 2023

Environment:

  • Cloud provider or hardware configuration: Controller issue, irrelevant

  • OS (printf "$(uname -srm)\n$(cat /etc/os-release)\n"): Controller issue, irrelevant

  • Version of Ansible (ansible --version):

ansible [core 2.12.5]
  config file = None
  configured module search path = ['/home/tristan/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/tristan/.local/share/virtualenvs/blablabla/lib/python3.10/site-packages/ansible
  ansible collection location = /home/tristan/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/tristan/.local/share/virtualenvs/blablabla/bin/ansible
  python version = 3.10.8 (main, Nov 16 2022, 04:03:17) [GCC 12.2.1 20220819 (Red Hat 12.2.1-2)]
  jinja version = 2.11.3
  libyaml = True
  • Version of Python (python --version):
Python 3.10.8

Kubespray version (commit) (git rev-parse --short HEAD):

c4346e5

Network plugin used: Cilium but irrelevant

Full inventory with variables (ansible -i inventory/sample/inventory.ini all -m debug -a "var=hostvars[inventory_hostname]"):

Relevant line is:

kubelet_enforce_node_allocatable: "pods,kube-reserved,system-reserved"

based on

# A comma separated list of levels of node allocatable enforcement to be enforced by kubelet.
kubelet_enforce_node_allocatable: "pods"
# kubelet_enforce_node_allocatable: "pods,kube-reserved"
# kubelet_enforce_node_allocatable: "pods,kube-reserved,system-reserved"

Command used to invoke ansible:

Irrelevant, but upgrade-cluster playbook

Output of ansible run:

The run goes fine, until we hit the first Kubelet restart and see the following in its logs:

Jan 20 23:06:01 core-ctr-1 kubelet[327519]: E0120 23:06:01.777975  327519 run.go:74] "command failed" err="failed to validate kubelet configuration, error: invalid configuration: option \"pods,kube-reserved,system-reserved\" specified for enforceNodeAllocatable (--enforce-node-allocatable). Valid options are \"pods\", \"system-reserved\", \"kube-reserved\", or \"none\", path: &TypeMeta{Kind:,APIVersion:,}"

And indeed:

$ cat /etc/kubernetes/kubelet-config.yaml | grep -A2 enforceNode
enforceNodeAllocatable:
- pods,kube-reserved,system-reserved
staticPodPath: /etc/kubernetes/manifests

Anything else do we need to know:

A call to jinja's split() function is made in the kubelet config template here:

{% if kubelet_enforce_node_allocatable is defined and kubelet_enforce_node_allocatable != "\"\"" %}
{% set kubelet_enforce_node_allocatable_list = kubelet_enforce_node_allocatable.split() %}
enforceNodeAllocatable:
{% for item in kubelet_enforce_node_allocatable_list %}
- {{ item }}
{% endfor %}
{% endif %}

This however doesn't work by default for comma-separated lists, which thus contradicts the inventory examples and documentation

See the following example matching Kubespray's current behaviour:

$ ansible -m debug -a "msg='{{ test_list | split() }}'" localhost --extra-vars='test_list=a,b,c'
localhost | SUCCESS => {
    "msg": [
        "a,b,c"
    ]
}

And what was intended (split(',')):

$ ansible -m debug -a "msg='{{ test_list | split(\",\") }}'" localhost --extra-vars='test_list=a,b,c'
localhost | SUCCESS => {
    "msg": [
        "a",
        "b",
        "c"
    ]
}

Small note: I also find the extra quote escaping on the if != "" just above quite suspicious. Didn't test it though, and it might work somewhat by chance, but still seems kind of wrong.

@Tristan971 Tristan971 added the kind/bug Categorizes issue or PR as related to a bug. label Jan 20, 2023
Tristan971 added a commit to Tristan971/kubespray that referenced this issue Jan 20, 2023
Tristan971 added a commit to Tristan971/kubespray that referenced this issue Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant