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

changes for ansible upgrade 9.1.0 #6209

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pvisweswar
Copy link
Contributor

@pvisweswar pvisweswar marked this pull request as draft January 24, 2024 04:46
@@ -33,7 +33,7 @@
tags: pghashlib

- name: Setup plproxy
include: plproxy.yml
include_tasks: plproxy.yml
when: item.get('host')
loop:
- "{{ postgresql_dbs.form_processing.proxy|default({}) }}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

include_tasks

I've replaced the include statement with include_tasks since it seems that the file plproxy.yml contains tasks.
The loop construct remains unchanged, enabling the task to iterate over the given list.

when: rabbitmq_version == '3.10.13-1'

- include: install_old.yml
- include_tasks: install_old.yml
when: rabbitmq_version == '3.10.7-1'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

include_tasks

  • I've replaced the include statements with include_tasks since it seems that these files contain tasks.
  • Each include task is named for clarity and readability.
  • The conditionals (when statements) are kept intact.

when: rabbitmq_create_cluster|bool

- include: config.yml
- include_tasks: config.yml
when: not rabbitmq_create_cluster or ( ansible_fqdn == hostvars[rabbitmq_cluster_master]['ansible_fqdn'] )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

include_tasks

  • I've replaced the include statements with include_tasks since it seems that these files contain tasks.
  • Each include task is named for clarity and readability.
  • The conditionals (when statements) are kept intact.

when: (rabbitmq_create_cluster | bool) and ( ansible_fqdn != hostvars[rabbitmq_cluster_master]['ansible_fqdn'] )

- include: ha-policy.yml
- include_tasks: ha-policy.yml
when: (rabbitmq_create_cluster| bool ) and ( ansible_fqdn == hostvars[rabbitmq_cluster_master]['ansible_fqdn'] )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

include_tasks

  • I've replaced the include statements with include_tasks since it seems that these files contain tasks.
  • Each include task is named for clarity and readability.
  • The conditionals (when statements) are kept intact.

@@ -1,6 +1,6 @@
---

- include: setup_cluster.yml
- include_tasks: setup_cluster.yml
when: is_redis_cluster|bool and inventory_hostname in groups['redis_cluster_master']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

include_tasks

  • I've replaced the include statement with include_tasks since it seems that the file setup_cluster.yml contains tasks.
  • The condition (when statement) is kept intact.

when: elasticsearch_version == '5.6.16'

- include: misc_v7.yml
- include_tasks: misc_v7.yml
when: elasticsearch_version is version('7.0.0', '>=')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

include_tasks:

  • I've replaced the include statements with include_tasks since it seems that these files contain tasks.
  • Each include task is named for clarity and readability.
  • The conditionals (when statements) are kept intact.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not totally clear whether include_tasks or import_tasks is the best replacement. The deprecated include docs aren't super clear on how to know which to use. I think include_tasks (which is dynamic) will work here and may even be required in situations where a when condition is applied to the include.

To summarize, this looks fine unless Ansible doesn't like it for some reason.

@@ -141,7 +141,7 @@
- name: check users password valid time
shell: getent shadow "{{ item }}" | cut -d':' -f5
register: users_pw_valid
loop: "{{ dev_users.present }} + ['ansible', '{{ cchq_user }}']"
loop: "{{ ['ubuntu'] + ['ansible', cchq_user] }}"
changed_when: False
Copy link
Contributor Author

Choose a reason for hiding this comment

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

change this to loop: "{{ ['dev_users.present'] + ['ansible', cchq_user] }}"

Copy link
Contributor

@millerdev millerdev left a comment

Choose a reason for hiding this comment

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

Looks like tests are failing because of new Ansible help output. I think this can be fixed by running make to update the docs, and then commit the changes.

@@ -141,7 +141,7 @@
- name: check users password valid time
shell: getent shadow "{{ item }}" | cut -d':' -f5
register: users_pw_valid
loop: "{{ dev_users.present }} + ['ansible', '{{ cchq_user }}']"
loop: "{{ ['ubuntu'] + ['ansible', cchq_user] }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look like it does what it did before. Would this work instead?

Suggested change
loop: "{{ ['ubuntu'] + ['ansible', cchq_user] }}"
loop: "{{ dev_users.present + ['ansible', cchq_user] }}"

when: elasticsearch_version == '5.6.16'

- include: misc_v7.yml
- include_tasks: misc_v7.yml
when: elasticsearch_version is version('7.0.0', '>=')
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not totally clear whether include_tasks or import_tasks is the best replacement. The deprecated include docs aren't super clear on how to know which to use. I think include_tasks (which is dynamic) will work here and may even be required in situations where a when condition is applied to the include.

To summarize, this looks fine unless Ansible doesn't like it for some reason.

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.

None yet

2 participants