ansible-community / ansible-nomad

:watch: Ansible role for Nomad
https://galaxy.ansible.com/brianshumate/nomad
BSD 2-Clause "Simplified" License
297 stars 165 forks source link

Ensure that OS package conflicts can be handled properly #194

Closed dosera closed 6 days ago

dosera commented 7 months ago

With https://github.com/ansible-community/ansible-nomad/pull/191 a conditional was introduced to not fail if curl-minimal is installed rather than curl in case of almalinux:9 docker image:

 "{% if (ansible_distribution == 'AlmaLinux' and ansible_distribution_version is version('9', '>=')) %}curl-minimal{% else %}curl{% endif %}"

In (at least) almalinux 9 both variants is possible.

This PR extends the possibility for the nomad_os_packages to declare potential conflicts and have them handled accordingly:

nomad_os_packages:
  - name: curl
    conflicts: curl-minimal
    handle: skip

Execution:

  1. Gather the OS packages using ansible.builtin.package_facts
  2. A fact is set that considers a package only if the conflicting package is not installed or handle is not set to skip
rndmh3ro commented 7 months ago

This seems very complex.

In another collection I use the following approach.

This task runs at the very start of the role:

- name: Fetch OS dependent variables
  ansible.builtin.include_vars:
    file: "{{ item }}"
    name: os_vars
  with_first_found:
    - files:
        - "{{ ansible_facts.distribution }}_{{ ansible_facts.distribution_major_version }}.yml"
        - "{{ ansible_facts.distribution }}.yml"
        - "{{ ansible_facts.os_family }}_{{ ansible_facts.distribution_major_version }}.yml"
        - "{{ ansible_facts.os_family }}.yml"
      skip: true
  tags: always

In the vars-folder I then have a vars-file for every needed combination of operating system and its version:

roles/os_hardening/vars/
├── Amazon.yml
├── Archlinux.yml
├── Debian.yml
├── Fedora.yml
├── main.yml
├── RedHat_7.yml
├── RedHat.yml
├── Suse.yml
└── Ubuntu.yml

This way I can define exactly what package will be installed on what operating system. I admit this will not handle conflicts but I'm also not sure that this should be done in this role at all, but rather it should be left to the user of the role.

dosera commented 7 months ago

This seems very complex.

I perfectly agree and I am also not very convinced of the solution here. I needed a running one quite quickly though.

In another collection I use the following approach.

This task runs at the very start of the role:

- name: Fetch OS dependent variables
  ansible.builtin.include_vars:
    file: "{{ item }}"
    name: os_vars
  with_first_found:
    - files:
        - "{{ ansible_facts.distribution }}_{{ ansible_facts.distribution_major_version }}.yml"
        - "{{ ansible_facts.distribution }}.yml"
        - "{{ ansible_facts.os_family }}_{{ ansible_facts.distribution_major_version }}.yml"
        - "{{ ansible_facts.os_family }}.yml"
      skip: true
  tags: always

In the vars-folder I then have a vars-file for every needed combination of operating system and its version:

roles/os_hardening/vars/
├── Amazon.yml
├── Archlinux.yml
├── Debian.yml
├── Fedora.yml
├── main.yml
├── RedHat_7.yml
├── RedHat.yml
├── Suse.yml
└── Ubuntu.yml

This way I can define exactly what package will be installed on what operating system. I admit this will not handle conflicts but I'm also not sure that this should be done in this role at all, but rather it should be left to the user of the role.

I understand the point but I do not see how this would help here - in the scenario here I cannot determine based on the distribution if curl or curl-minimal is installed since both are available via the package manager (at least in AlmaLinux 9) - and either one could be already installed when this role is ran. Furthermore I didn't want to rework the whole package installation process (I also fully agree with you that this should not be part of the role ..)

tpendragon commented 4 months ago

Oh yeah I had the same problem with Rocky, and just added a curl-minimal variable in https://github.com/ansible-community/ansible-nomad/pull/197

dosera commented 6 days ago

closing in favor of #197