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

Add support for Rocky Linux #197

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tpendragon
Copy link

This enables installation of Nomad on Rocky machines. It also lets you define a variable minimal_curl which will force installation of curl-minimal, as sometimes molecule images have one thing and production installations have another.

These are an upstream push of some changes we had to do in order to get Nomad running on our Rocky boxes for podman support: https://github.com/pulibrary/princeton_ansible/pull/5098/files

This enables installation of Nomad on Rocky machines. It also lets you
define a variable `minimal_curl` which will force installation of
curl-minimal, as sometimes molecule images have one thing and production
installations have another.
@tpendragon
Copy link
Author

I hope CI can run these tests, because I couldn't get molecule running locally. Something about it wanting a sudo password.

@tpendragon
Copy link
Author

@beechesII Sorry to ping, but going by recent merges I think you're active on this project - is there something I can do to get this in?

vars/RedHat.yml Outdated
@@ -2,7 +2,7 @@
# File: vars/RedHat.yml - Red Hat OS variables for Nomad

nomad_os_packages:
- "{% if (ansible_distribution == 'AlmaLinux' and ansible_distribution_version is version('9', '>=')) %}curl-minimal{% else %}curl{% endif %}"
- "{% if (ansible_distribution == 'AlmaLinux' and ansible_distribution_version is version('9', '>=')) or (minimal_curl is defined) %}curl-minimal{% else %}curl{% endif %}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not use two different ways to tackle this problem. Either use the variable minimal_curl or rely on the distribution facts. Both seem okay, but I'd probably use the distribution-vars since then we have no additional variable.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd be great, but the problem is some AlmaLinux or Rocky setups use curl, while some use curl-minimal. The distribution setup isn't consistent.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could solve this by changing the task `Include OS variables" to something like this:

- name: Fetch OS dependent variables
  ansible.builtin.include_vars:
    file: "{{ item }}"
  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

Then we could seperate different os-versions. I guess even without any breaking changes. Do you want to take a stab at this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That wouldn't help - sorry I wasn't clear. One install of Rocky 9 might require curl-minimal, and another might require curl.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got a middle ground I'm trying.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rndmh3ro Maybe something like this? Where the default is in defaults, and we leave it for Alma 9 for backwards compatibility, but otherwise you can just override it.

This sets an overridable default of 'curl', but for backwards
compatibility makes it curl-minimal for AlmaLinux 9.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants