redhat-cop / aap_utilities

Ansible Collection for automated deployment of AAP and other objects for general use
https://galaxy.ansible.com/infra/aap_utilities
GNU General Public License v3.0
74 stars 44 forks source link

Allow individual components to be installed on OCP #93

Closed branic closed 1 year ago

branic commented 1 year ago

What does this PR do?

Allow each component to be individually installed (operator, controller, hub).

As an example the below playbook will now execute without generating an error for missing hub configuration info and successfully installs the operator and controller.

- name: Install AAP on OCP playbook
  hosts: localhost
  gather_facts: false

  vars:
    aap_ocp_install_ocp_connection:
      host: "https://api.crc.testing:6443"
      username: kubeadmin
      password: <REDACTED>
      validate_certs: false
    aap_ocp_install_namespace: aap-test
    aap_ocp_install_operator:
      channel: "stable-2.2"
    aap_ocp_install_controller:
      instance_name: automationcontroller

  roles:
    - redhat_cop.aap_utilities.aap_ocp_install

How should this be tested?

Automated tests are preferred, but not always doable - especially for infrastructure. Include commands to run your new feature, and also post-run commands to validate that it worked. (please use code blocks to format code samples)

Is there a relevant Issue open for this?

No

Other Relevant info, PRs, etc

None

ericzolf commented 1 year ago

I didn't go (yet) into the details of the review but I'm slightly worried by the explicit validation of ansible_run_tags.

Let me try to word my worries:

  1. first time I see this, so always worrying (or a cool new thing to learn :+1: )
  2. more seriously, checking ansible_run_tags without checking ansible_skip_tags could lead to unexpected behavior of tags,
  3. mixing tags and variables could lead to strange side effects
  4. and then, overall, this could make the whole setup rather complex
  5. and, in the good practices, we recommend against tags within roles

I'm not fully against it, I'm just wondering if it's a good idea, and open to discussion.

branic commented 1 year ago

I can remove the tags and just base the task execution on the existence of the variables. The tags do add a level of complexity.

branic commented 1 year ago

I finally got back around to working on this PR. I rebased it so there wouldn't be any merge conflicts as its been a while. I also simplified the when conditions and removed the use of ansible_run_tags.