-
-
Notifications
You must be signed in to change notification settings - Fork 441
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 Ubuntu 22.04 (jammy) #406
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
...and remove trusty from meta file since it is already no longer supported
friendly bump |
eb4x
added a commit
to eb4x/community.zabbix
that referenced
this pull request
May 25, 2024
Remove ubuntu-22.04 from this workaround, support for ubuntu-22.04 was added in 5.0.0[1]. Not sure if it's a good idea to pin the geerlingguy role? We can pin it to a specific version, but I haven't figured out why it's not respecting a relative version, like '>=5.0.0' [1] geerlingguy/ansible-role-php#406
eb4x
added a commit
to eb4x/community.zabbix
that referenced
this pull request
Jun 5, 2024
Remove ubuntu-22.04 from this workaround, support for ubuntu-22.04 was added in 5.0.0[1]. Not sure if it's a good idea to pin the geerlingguy role? We can pin it to a specific version, but I haven't figured out why it's not respecting a relative version, like '>=5.0.0' [1] geerlingguy/ansible-role-php#406
eb4x
added a commit
to eb4x/community.zabbix
that referenced
this pull request
Jun 8, 2024
Remove ubuntu-22.04 from this workaround, support for ubuntu-22.04 was added in 5.0.0[1]. Not sure if it's a good idea to pin the geerlingguy role? We can pin it to a specific version, but I haven't figured out why it's not respecting a relative version, like '>=5.0.0' [1] geerlingguy/ansible-role-php#406
eb4x
added a commit
to eb4x/community.zabbix
that referenced
this pull request
Jun 8, 2024
Remove ubuntu-22.04 from this workaround, support for ubuntu-22.04 was added in 5.0.0[1]. Not sure if it's a good idea to pin the geerlingguy role? We can pin it to a specific version, but I haven't figured out why it's not respecting a relative version, like '>=5.0.0' [1] geerlingguy/ansible-role-php#406
eb4x
added a commit
to eb4x/community.zabbix
that referenced
this pull request
Jun 8, 2024
Remove ubuntu-22.04 from this workaround, support for ubuntu-22.04 was added in 5.0.0[1]. Not sure if it's a good idea to pin the geerlingguy role? We can pin it to a specific version, but I haven't figured out why it's not respecting a relative version, like '>=5.0.0' [1] geerlingguy/ansible-role-php#406
pyrodie18
added a commit
to ansible-collections/community.zabbix
that referenced
this pull request
Jun 11, 2024
* Single common task to install zabbix-web RedHat can pin the minor version, and has a toggle to disable repo. Debian can't pin minor version. The other debian option cache_valid_time seems irrelevant? We use the common package module, which is just a wrapper around apt/yum, and use this construction; user_supplied_var | default(_calculated_var | default(omit)) to send additional parameters to the respective modules. The workaround ZBX-10467 is dependent on packages being installed, so had to get moved aswell. * Remove mysql install tasks It doesn't really make sense installing this package on all of RedHat family, when there's no corresponding Debian task doing the same. MySQL/mariadb is gets installed by the zabbix_server role. And the PyMySQL dependency is only needed for the ansible collection community.mysql, which we don't use in this role. * Improve when condition logic for os_family It's better to check for what it IS we're looking for rather than what it's not. You could be looking at 'Suse' at some point, and this logic wouldn't hold. * Swap out remi for appstream php:8.0/common Use dnf module instead of command, this makes the task idempotent. * Make it clear tasks are a workaround Remove ubuntu-22.04 from this workaround, support for ubuntu-22.04 was added in 5.0.0[1]. Not sure if it's a good idea to pin the geerlingguy role? We can pin it to a specific version, but I haven't figured out why it's not respecting a relative version, like '>=5.0.0' [1] geerlingguy/ansible-role-php#406 * Remove zabbix 5.0 related task Zabbix 5 wanted to install the sql-scripts to /usr/share/doc, and this task removes a line in yum.conf that prevented that from happening. But since 6.0 we haven't had to deal with that, so out it goes. * Remove tasks and checks guarding against EL7 This role doesn't support EL7 anymore, so we can remove these tasks/checks. * Move delegated_dbhost to first block This calculation doesn't change. So let's provide/reuse it across the whole block. * Bump mysql container image, add workarounds PyMySQL 9>=,<10 did not have issues with this pinned mysql:8.0.32, but does with later releases, so we bump pymysql for additional debian and ubuntu releases. * Increase similarity in init-mysql between roles * Provide the default port for database servers This could have been an if-statement, I just don't like the "looseness" of the else part, so I opted for a lookup-table. * Remove unused zabbix_selinux_dependencies This lookup goes unused. * Group selinux booleans, and apply regardless Having them only applied with a when condition leaves no way to undo the booleans once set. So we apply them regardless. If the booleans are defaulting to false, we set them false on the system. * Install selinux packages regardless Use the generic `package` module for package installs, and install this ansible dependency regardless. * RedHat: zabbix_web_php_dependencies goes unused * Single task for installing zabbix-apache-conf Also, this zabbix_agent_disable_repo is probably a copy-paste error introduced at some point. * Single task for installing zabbix-nginx-conf * Move installation of debian php deps By converting zabbix_web_http_server_package to a list of one package, we can add zabbix_web_php_dependencies to it during installation of those packages. * Move installation of debian php-pgsql There is a corresponding php-mysql package, and we might aswell make sure that is installed based on zabbix_server_database. * RedHat: Remove zabbix-{{ ..._http_server }}-conf I'm not sure what this when condition is for; when: - zabbix_web_version is version('6.0', '!=') - ansible_distribution_major_version == '9' But we're installing the package anyway. So just drop this task, maybe? * zabbix_underscore_version goes unused * Remove useless when conditions and notifies We don't need restart apache/nginx when a php file gets updated, they are completely separate from running configuration. We can also drop the >= zabbix-5.0 condition. In the rest of the role, we're only calling the respective handler when we need to, so no more when conditions needed. --------- Co-authored-by: Troy W <[email protected]>
pyrodie18
added a commit
to pyrodie18/community.zabbix
that referenced
this pull request
Jun 12, 2024
* Single common task to install zabbix-web RedHat can pin the minor version, and has a toggle to disable repo. Debian can't pin minor version. The other debian option cache_valid_time seems irrelevant? We use the common package module, which is just a wrapper around apt/yum, and use this construction; user_supplied_var | default(_calculated_var | default(omit)) to send additional parameters to the respective modules. The workaround ZBX-10467 is dependent on packages being installed, so had to get moved aswell. * Remove mysql install tasks It doesn't really make sense installing this package on all of RedHat family, when there's no corresponding Debian task doing the same. MySQL/mariadb is gets installed by the zabbix_server role. And the PyMySQL dependency is only needed for the ansible collection community.mysql, which we don't use in this role. * Improve when condition logic for os_family It's better to check for what it IS we're looking for rather than what it's not. You could be looking at 'Suse' at some point, and this logic wouldn't hold. * Swap out remi for appstream php:8.0/common Use dnf module instead of command, this makes the task idempotent. * Make it clear tasks are a workaround Remove ubuntu-22.04 from this workaround, support for ubuntu-22.04 was added in 5.0.0[1]. Not sure if it's a good idea to pin the geerlingguy role? We can pin it to a specific version, but I haven't figured out why it's not respecting a relative version, like '>=5.0.0' [1] geerlingguy/ansible-role-php#406 * Remove zabbix 5.0 related task Zabbix 5 wanted to install the sql-scripts to /usr/share/doc, and this task removes a line in yum.conf that prevented that from happening. But since 6.0 we haven't had to deal with that, so out it goes. * Remove tasks and checks guarding against EL7 This role doesn't support EL7 anymore, so we can remove these tasks/checks. * Move delegated_dbhost to first block This calculation doesn't change. So let's provide/reuse it across the whole block. * Bump mysql container image, add workarounds PyMySQL 9>=,<10 did not have issues with this pinned mysql:8.0.32, but does with later releases, so we bump pymysql for additional debian and ubuntu releases. * Increase similarity in init-mysql between roles * Provide the default port for database servers This could have been an if-statement, I just don't like the "looseness" of the else part, so I opted for a lookup-table. * Remove unused zabbix_selinux_dependencies This lookup goes unused. * Group selinux booleans, and apply regardless Having them only applied with a when condition leaves no way to undo the booleans once set. So we apply them regardless. If the booleans are defaulting to false, we set them false on the system. * Install selinux packages regardless Use the generic `package` module for package installs, and install this ansible dependency regardless. * RedHat: zabbix_web_php_dependencies goes unused * Single task for installing zabbix-apache-conf Also, this zabbix_agent_disable_repo is probably a copy-paste error introduced at some point. * Single task for installing zabbix-nginx-conf * Move installation of debian php deps By converting zabbix_web_http_server_package to a list of one package, we can add zabbix_web_php_dependencies to it during installation of those packages. * Move installation of debian php-pgsql There is a corresponding php-mysql package, and we might aswell make sure that is installed based on zabbix_server_database. * RedHat: Remove zabbix-{{ ..._http_server }}-conf I'm not sure what this when condition is for; when: - zabbix_web_version is version('6.0', '!=') - ansible_distribution_major_version == '9' But we're installing the package anyway. So just drop this task, maybe? * zabbix_underscore_version goes unused * Remove useless when conditions and notifies We don't need restart apache/nginx when a php file gets updated, they are completely separate from running configuration. We can also drop the >= zabbix-5.0 condition. In the rest of the role, we're only calling the respective handler when we need to, so no more when conditions needed. --------- Co-authored-by: Troy W <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
...and remove trusty from meta file since it is already no longer supported
/cc @geerlingguy