Compatibility with newer Ansible versions (tested 2.17.5) #411
Conversation
- Per https://docs.ansible.com/ansible/latest/dev_guide/testing/sanity/shebang.html: It states that #!/usr/bin/env python should not be used, but rather #!/usr/bin/python.
These handler class variables were removed, and instead became options that can be set.
…'t exist. Bug fix: A service that is defined in ansible facts, is no guarantee for its existence. A service such as "firewalld" may appear, but it could have a status as 'not-found'.
… starting the manager. Errors should not be ignored when the manager service cannot start.
| internode: | ||
| enabled: false | ||
| client: | ||
| enabled: false |
There was a problem hiding this comment.
So, this is how you do it these days, huh?
Interesting...
| when: manager_agent_config_change.changed and start_scylla_service is defined and start_scylla_service|bool | ||
| ignore_errors: true | ||
| #TODO: remove ignore_errors when ansible is bumped to 2.10.4 or 2.9.16 as per https://github.com/ansible/ansible/issues/71528 | ||
|
|
There was a problem hiding this comment.
This is one of the changes that may break things with older Ansible
There was a problem hiding this comment.
This is the only such change AFAICT
ansible-scylla-manager/meta/main.yml
Outdated
| dependencies: | ||
| - role: "ansible-scylla-node" | ||
| vars: | ||
| scylla_version: "{{ scylla_manager_db_vars.scylla_version|default('latest') }}" |
There was a problem hiding this comment.
This is a functional change and shouldn't be part of this patch
You should move it to a separate patch and explain the reason for the change, or maybe just remove it from the current PR since it doesn't seem to be related.
| scylla_manager_enabled: false | ||
| scylla_version: 'latest' | ||
| scylla_edition: "{{ scylla_manager_db_vars.scylla_edition|default('oss') }}" | ||
| elrepo_kernel: false |
There was a problem hiding this comment.
Why did you remove this variable?
There was a problem hiding this comment.
I get the impression that this manager setup wants the most basic local ScyllaDB setup as possible. Ideally, there shouldn't be much to pass in terms of variables in such a case.
This variable it already happens to be false by default in the scylla-node role. Since it's not doing anything, I removed it.
I.e. I think we should align the scylla-node role to have the most 'basic' defaults as possible. Minimal hassle to get an out-of-the-box working experience.
There was a problem hiding this comment.
I've placed the variable back, and we can address such variables and defaults in a separate PR.
Newer ansible versions change the behavior regarding dependencies. Variables were not properly passed, such as the desired scylla_version, which ended up using ansible-scylla-node defaults such as a scylla_version of '0.0.0'. This removes the now redundant import role in common.yml and properly specifies the dependency along with its variables. Signed-off-by: Vincent Reniers <[email protected]>
vreniers
force-pushed
the
ansible_2_17_fix
branch
from
42df3ce
to
f7aec20
Compare
This patch introduces several fixes to allow compatibility with newer ansible versions.
Ansible versions 2.14 or older are EOL (source)
The patch introduces the following:
Fixes #409: A newer Ansible version was required to execute properly with Ubuntu 24 which came with a newer Python version out-of-the-box, which was not supported by older Ansible versions.