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 45 forks source link

aap_setup_rhel_version confusion #84

Closed sean-m-sullivan closed 3 weeks ago

sean-m-sullivan commented 1 year ago

aap_setup_rhel_version: "{{ ansible_distribution_major_version | default(8, true) }}"

has issues if the installer is not run on the same destination OS, we might grab this from an inventory item? or just set it to 9, and let user overwrite it, or update the docs.

sean-m-sullivan commented 1 year ago

We should update documentation, and if the version doesnt = 8, 9, 10, we should default to 9, just to make it make sense, and b future proof for when 10 comes out.

djdanielsson commented 1 year ago

I think we should default 8 for now, idk anyone running 9 so it is a better default currently.

sean-m-sullivan commented 1 year ago

I'm up for debate on which is defaulted, atm the better question is How to and where to :)

branic commented 1 year ago

I agree with @djdanielsson about defaulting to 8 for now.

djdanielsson commented 1 year ago

what if you did something like

aap_setup_rhel_version: "{{ rhel_version | default(ansible_distribution_major_version) | default(8, true) }}"

or more crazy aap_setup_rhel_version: "{% if ansible_distribution == 'RedHat' %}{{ ansible_distribution_major_version | default(8, true) }}{% elif rhel_version is defined %}{{ rhel_version }}{% else %}8{% endif %}"

ericzolf commented 1 year ago

Few thoughts:

  1. I think, we can assume RHEL is running everywhere (else it's anyway not supported). If the version isn't 8 or 9, download will just fail, end of the discussion.
  2. we should avoid things like aap_setup_rhel_version: "{{ rhel_version... - the user of the collection can directly overwrite the default value of aap_setup_rhel_version in their inventory, we don't need yet another variable.
  3. I would assume by default that the environment is RHEL X (same version) everywhere so that I find ansible_distribution_major_version as default not such a bad idea
  4. I hesitate if I like the default of 8, which is rather random, all things considered, but on the other hand we're lazy and a solid default helps here.

So I would vote for closing this issue and leave as it is, there is no really better solution IMHO, only differently good and bad ones.

sean-m-sullivan commented 1 year ago

@ericzolf Here is the issue If you run the aap_utilities on the following you get: MacOsX: "https://api.access.redhat.com/management/v1/images/cset/ansible-automation-platform-2.2-for-rhel-12-x86_64-files", Fedora 36: "https://api.access.redhat.com/management/v1/images/cset/ansible-automation-platform-2.2-for-rhel-36-x86_64-files",

The ansible_distribution_major_version comes from the box you are running the installer on, which isn't necessarily rhel, when you are installing on Rhel machines, leading to an error as the following:

TASK [redhat_cop.aap_utilities.aap_setup_download : collecting the available installers] *****************************************************************************************************
fatal: [localhost]: FAILED! => {"cache_control": "no-cache, no-store, max-age=0, must-revalidate", "changed": false, "connection":
"close", "content": "{\"error\":{\"code\":404,\"message\":\"Not Found\"}}", "content_length": "44", "content_type": "application/json", 
"date": "Tue, 02 Aug 2022 19:50:03 GMT", "elapsed": 0, "expires": "0", "json": {"error": {"code": 404, "message": "Not Found"}}, 
"msg": "Status code was 404 and not [200]: HTTP Error 404: Not Found", "pragma": "no-cache", "redirected": false, 
"referrer_policy": "no-referrer", "set_cookie": "1a500012c7bab34b642b199769f73ca5=e401cf0b7e726c6bb22a2a9ef753d81d; 
path=/; HttpOnly; Secure; SameSite=None", "status": 404, "strict_transport_security": "max-age=31536000 ; includeSubDomains", 
"traceparent": "00-f6f4a90524589f23143c0b91b5c175ec-fe80a9a901450aea-01", "url": "https://api.access.redhat.com
/management/v1/images/cset/ansible-automation-platform-2.2-for-rhel-36-x86_64-files", "vary": "Origin", 
"x_content_type_options": "nosniff", "x_frame_options": "DENY", "x_xss_protection": "1 ; mode=block"}
sean-m-sullivan commented 1 year ago

Besides updating the docs, I was hoping for discussion on basically the following options

  1. Not doing things, letting things fail naturally
  2. Putting the download filename in the task name, and maybe even say Rhel X so its more obvious when looking for the error.
  3. Putting in an if check if the version doesn't match 8,9,11
  4. Putting in a fail message for the above task if it fails, around checking that variable.

This is in addition to updating the docs.

Tompage1994 commented 1 year ago

I think an update of documentation to make it clearer that value aap_setup_rhel_version likely needs to be set, but that it will default to the version of the OS of the host running the setup job. And then also having a descriptive fail message if it doesn't match 8/9

ericzolf commented 1 year ago

I'm still wondering what needs to be made clearer as the README already states:

djdanielsson commented 2 months ago

Is this still an issue?