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

Let the collection to use both awx.awx and ansible.controller collections, as per user decision #351

Conversation

ivarmu
Copy link
Contributor

@ivarmu ivarmu commented Sep 9, 2022

add variable to set the plugin name to use everywhere

What does this PR do?

Right now, the tasks in filetree_create and object_diff roles are using the ansible.controller collection to make use of the controller_api plugin, so anyone trying to use the awx.awx collection will get in trouble as the ansible.controller collection should not be present. To let the end users to chose freely what collection they want to use, I've modified the code to get that collection name from a variable, which must be defined with the correct value to work successfully.

How should this be tested?

$ cat filetree_create.yml
---
- hosts: localhost
  connection: local
  gather_facts: false
  vars:
    controller_hostname: "{{ vault_controller_hostname }}"
    controller_username: "{{ vault_controller_username }}"
    controller_password: "{{ vault_controller_password }}"
    controller_validate_certs: "{{ vault_controller_validate_certs }}"
    controller_api_plugin: "ansible.controller.controller_api"
  pre_tasks:
    - name: "Check if the required input variables are present"
      assert:
        that:
          - input_tag is defined
          - (input_tag | type_debug) == 'list'
        fail_msg: 'A variable called ''input_tag'' of type ''list'' is needed: -e ''{input_tag: [organizations, projects]}'''
        quiet: true

    - name: "Check if the required input values are correct"
      assert:
        that:
          - tag_item in valid_tags
        fail_msg: "The valid tags are the following ones: {{ valid_tags | join(', ') }}"
        quiet: true
      loop: "{{ input_tag }}"
      loop_control:
        loop_var: tag_item
  roles:
    - redhat_cop.controller_configuration.filetree_create
...
$ ansible-playbook filetree_create.yml -e '{input_tag: [inventory], output_path: ~/iam/filetree_create.2.1.7 }'

Is there a relevant Issue open for this?

No issue opened for this.

Other Relevant info, PRs, etc.

N/A

@ivarmu
Copy link
Contributor Author

ivarmu commented Sep 9, 2022

@sean-m-sullivan @Tompage1994 @djdanielsson I think it would be a good idea to have a new release based on that PR, as the current version is tied to the ansible.controllercollection and will not work if the user only {wants to,can} have awx.awx collection installed. What do you think?

Copy link
Collaborator

@Tompage1994 Tompage1994 left a comment

Choose a reason for hiding this comment

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

I don't necessarily like this solution but it may be the best we can do. I'm not sure if there is some magic we could do to look up what collection we are using.

That said, I'm happy to go with this, at least for now. My only thought is that we should probably default to ansible.controller rather than awx.awx.

@ivarmu
Copy link
Contributor Author

ivarmu commented Sep 9, 2022

I don't necessarily like this solution but it may be the best we can do. I'm not sure if there is some magic we could do to look up what collection we are using.

That said, I'm happy to go with this, at least for now. My only thought is that we should probably default to ansible.controller rather than awx.awx.

I've set it to awx.awx to be consistent with the collection already used in the example two lines after my addition. We can change it if you preffer, but the rest of the examples and documentation is based in awx.awx as well...

@ivarmu ivarmu requested a review from Tompage1994 September 9, 2022 11:14
@ivarmu
Copy link
Contributor Author

ivarmu commented Sep 9, 2022

If you agree, I could add this to automatically detect what collection is installed, and default to ansible.controller if both of them are present:

https://docs.ansible.com/ansible/latest/collections/community/general/collection_version_lookup.html

or the following to run ansible-galaxy collection list | grep ansible.controller if we don't want to depend on community.general:

https://docs.ansible.com/ansible/latest/collections/ansible/builtin/pipe_lookup.html#ansible-collections-ansible-builtin-pipe-lookup

@ivarmu
Copy link
Contributor Author

ivarmu commented Sep 9, 2022

If you agree, I could add this to automatically detect what collection is installed, and default to ansible.controller if both of them are present:

https://docs.ansible.com/ansible/latest/collections/community/general/collection_version_lookup.html

or the following to run ansible-galaxy collection list | grep ansible.controller if we don't want to depend on community.general:

https://docs.ansible.com/ansible/latest/collections/ansible/builtin/pipe_lookup.html#ansible-collections-ansible-builtin-pipe-lookup

To avoid depending on community.general collection, I can integrate the following code if it look good for you:

---
- hosts: localhost
  connection: local
  gather_facts: false
  tasks:
    - block:
        - name: "Check if the collection ansible.controller is installed"
          set_fact:
            ansible_controller_collection_installed: "{{ lookup('pipe', 'ansible-galaxy collection list | grep -i ansible.controllersss') }}"
      rescue:
        - name: "Check if the collection awx.awx is installed"
          set_fact:
            awx_awx_collection_installed: "{{ lookup('pipe', 'ansible-galaxy collection list | grep -i awx.awx') }}"
    - name: "Set the collection providing the controller_api lookup plugin"
      set_fact:
        collection_name: "{{ ('ansible.controller.controller_api' if ansible_controller_collection_installed is defined) | default('awx.awx.controller_api' if awx_awx_collection_installed is defined) | default('NONE') }}"
    - debug:
        msg:
          - "collection_name: {{ collection_name }}"
...

@djdanielsson
Copy link
Collaborator

I like that solution to figure out which collection is being used

@ivarmu
Copy link
Contributor Author

ivarmu commented Sep 9, 2022

On Monday I'm working on it 😊

updated README.md

Added checks to automatically detect what collection is installed
@ivarmu ivarmu force-pushed the awx.awx-ansible_controller-independence branch from f4d740e to 61cd955 Compare September 12, 2022 08:13
@ivarmu ivarmu requested review from sean-m-sullivan and removed request for Tompage1994 September 14, 2022 06:38
@ivarmu ivarmu requested review from Tompage1994 and removed request for sean-m-sullivan September 15, 2022 12:35
@ivarmu
Copy link
Contributor Author

ivarmu commented Sep 21, 2022

@Tompage1994 @djdanielsson @sean-m-sullivan, I'm not sure if there's anything pending in this PR.

@sean-m-sullivan sean-m-sullivan self-requested a review September 22, 2022 01:39
Copy link
Collaborator

@sean-m-sullivan sean-m-sullivan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for making the changes.

@sean-m-sullivan sean-m-sullivan enabled auto-merge (rebase) September 22, 2022 01:55
@sean-m-sullivan sean-m-sullivan merged commit 5a4164c into redhat-cop:devel Sep 22, 2022
@ivarmu ivarmu deleted the awx.awx-ansible_controller-independence branch October 11, 2022 09:57
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.

4 participants