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

Use a single command to upgrade nodes #94

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sbruzzese902
Copy link
Contributor

Hi 👋

I propose to switch to a single playbook when upgrading control planes and workers.

Why?

This will allow you to choose how many nodes you want to upgrade without launching the ansible-playbook command multiple times. You will only need to adjust the serial parameter, in case.

For example, now if I want to upgrade 3 nodes node-a, node-b, and node-c I need to do the following (assuming that I don't want the 3 nodes to be offline at the same time):

ansible-playbook 56.upgrade-worker-nodes.yml -l node-a
ansible-playbook 56.upgrade-worker-nodes.yml -l node-b
ansible-playbook 56.upgrade-worker-nodes.yml -l node-c

With the proposed setup I would do this:

ansible-playbook 56.upgrade-worker-nodes.yml -l node-a,node-b,node-c

and the playbook would execute the tasks on the hosts, one by one, without the need for my interactions.

@sbruzzese902 sbruzzese902 self-assigned this Aug 21, 2024
Copy link
Member

@ralgozino ralgozino left a comment

Choose a reason for hiding this comment

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

Hi @sbruzzese902!

Just to be clear, what you are proposing is to run a single command for all nodes instead of one command for each node. And not a merging the two playbooks into one as the PR title may suggest, right?

I agree with you that when using the playbook manually this change could be handy, not so when you are upgrading the cluster with furyctl that runs the commands for you, so it is the same.

The only downside that I see with this change is that if in the future we would like to upgrade more than one node in parallel it won't be doable, correct? Or can the serial = 1 be modified when running the playbook somehow?

Have you checked if this change is compatible with furyctl upgrade proces? if yes, are any changes needed there?

Thanks!

examples/playbooks/55.upgrade-control-plane.yml Outdated Show resolved Hide resolved
@sbruzzese902 sbruzzese902 changed the title Use a single playbook to upgrade nodes Use a single command to upgrade nodes Aug 22, 2024
@sbruzzese902
Copy link
Contributor Author

sbruzzese902 commented Aug 22, 2024

@ralgozino you're right, I've changed the PR title to be clearer 😄

As it is, the serial is hard coded, but can be turned into a variable and a default can be set, like this:

- name: playbook-name
  hosts: all
  serial: "{{ serial_number | default(1) }}"

and then at runtime, you can specify how many hosts you want to affect at the same time:

ansible-playbook 56.upgrade-worker-nodes.yml -e serial_number=3

What do you think?


The upgrade process won't be affected, because the old command still works in the same way.
In the future the approach may be improved, for example here:

# Before
{{- range $h := .spec.kubernetes.masters.hosts }}
ansible-playbook 55.upgrade-control-plane.yml --limit "{{ $h.name }}" --become
{{- end }}

# After
ansible-playbook 55.upgrade-control-plane.yml --become

@ralgozino
Copy link
Member

I think that setting the serial value as a variable and having it default to one is better than hardcoding it.

I'm not sure we will need to change it but, if it is as simple as that, having the possibility to upgrade more than one node in parallel could come handy in the future.

Thank you for checking the furyctl side, it is good to know that we don't need to change anything over there :)

@nutellinoit, what are your thoughts on the serial value variable?

I'm personally OK with both situations, but I'm more inclined on having the option to change it easily in the future if it is a simple change.

Copy link
Member

@ralgozino ralgozino left a comment

Choose a reason for hiding this comment

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

lgtm!

great job @sbruzzese902 <3

Copy link
Member

@nutellinoit nutellinoit left a comment

Choose a reason for hiding this comment

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

LGTM as well for me, we should also see how to implement this using the distro and furyctl, since these are example playbooks only

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants