search

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
78 stars 47 forks source link

update tower install #25

Closed willtome closed 3 years ago

willtome commented 3 years ago

What does this PR do?

Update Tower Install role

How should this be tested?

- hosts: localhost
   roles:
     - redhat_cop.tower_utilities.install

Is there a relevant Issue open for this?

Integration into redhat-cop/agnosticD and ansible/workshops

Other Relevant info, PRs, etc.

sean-m-sullivan commented 3 years ago

I'm not against these changes, but the check for tower release version is in pre-tasks, and is a meta dependency of this role https://github.com/redhat-cop/tower_utilities/blob/devel/roles/pre_tasks/tasks/main.yml

Also not sure on the removal of tower alive after check, could see about reducing the time, but would want to test it.

willtome commented 3 years ago

@sean-m-sullivan this change checks to see if Tower is already installed and skips the installer if it is. On the note of pre_tasks role, why does that exist separately to begin with?

sean-m-sullivan commented 3 years ago

pre_tasks exists separately as its common tasks used by install, install_ocp, restore, and backop.

But realize that now on the change, Should be fine once the whitespace issue fixed.

ericzolf commented 3 years ago

Sorry if I reacted too late but I am the strong opinion that this MR was wrong: in the previous state, it was possible to use the role to fix, extend, upgrade Tower, and now it can only be used to initially install Tower. I still see the revert button and am very tempted to press it, but of course we should first discuss this.

Tompage1994 commented 3 years ago

Sorry if I reacted too late but I am the strong opinion that this MR was wrong: in the previous state, it was possible to use the role to fix, extend, upgrade Tower, and now it can only be used to initially install Tower. I still see the revert button and am very tempted to press it, but of course we should first discuss this.

I think I agree with you here. Maybe instead we could have some variable which will force the installer to run (perhaps by default)

sean-m-sullivan commented 3 years ago

I think we should just do another PR to fix it, I've had bad experiences with the revert before.