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

Added support for redhat 8 and 9 #177

Closed
wants to merge 4 commits into from
Closed

Conversation

cmayer89
Copy link

@cmayer89 cmayer89 commented Sep 7, 2023

Description

The process for adding the epel repo is a bit different for redhat. This pull adds an ansible task to add the epel repo for rhel 8 and 9.

Related Issue

#175

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@eRadical
Copy link
Collaborator

eRadical commented Sep 8, 2023

Rocky 8 & 9 failed because of epel repo - see https://github.com/mrlesmithjr/ansible-mariadb-galera-cluster/actions/runs/6113551752/job/16596664618?pr=177#step:6:270

Why is epel required in redhat?

@cmayer89
Copy link
Author

cmayer89 commented Sep 11, 2023

epel is required for rocky 8 and 9 also, but the method for adding the epel repo is different for redhat.

https://docs.fedoraproject.org/en-US/epel/

I'll see if I can figure out why the tests started failing.

@cmayer89
Copy link
Author

I'm having trouble getting the molecule test to run locally. It's getting stuck when it tries to build the image.

image

Maybe something different about my environment. I'm running on redhat 8.

@cmayer89
Copy link
Author

The tests do run via github actions on my fork. The tests passed for my latest commit.

@kwizart
Copy link
Contributor

kwizart commented Sep 11, 2023

Enabling EPEL can be very. difficult in some cases. A subject by itself.

A generic rocky looks easy. There is just a need to install an epel-release package that is available by default in the rocky repositories. Now on "real world users", there is likely a need to provide a dedicated internal mirror and/or a special process to enable it (via redhat satellite, katello or alikes). So this is probably worth a role by itself to handle all cases.

The main problem by installing the yum configuration by hands (not using the package) is that epel-release won't be updated as needed (like repository layout change or GPG key rotation, etc). So there is indeed a need to "maintain" theses URL in a project which main purpose is not EPEL.

I would go with an easier route to only enable epel-release package installation on the default rocky cases (and eventually alma if relevant) and use the galera_enable_epel_release boolean to discard when not relevant...

@cmayer89
Copy link
Author

Something like this?

tasks/redhat.yml

- name: redhat | Install the epel repo
  yum:
    name: "https://dl.fedoraproject.org/pub/epel/epel-release-latest-{{ ansible_distribution_major_version }}.noarch.rpm"
    state: present
  when:
    - galera_enable_epel_repo | bool

vars/redhat-8.yml

---
mariadb_login_unix_socket: "/var/lib/mysql/mysql.sock"
mariadb_pre_req_packages:
  - "procps-ng"
mariadb_packages:
  - "python3-mysql"
  - "MariaDB-server"
  - "galera-4"
mariabackup_packages:
  - "MariaDB-backup"
mariadb_certificates_dir: "/etc/my.cnf.d/certificates"
mariadb_systemd_service_name: "mariadb"
mariadb_confs:
  - "etc/my.cnf.d/server.cnf"
mariadb_temp_confs:
  - "etc/my.cnf.d/server.cnf"
galera_wsrep_provider: "/usr/lib64/galera-4/libgalera_smm.so"
galera_enable_epel_repo: true

@kwizart
Copy link
Contributor

kwizart commented Sep 11, 2023

Something like this?

tasks/redhat.yml

- name: redhat | Install the epel repo
  yum:
    name: "https://dl.fedoraproject.org/pub/epel/epel-release-latest-{{ ansible_distribution_major_version }}.noarch.rpm"
    state: present
  when:
    - galera_enable_epel_repo | bool

This looks fine with RHEL case or even as a generic case (for RedHat family without Fedora), but then this way or installing RPM with an external url tends to not properly detect changed when cases.
Now that's probably something we can accomodate (specially if it can be disabled by a boolean).

vars/redhat-8.yml

It looks possible to handle redhat-9.yml cases easily also....

@eRadical
Copy link
Collaborator

LGTM - @elcomtik thoughts?

@elcomtik
Copy link
Collaborator

I saw so many implementations for installing epel repo, so I tend to have an opinion to extract these to separate role.

I never used RHEL

Never mind, I have one dispute for this PR. Variable galera_enable_epel_repo would not be defined in both variables & defaults. In my opinion, this should be in defaults. Choosing distros where this task may run can be handled in another 'when' condition based on ansible_distribution_major_version. This principle is already used in other tasks.

@eRadical
Copy link
Collaborator

So galera_enable_epel_repo: false should be removed from defaults/main.yml and change

when:
    - galera_enable_epel_repo | bool

to

when:
    - galera_enable_epel_repo is defined
    - galera_enable_epel_repo | bool

Would that work?

@kwizart
Copy link
Contributor

kwizart commented Sep 20, 2023

@elcomtik I'm not sure to understand the benefit of such variable not defined in defaults/main ?
This is usually helpful to "document" which variable needs to be changed instead of parsing the whole playbook.

@cmayer89
On my side I would just add a when condition such alike:

  • Red Hat family and not Fedora

That way EPEL can be automatically set by default and disabled by the (few) users that need to handle this differently (or in a separate role).

I would also change the variable name to galera_enable_epel_release (instead of galera_enable_epel_repo) to further highlight that the epel-release RPM install method is used.

@elcomtik
Copy link
Collaborator

@eRadical @kwizart You both misunderstood me. I said that I want to keep this variable in defaults and remove it from variables.

@elcomtik
Copy link
Collaborator

@kwizart I agree with you on this

On my side I would just add a when condition such alike:

  • Red Hat family and not Fedora

That way EPEL can be automatically set by default and disabled by the (few) users that need to handle this differently (or in a separate role).

I would also change the variable name to galera_enable_epel_release (instead of galera_enable_epel_repo) to further highlight that the epel-release RPM install method is used.

Copy link

stale bot commented Nov 19, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 19, 2023
@eRadical
Copy link
Collaborator

@cmayer89 - are you still interested in this?

@stale stale bot removed the wontfix label Nov 19, 2023
Copy link

stale bot commented Jan 19, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 19, 2024
@stale stale bot closed this Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants