redhat-cop / infra.leapp

Collection of Ansible roles for automating RHEL in-place upgrades using Leapp.
MIT License
44 stars 34 forks source link

WIP: DO NOT MERGE - redhat.rhel_system_roles instead of fedora.linux_system_roles. #136

Closed jeffmcutter closed 3 months ago

jeffmcutter commented 10 months ago

I tested these changes to use redhat.rhel_system_roles instead of fedora.linux_system_roles. We would want to use redhat.rhel_system_roles for the collection on console.redhat.com (Automation Hub) but would not want to use these changes on galaxy.ansible.com (Galaxy).

Tested OK RHEL 6-7, 7-8, and 8-9.

jeffmcutter commented 10 months ago

Hi @swapdisk,

It's not my intention for this to be merged. I just wanted to share the changes I made to "test" using redhat.rhel_system_roles instead of fedora.linux_system_roles. I actually pulled the collections locally and ran from ansible-navigator command line. I just wanted to share that it does work with rhel_system_roles and where the changes will need to be made. I think someone like @djdanielsson will need to look at how we want to handle this in the pipeline or other.

Feel free to close once the right folks have been involved.

Thanks, -Jeff

swapdisk commented 10 months ago

It's not my intention for this to be merged. I just wanted to share the changes I made to "test" using redhat.rhel_system_roles instead of fedora.linux_system_roles.

Gotcha, that makes sense. We can cite this PR to help users who can't include the upstream Linux system roles for whatever reason or just prefer to use the RHEL system roles supported by Red Hat.

Maybe the "solution" here is just to add a footnote in the README of the collection or upgrade role.

I'll leave the PR open for now to allow others to share their feedback.

djdanielsson commented 5 months ago

I have been thinking about this some more, there is a case to be made that if you are trying to upgrade a RHEL server you then should have access to redhat.rhel_system_roles because those come with a sub to RHEL unless I am mistaken so maybe we should just make this change and not mess with the CI

Denney-tech commented 5 months ago

@djdanielsson perhaps it could work more like the infra.controller_configuration collection? It supports both the awx.awx upstream collection and ansible.controller downstream collection. Rather than listing fedora.linux_system_roles or redhat.rhel_system_roles as dependencies in galaxy.yml, list them as collections in the role/meta/main.yaml files. Then, for e.g., leapp-post-upgrade-crypto.yml would use the short name for the role like so:

---
- name: leapp-post-upgrade-crypto | Include rhel_system_roles.crypto_policies role
  ansible.builtin.include_role:
    name: crypto_policies
  vars:
    crypto_policies_policy: "{{ crypto_policy }}"
...

I'm not necessarily a fan of using a role's shortname, but I don't have any better ideas right now on how to dynamically grab the upstream verses downstream collection. Other than to build infra.leapp as redhat.leapp and template the dependencies between up/downstream leapp collections.

djdanielsson commented 5 months ago

yea that is true we could go that way, thoughts @swapdisk @jeffmcutter @heatmiser

jeffmcutter commented 4 months ago

Seems reasonable, if it's good enough for infra.controller_collection it should be OK here too. We can # noqa the tasks to pass lint. I'm willing to give it a go, but not sure when. If anyone else wants to try it out and PR it, I'm good with that also.

jeffmcutter commented 3 months ago

Closing in favor of https://github.com/redhat-cop/infra.leapp/pull/187. Thanks @Denney-tech.