scylla-ansible-roles: Adds example playbook "kernel_version_enforcer"#402
scylla-ansible-roles: Adds example playbook "kernel_version_enforcer"#402ebenzecri wants to merge 1 commit intoscylladb:masterfrom
Conversation
ebenzecri
force-pushed
the
debian_ubuntu_image_kernel_pin
branch
from
24c063d to
effc316
Compare
vladzcloudius
left a comment
vladzcloudius
left a comment
There was a problem hiding this comment.
Eduardo, as we discussed, it would be better to have this functionality integrated into the Node Role.
| ansible.builtin.set_fact: | ||
| detected_image_version="{{ uname_pre_output.stdout_lines | first }}" | ||
|
|
||
| - name: Define if the kernel image should be installed |
There was a problem hiding this comment.
This task checks if the image_version is the one that is currently used OR if upgrade_latest_kernel is true.
The above is definitely not what the "name" claims because the package with the image_version may be installed but this Linux kernel may not be used.
| kernel_related_packages: | ||
| - linux-gcp | ||
| - linux-image-gcp | ||
| - linux-headers-gcp |
There was a problem hiding this comment.
What about linux-image-generic, linux-image-aws and corresponding headers packages?
| become: true | ||
| when: | ||
| - kernel_image_required | ||
| - not upgrade_latest_kernel |
There was a problem hiding this comment.
Why did you use upgrade_latest_kernel calculating kernel_image_required value if you are using condition like above?
|
|
||
| - name: Define if the kernel image should be installed | ||
| ansible.builtin.set_fact: | ||
| kernel_image_required="{{ image_version is version(detected_image_version, 'ne') or upgrade_latest_kernel }}" |
There was a problem hiding this comment.
Why do you have a "or upgrade_latest_kernel" part in the kernel_image_required value expression?
| autoremove: true | ||
| force_apt_get: true | ||
| become: true | ||
| when: upgrade_all_packages |
There was a problem hiding this comment.
Why do you add this to this playbook which allegedly takes care of Linux kernel (only)?
| selection: hold | ||
| loop: "{{ kernel_related_packages }}" | ||
| become: true | ||
| when: pin_kernel_version |
There was a problem hiding this comment.
dpkg hold and pinning are not the same. You should align names of parameters (pin_kernel_version) to correspond.
| selection: install | ||
| loop: "{{ kernel_related_packages }}" | ||
| become: true | ||
| when: reconfiguration_required |
There was a problem hiding this comment.
Why do you need to unhold kernel related packages?
| ansible.builtin.include_tasks: stop_reboot_start.yml | ||
| when: reconfiguration_required | ||
|
|
||
| - name: Set final kernel image version if '{{ image_version }}' was installed |
There was a problem hiding this comment.
I don't understand why all tasks below this one are required to begin with.
|
|
||
| - name: Enforce kernel version '{{ final_image_version }}' usage | ||
| ansible.builtin.include_tasks: kernel_enforce_cleanup.yml | ||
| when: reconfiguration_required |
There was a problem hiding this comment.
Why not using apt autoremove?
| - name: Mask scylla-server service | ||
| ansible.builtin.systemd: | ||
| name: scylla-server | ||
| masked: true |
| ansible.builtin.set_fact: | ||
| scylla_installation="{{ true if ansible_facts.services['scylla-server.service'] is defined else false }}" | ||
|
|
||
| - name: Stop Scylla |
There was a problem hiding this comment.
We should stop duplicating "Scylla Rolling Restart" versions and have one that is used everywhere.
The best version of Scylla shutdown can be found in https://github.com/scylladb/scylla-ansible-roles/blob/master/example-playbooks/rolling_ops/rolling_custom_build_install.yml.
We should add the missing "rescue" block to https://github.com/scylladb/scylla-ansible-roles/blob/master/example-playbooks/rolling_ops/rolling_restart.yml and then it would be good to be used everywhere.
There was a problem hiding this comment.
This stop and start functionality, is indeed preferably implemented only once (DRY principle).
Perhaps as a task in ansible-scylla-node role, and perhaps it even makes sense to group such tasks in a folder. There is already such a task present start_one_node.yml.
| @@ -0,0 +1,30 @@ | |||
| --- | |||
There was a problem hiding this comment.
Procedure below doesn't seem to be standard.
Here is an example of a procedure I'm aware of: https://askubuntu.com/questions/100232/how-do-i-change-the-grub-boot-order.
In gist you need to change GRUB_DEFAULT value in /etc/default/grub and then run sudo update-grub.
| ansible.builtin.set_fact: | ||
| scylla_installation="{{ true if ansible_facts.services['scylla-server.service'] is defined else false }}" | ||
|
|
||
| - name: Stop Scylla |
There was a problem hiding this comment.
This stop and start functionality, is indeed preferably implemented only once (DRY principle).
Perhaps as a task in ansible-scylla-node role, and perhaps it even makes sense to group such tasks in a folder. There is already such a task present start_one_node.yml.
| @@ -0,0 +1,118 @@ | |||
| --- | |||
There was a problem hiding this comment.
I don't think this functionality should be inside a playbook. We will want to re-use this functionality when setting up a ScyllaDB node.
ebenzecri
force-pushed
the
debian_ubuntu_image_kernel_pin
branch
9 times, most recently
from
259a01a to
80d87bc
Compare
"kernel_version_enforcer" playbook allow the user to: - Pin a specific kernel version (and ensure it will be picked in the next reboot) if required - Upgrade kernel version to the latest available - Purge all old kernel versions - Upgrade all upgradable packages Signed-off-by: Eduardo Benzecri <eduardo.benzecri@scylladb.com>
ebenzecri
force-pushed
the
debian_ubuntu_image_kernel_pin
branch
from
80d87bc to
61621a6
Compare
3 participants
"kernel_version_enforcer" playbook allow the user to: