-
Notifications
You must be signed in to change notification settings - Fork 295
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
Separate installation of repos #1272
Conversation
@@ -44,6 +44,3 @@ zabbix_valid_web_versions: | |||
- 6.4 | |||
- 6.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove 6.2? add 7.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't add 7.0 yet (doing things one at a time). But removing 6.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still seeing 6.2 ? or ...?
Would you mind changing the role name to just It just keeps things (mostly) consistent between all role names and their defaults. |
To easy |
roles/zabbix_server/tasks/main.yml
Outdated
- name: "Configure SELinux when enabled" | ||
ansible.builtin.include_tasks: selinux.yml | ||
when: | ||
- zabbix_server_selinux | default (false) | bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You sure we don't want to reuse the previous when: ansible_facts.selinux.status | default('disabled') == 'enabled'
here? i.e. anything that reports selinux_enabled we apply on. (Suse also comes with selinux, #1194 and I think he would have an easier job (smaller PR) adding support after this merges.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, actually I think even better would be to put this when condition into defaults, so users can explicitly override us. Shame I didn't think about that in the selinux refactor.
--- # defaults/main.yml
zabbix_server_selinux: "{{ ansible_facts.selinux.status | default('disabled') == 'enabled' }}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't remember where we had that but now that you say it ya I remember seeing that. Ya I think I like that better.
@BGmot I think this one is ready to go if you want to take a peak |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confused... still seeing 6.2 in some places. Have you removed them?
@@ -20,6 +20,3 @@ zabbix_valid_javagateway_versions: | |||
- 6.4 | |||
- 6.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove 6.2
@@ -27,9 +27,6 @@ zabbix_valid_server_versions: | |||
- 6.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove 6.2
@@ -44,6 +44,3 @@ zabbix_valid_web_versions: | |||
- 6.4 | |||
- 6.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still seeing 6.2 ? or ...?
Ya I removed 6.2 in a different PR..... #1274 I think is ready so if you want to approve that one too it should automagically take care of this one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! Thanks!
SUMMARY
This PR will remove the repo installation tasks from the individual roles and consolidate them into a single role that each of the roles references as requested in #1242.
Also adds the downloading of the GPG key for RHEL based operating systems as was started in #1239 (credit to @jonathananspw)
All adds support for yum authentication on RHEL based operating systems as was started in #1185 (credit to @USer78012)
ISSUE TYPE
COMPONENT NAME
All roles