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

Port to minimum of Ansible 2.5 (resolves #190) #215

Merged
merged 8 commits into from
Dec 18, 2018

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Nov 10, 2018

Port to Ansible 2.5 (resolves #190)

  • Use loop instead of with_*
  • Use import_tasks for static includes
  • Use include_tasks for dynamic includes
  • Apply tags to include_tasks tasks
  • bump minimum for remaining roles
  • Use ansible_facts namespaced vars
  • Superficial migration of BWC -> EWC (tags, task names, comments)

Thanks to @tzyychin for helping me put this together!

Replaces `with_*` loops with the new `loop` keyword.[1]

Note that with_dict => loop uses the dictsort filter as dict2items is
not available until Ansible 2.6.

[1] https://docs.ansible.com/ansible/2.7/porting_guides/porting_guide_2.5.html#migrating-from-with-x-to-loop
Replaces static includes with import_tasks (partially resolves StackStorm#190).
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Left a few cosmetic comments. Overall looks nice.
Need to check also not so obvious migration points like with_items.

roles/bwc/tasks/bwc_repos_apt.yml Outdated Show resolved Hide resolved
@@ -5,3 +5,6 @@
yum_resository:
name: "StackStorm_{{ bwc_repo }}"
state: absent
tags:
- BWC repos
Copy link
Member

Choose a reason for hiding this comment

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

Don't need BWC repos tag

Copy link
Member Author

Choose a reason for hiding this comment

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

These tags orignally came from:

  1. roles/bwc/tasks/bwc_repos_setup.yml: tags on "Handle bwc_license change" include: license.yml
  2. roles/bwc/tasks/license.yml: "Cleanup repo list file from disk"

Since include_tasks can't propagate tags until a later version of ansible, I manually propagated the tags.

If you want me to drop the ewc tag, that's fine.

@cognifloyd
Copy link
Member Author

I did a general switch of BWC -> EWC including tags, task names, comments and some readme entries. I did not update variable names, file names, or role names to avoid breaking anyone using these roles.

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Looks good 👍
Obviously 2.5 migration is a little bit more involved than we all thought :)

Will do a few more checks and merge it.

tzyychin and others added 5 commits November 12, 2018 20:07
Replaces dynamic includes with include_tasks (resolves StackStorm#190).

include_tasks (introduced in Ansible 2.4) does not support ansible 1.x
style inline vars syntax (var=value) (since at least Ansible 2.4.1)[1].
If the syntax ever worked, removing support for it was announced in the
Ansible 2.7 Porting Guide[2]. So, this modifies include loops to be
include_tasks loops using the `vars` key instead of the inline syntax.

[1] ansible/ansible#32812
[2] https://docs.ansible.com/ansible/2.7/porting_guides/porting_guide_2.7.html#include-tasks-import-tasks-inline-variables
Dynamic includes (include_tasks) do not apply tags to internal tasks.[1]
Once we transition to 2.7, we can use `apply` on the `include_tasks`
module to specify the tags for included files.[2]

[1] https://docs.ansible.com/ansible/2.7/porting_guides/porting_guide_2.5.html#dynamic-includes-and-attribute-inheritance
[2] https://docs.ansible.com/ansible/2.7/modules/include_tasks_module.html
Bump the minimum ansible version for the remaining roles.
With 2.5, facts are now namespaced under ansible_facts.[1]
They are still accessible under the ansible_* vars, however that can be
disabled by setting `inject_facts_as_vars` to False. The default is True
in 2.5, but "is expected to be set to 'False' in a future release.
Migrating now means these roles and playbooks do not depend on this
setting being True.

[1] https://docs.ansible.com/ansible/2.7/porting_guides/porting_guide_2.5.html#ansible-fact-namespacing
@arm4b arm4b merged commit 856cc37 into StackStorm:master Dec 18, 2018
@arm4b arm4b added this to the v1.0.0 milestone Dec 20, 2018
@cognifloyd cognifloyd deleted the ansible2.5-support branch September 24, 2020 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ansible 2.4+ deprecation warnings
4 participants