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

Support fort Zabbix 7.2 #981

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Safranil
Copy link
Contributor

@Safranil Safranil commented Dec 14, 2024

Pull Request (PR) description

Zabbix 7.2 was published some day ago and some structural change was made by Zabbix Team:

  • The repository base URL changed to https://repo.zabbix.com/zabbix/7.2/release/<os> ;
  • The PHP UI directory change to /usr/share/zabbix/ui ;
  • The SQL script directory change to /usr/share/zabbix/sql-scripts.

This Pull Request (PR) fixes the following issues

No issue was opened.

@Safranil Safranil marked this pull request as draft December 14, 2024 23:08
@Safranil Safranil force-pushed the master branch 2 times, most recently from f104712 to e5147c9 Compare December 14, 2024 23:36
@Safranil Safranil changed the title Repository URL change since Zabbix 7.2 Support fort Zabbix 7.2 Dec 14, 2024
@Safranil Safranil force-pushed the master branch 2 times, most recently from 08a00e2 to 2496d29 Compare December 15, 2024 00:03
@Safranil Safranil marked this pull request as ready for review December 15, 2024 00:30
@Valantin
Copy link
Contributor

Hi, actually the module test supports only LTS release of zabbix, due to the short release schedule of that standard one.
I suggest you to change the PR to add parameter to support 7.2 without add explicit check for it.

Copy link
Contributor

@Valantin Valantin left a comment

Choose a reason for hiding this comment

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

use the variable instead add new complexity for standar release every 6 month

@@ -24,7 +24,9 @@
assert_private()

if ($database_schema_path == false) or ($database_schema_path == '') {
if versioncmp($zabbix_version, '6.0') >= 0 {
if versioncmp($zabbix_version, '7.2') >= 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just use $database_schema_path?

@@ -25,7 +25,9 @@

if $database_schema_path != false and $database_schema_path != '' {
$schema_path = $database_schema_path
} elsif versioncmp($zabbix_version, '6.0') >= 0 {
} elsif versioncmp($zabbix_version, '7.2') >= 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as for mysql

@@ -37,7 +37,12 @@
}

$_repo_location = $repo_location ? {
undef => "https://repo.zabbix.com/zabbix/${zabbix_version}/rhel/${majorrelease}/\$basearch/",
undef => versioncmp($zabbix_version, '7.2') ? {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can specify $repo_location

@@ -258,6 +258,13 @@

# Is set to true, it will create the apache vhost.
if $manage_vhost {
$zabbix_ui_dir = versioncmp($zabbix_version, '7.2') ? {
Copy link
Contributor

Choose a reason for hiding this comment

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

set zabbix_ui_dir as a parameter with the default as /usr/share/zabbix

@@ -23,7 +23,9 @@

supported_versions.each do |zabbix_version|
# path to sql files on Debian and RedHat
path = if Puppet::Util::Package.versioncmp(zabbix_version, '6.0') >= 0
path = if Puppet::Util::Package.versioncmp(zabbix_version, '7.2') >= 0
Copy link
Contributor

Choose a reason for hiding this comment

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

no test for standard release

@@ -28,7 +28,9 @@
end

supported_versions.each do |zabbix_version|
path = if Puppet::Util::Package.versioncmp(zabbix_version, '6.0') >= 0
path = if Puppet::Util::Package.versioncmp(zabbix_version, '7.2') >= 0
Copy link
Contributor

Choose a reason for hiding this comment

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

no test for standard release

@@ -137,6 +137,22 @@
it { is_expected.to contain_yumrepo('zabbix-nonsupported').with_gpgkey('https://repo.zabbix.com/RPM-GPG-KEY-ZABBIX-B5333005') } if facts[:os]['release']['major'].to_i < 9
it { is_expected.to contain_yumrepo('zabbix-nonsupported').with_gpgkey('https://repo.zabbix.com/RPM-GPG-KEY-ZABBIX-08EFA7DD') } if facts[:os]['release']['major'].to_i >= 9
end

Copy link
Contributor

Choose a reason for hiding this comment

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

no test for standard release

@@ -1,7 +1,7 @@
# frozen_string_literal: true

def supported_versions
supported_versions = %w[5.0 6.0 7.0]
supported_versions = %w[5.0 6.0 7.0 7.2]
Copy link
Contributor

Choose a reason for hiding this comment

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

remove 7.2 because we only test LTS release

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