Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Use vars/RedHat_N.yml symlink for CentOS, Rocky, Alma wherever possible #236

Merged
merged 1 commit into from
Oct 25, 2024

Commits on Oct 23, 2024

  1. refactor: Use vars/RedHat_N.yml symlink for CentOS, Rocky, Alma where…

    …ver possible
    
    We have a lot of requests to support Rocky and Alma in various system roles. The
    first part of adding support is adding `vars/` files for these platforms. In
    almost every case, for a given major version N, the vars file RedHat_N.yml can
    be used for CentOS, Rocky, and Alma.  Rather than making a copy of the
    RedHat_N.yml file, just use a symlink to reduce size and maintenance burden, and
    standardize this across all system roles for consistency.
    
    NOTE: There is no Alma or Rocky version 7 or less.
    
    NOTE: OracleLinux is not a strict clone, so we are not going to do this for
    OracleLinux at this time.  Support for OracleLinux will need to be done in
    separate PRs. For more information, see
    linux-system-roles/cockpit#130
    
    **Question**: Why not just use `ansible_facts["os_family"] == "RedHat"`?
    
    **Answer**:  This is what Ansible uses as the RedHat os_family:
    https://github.com/ansible/ansible/blob/1e6ffc1d02559a26def6c9c3b07baf27032865a2/lib/ansible/module_utils/facts/system/distribution.py#L511
    There are a lot of distributions in there. I know that Fedora is not a clone of
    RHEL, but it is very closely related. Most of the others are not clones, and it
    would generally not work to replace ansible_distribution in ['CentOS', 'Fedora',
    'RedHat'] with ansible_facts['os_family'] == 'RedHat' (but it would probably
    work in specific cases with specific distributions).  For example, OracleLinux
    is in there, and we know that doesn't generally work.  The only ones we can be
    pretty sure about are `RedHat`, `CentOS`, `Fedora`, `AlmaLinux`, and `Rocky`.
    
    **Question**: Does my role really need this because it should already work on
    RHEL clones?
    
    **Answer**: Maybe not - but:
    
    * it doesn't hurt anything
    * it's there if we need it in the future
    * the role will be inconsistent with the other system roles if we don't have this
    
    **Question**: Why do I need the `tests/vars/rh_distros_vars.yml` file?  Doesn't
    the test load the vars from the role?
    
    **Answer**: No, the test does not load the vars from the role until the role is
    included, and many tests use version and distribution before including the role.
    
    **Question**: Do we need to change the code now to use the new variables?
    
    **Answer**: No, not now, in subsequent PRs, hopefully by Alma and Rocky users.
    
    Note that there may be more work to be done to the role to fully support Rocky
    and Alma.  Many roles have conditionals like this:
    
    ```yaml
    some_var: "{{ 'some value' if ansible_distribution in ['CentOS', 'RedHat'] else 'other value' }}"
    another_var: "{{ 'some value' if ansible_distribution in ['CentOS', 'Fedora', 'RedHat'] else 'other value' }}"
    
    ...
    
    - name: Do something
      when: ansible_distribution in ['CentOS', 'RedHat']
      ...
    - name: Do something else
      when: ansible_distribution in ['CentOS', 'Fedora', 'RedHat']
      ...
    ```
    
    Adding Rocky and AlmaLinux to these conditionals will have to be done
    separately. In order to simplify the task, some new variables are being
    introduced:
    
    ```yaml
    __$rolename_rh_distros:
      - AlmaLinux
      - CentOS
      - RedHat
      - Rocky
    
    __$rolename_rh_distros_fedora: "{{ __$rolename_rh_distros + ['Fedora'] }}"
    
    __$rolename_is_rh_distro: "{{ ansible_distribution in __$rolename_rh_distros }}"
    __$rolename_is_rh_distro_fedora: "{{ ansible_distribution in __$rolename_rh_distros_fedora }}"
    ```
    
    Then the conditionals can be rewritten as:
    
    ```yaml
    some_var: "{{ 'some value' if __$rolename_is_rh_distro else 'other value' }}"
    another_var: "{{ 'some value' if __$rolename_is_rh_distro_fedora else 'other value' }}"
    
    ...
    
    - name: Do something
      when: __$rolename_is_rh_distro | bool
      ...
    - name: Do something else
      when: __$rolename_is_rh_distro_fedora | bool
      ...
    ```
    
    For tests - tests that use such conditionals will need to use `vars_files` or
    `include_vars` to load the variables that are defined in
    `tests/vars/rh_distros_vars.yml`:
    
    ```yaml
    vars_files:
      - vars/rh_distros_vars.yml
    ```
    
    We don't currently have CI testing for Rocky or Alma, so someone wanting to run
    tests on those platforms would need to change the test code to use these.
    
    Signed-off-by: Rich Megginson <[email protected]>
    richm committed Oct 23, 2024
    Configuration menu
    Copy the full SHA
    b1a0aba View commit details
    Browse the repository at this point in the history