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 custom packages list in 'manage-haproxy' role #669

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jfilipcz
Copy link
Contributor

What does this PR do?

This PR removes hard-coded list of packages to be installed on LB, in favor of configurable list.
Default list holds packages list valid for Centos7

How should this be tested?

Run the role with supplied defaults

Is there a relevant Issue open for this?

N/A

Other Relevant info, PRs, etc.

N/A

People to notify

cc: @redhat-cop/infra-ansible

* Introduce lb_install_packages list
* Defaults point to packages list valid for Centos7
- python3-firewall
- libsemanage-python
- policycoreutils-python
loop: '{{ lb_install_packages|flatten(levels=-1) }}'
Copy link
Contributor

Choose a reason for hiding this comment

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

the loops isn't necessary anymore now that package actually takes a single string or a list of packages to install - hence you can just feed the lb_install_packages to the name parameter above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code updated

@oybed
Copy link
Contributor

oybed commented Apr 18, 2022

In general, this change is fine. However, the role was just updated to work with RHEL8 as-is [1], so the existing content isn't a CentOS7 supported install (it may still work on CentOS7, but not tested). As the goal of the new releases of infra-ansible is to support RHEL8 (and CentOS8), I don't think we necessarily need to keep the roles backwards compatible. Your change is probably a good one to keep, but I'd recommend that we make the default values work with RHEL8/CentOS8.

[1] : https://github.com/redhat-cop/infra-ansible/pull/667/files#diff-6bf4f471bf5fa6dfa16f796613bb208747f7587218687d0dff3416e909348902

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