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

fix: restore clustering and avoid failure with jinja2_native=true #135

Merged
merged 2 commits into from
Jul 21, 2021
Merged

fix: restore clustering and avoid failure with jinja2_native=true #135

merged 2 commits into from
Jul 21, 2021

Conversation

yajo
Copy link
Contributor

@yajo yajo commented Jul 19, 2021

Summary

If you run the role on an ansible configured with that setting, it will fail with:

fatal: [vm0]: FAILED! => {"msg": "Unexpected templating type error occurred on ({% for host in ansible_play_hosts_all %}\n{% filter string %}\n{% filter replace('\\n', ' ') %}\n{{ host }}\n@@@\n{{ hostvars[host].ansible_host | default(hostvars[host].ansible_fqdn) }}\n@@@\nC_{{ hostvars[host].k3s_control_node }}\n@@@\nP_{{ hostvars[host].k3s_primary_control_node | default(False) }}\n{% endfilter %}\n{% endfilter %}\n@@@ END:{{ host }}\n{% endfor %}): sequence item 4: expected str instance, bool found"}

Issue type

  • Bugfix

Test instructions

Just run the playbook prefixing the command with env ANSIBLE_JINJA2_NATIVE=true and see that it works now.

yajo added 2 commits July 19, 2021 09:26
If you run the role on an ansible configured with that setting, it will fail with:

    fatal: [vm0]: FAILED! => {"msg": "Unexpected templating type error occurred on ({% for host in ansible_play_hosts_all %}\n{% filter string %}\n{% filter replace('\\n', ' ') %}\n{{ host }}\n@@@\n{{ hostvars[host].ansible_host | default(hostvars[host].ansible_fqdn) }}\n@@@\nC_{{ hostvars[host].k3s_control_node }}\n@@@\nP_{{ hostvars[host].k3s_primary_control_node | default(False) }}\n{% endfilter %}\n{% endfilter %}\n@@@ END:{{ host }}\n{% endfor %}): sequence item 4: expected str instance, bool found"}
For some weird reason, string booleans were set on `k3s_control_node` and `k3s_primary_control_node`, making their behavior non-obvious (for python `bool("false") == True`).

This fixes that problem, and BTW restores the ability to create clusters, which got lost with this bug.

After running the role against a cluster, see:

```sh
❯ ansible -i inventories/testing.yaml k8s_node -m command -ba 'kubectl get node'
vm0 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE     VERSION
vm0    Ready    control-plane,etcd,master   9m19s   v1.21.2+k3s1
vm2 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE     VERSION
vm2    Ready    control-plane,etcd,master   9m22s   v1.21.2+k3s1
vm1 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE     VERSION
vm1    Ready    control-plane,etcd,master   9m22s   v1.21.2+k3s1
```

Now, after the patch:

```sh
❯ ansible -i inventories/testing.yaml k8s_node -m command -ba 'kubectl get node'
vm0 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE    VERSION
vm0    Ready    control-plane,etcd,master   2m2s   v1.21.2+k3s1
vm1    Ready    control-plane,etcd,master   58s    v1.21.2+k3s1
vm2    Ready    control-plane,etcd,master   80s    v1.21.2+k3s1
vm1 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE    VERSION
vm0    Ready    control-plane,etcd,master   2m2s   v1.21.2+k3s1
vm1    Ready    control-plane,etcd,master   58s    v1.21.2+k3s1
vm2    Ready    control-plane,etcd,master   80s    v1.21.2+k3s1
vm2 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE    VERSION
vm0    Ready    control-plane,etcd,master   2m2s   v1.21.2+k3s1
vm1    Ready    control-plane,etcd,master   58s    v1.21.2+k3s1
vm2    Ready    control-plane,etcd,master   80s    v1.21.2+k3s1
```

@Tecnativa TT2541
@yajo yajo changed the title fix: avoid failure with jinja2_native=true fix: restore clustering and avoid failure with jinja2_native=true Jul 21, 2021
@yajo
Copy link
Contributor Author

yajo commented Jul 21, 2021

Hello, dear maintainers.

This PR suddenly graduated to being something essential. The role had lost its possibilities to cluster!

Quoting from last commit:

fix: restore clustering feature

For some weird reason, string booleans were set on k3s_control_node and k3s_primary_control_node, making their behavior non-obvious (for python bool("false") == True).

This fixes that problem, and BTW restores the ability to create clusters, which got lost with this bug.

After running the role against a cluster, see:

❯ ansible -i inventories/testing.yaml k8s_node -m command -ba 'kubectl get node'
vm0 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE     VERSION
vm0    Ready    control-plane,etcd,master   9m19s   v1.21.2+k3s1
vm2 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE     VERSION
vm2    Ready    control-plane,etcd,master   9m22s   v1.21.2+k3s1
vm1 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE     VERSION
vm1    Ready    control-plane,etcd,master   9m22s   v1.21.2+k3s1

Now, after the patch:

❯ ansible -i inventories/testing.yaml k8s_node -m command -ba 'kubectl get node'
vm0 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE    VERSION
vm0    Ready    control-plane,etcd,master   2m2s   v1.21.2+k3s1
vm1    Ready    control-plane,etcd,master   58s    v1.21.2+k3s1
vm2    Ready    control-plane,etcd,master   80s    v1.21.2+k3s1
vm1 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE    VERSION
vm0    Ready    control-plane,etcd,master   2m2s   v1.21.2+k3s1
vm1    Ready    control-plane,etcd,master   58s    v1.21.2+k3s1
vm2    Ready    control-plane,etcd,master   80s    v1.21.2+k3s1
vm2 | CHANGED | rc=0 >>
NAME   STATUS   ROLES                       AGE    VERSION
vm0    Ready    control-plane,etcd,master   2m2s   v1.21.2+k3s1
vm1    Ready    control-plane,etcd,master   58s    v1.21.2+k3s1
vm2    Ready    control-plane,etcd,master   80s    v1.21.2+k3s1

@Tecnativa TT2541

@yajo
Copy link
Contributor Author

yajo commented Jul 21, 2021

BTW I think there should be some assertion on roles that makes sure nodes got properly clustered. Any clue on how to do that? Thanks!

Copy link
Member

@xanmanning xanmanning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 , thanks.

@xanmanning
Copy link
Member

BTW I think there should be some assertion on roles that makes sure nodes got properly clustered. Any clue on how to do that? Thanks!

I have thoughts, but I am going to be doing a lot of refactoring for v3 (when I have got time...)

@xanmanning xanmanning merged commit d2968d5 into PyratLabs:main Jul 21, 2021
@xanmanning xanmanning self-assigned this Jul 21, 2021
@xanmanning xanmanning added the bug Something isn't working label Jul 21, 2021
@yajo yajo deleted the fix-jinja2-native branch July 22, 2021 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants