Add suse support (with mysql defaults this time) #146

ghormoon · 2021-04-15T23:12:49Z

PR progress checklist (to be filled in by reviewers)

Changes to documentation are appropriate (or tick if not required)
Changes to tests are appropriate (or tick if not required)
Reviews completed

What type of PR is this?

Primary type

[feat] A new feature

Does this PR introduce a `BREAKING CHANGE`?

No.

Related issues and/or pull requests

Replaces #140

Describe the changes you're proposing

Following up on my colleagues work from #140 i made adjustments to the defaults to go with mysql as you prefer. Please let me know if there are any other issues i did not address properly.

myii

Thanks for this PR, @ghormoon. Unfortunately, this is failing with conflicting IDs in the CI. Please uncomment these two lines in order to test on openSUSE during this PR's development:

zabbix-formula/.gitlab-ci.yml

Lines 145 to 146 in 25de37e

    
           # default-opensuse-leap-152-master-py3: {extends: '.test_instance'} 
        
           # default-opensuse-tmbl-latest-master-py3: {extends: '.test_instance'}

I've run these in my fork and you can see the results so far:

       local:
           Data failed to compile:
       ----------
           Detected conflicting IDs, SLS IDs need to be globally unique.
           The conflicting ID is 'zabbix_repo' and is found in SLS 'base:zabbix.agent.repo' and SLS 'base:zabbix.server.repo'
       ----------
           Detected conflicting IDs, SLS IDs need to be globally unique.
           The conflicting ID is 'zabbix_repo' and is found in SLS 'base:zabbix.server.repo' and SLS 'base:zabbix.frontend.repo'

hatifnatt · 2021-04-16T11:31:07Z

zabbix/repo.sls

    - gpgkey: https://repo.zabbix.com/RPM-GPG-KEY-ZABBIX-79EA5ED4

+{%- elif salt['grains.get']('os_family') == 'Suse' %}
+zabbix_repo:


Missed this part, thanks @myii for pointing this issue. Variable must be used in state id to prevent duplicatees.

Suggested change

zabbix_repo:

{{ id_prefix }}_repo:

@hatifnatt Yes, unfortunately that's not enough, getting some strange errors after fixing that:

https://gitlab.com/myii/zabbix-formula/-/jobs/1186085485

https://gitlab.com/myii/zabbix-formula/-/jobs/1186085486

[ERROR ] An un-handled exception was caught by salt's global exception handler: StopIteration: Traceback (most recent call last): File "/usr/bin/salt-call", line 8, in <module> sys.exit(salt_call()) File "/usr/lib/python3.8/site-packages/salt/scripts.py", line 449, in salt_call client.run() File "/usr/lib/python3.8/site-packages/salt/cli/call.py", line 58, in run caller.run() File "/usr/lib/python3.8/site-packages/salt/cli/caller.py", line 112, in run ret = self.call() File "/usr/lib/python3.8/site-packages/salt/cli/caller.py", line 219, in call ret["return"] = self.minion.executors[fname]( File "/usr/lib/python3.8/site-packages/salt/loader.py", line 1235, in __call__ return self.loader.run(run_func, *args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/loader.py", line 2268, in run return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/loader.py", line 2283, in _run_as return _func_or_method(*args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/executors/direct_call.py", line 12, in execute return func(*args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/loader.py", line 1235, in __call__ return self.loader.run(run_func, *args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/loader.py", line 2268, in run return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/loader.py", line 2283, in _run_as return _func_or_method(*args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/modules/state.py", line 1115, in highstate ret = st_.call_highstate( File "/usr/lib/python3.8/site-packages/salt/state.py", line 4527, in call_highstate return self.state.call_high(high, orchestration_jid) File "/usr/lib/python3.8/site-packages/salt/state.py", line 3251, in call_high high, ext_errors = self.reconcile_extend(high) File "/usr/lib/python3.8/site-packages/salt/state.py", line 1627, in reconcile_extend state_type = next(x for x in body if not x.startswith("__")) StopIteration Traceback (most recent call last): File "/usr/bin/salt-call", line 8, in <module> sys.exit(salt_call()) File "/usr/lib/python3.8/site-packages/salt/scripts.py", line 449, in salt_call client.run() File "/usr/lib/python3.8/site-packages/salt/cli/call.py", line 58, in run caller.run() File "/usr/lib/python3.8/site-packages/salt/cli/caller.py", line 112, in run ret = self.call() File "/usr/lib/python3.8/site-packages/salt/cli/caller.py", line 219, in call ret["return"] = self.minion.executors[fname]( File "/usr/lib/python3.8/site-packages/salt/loader.py", line 1235, in __call__ return self.loader.run(run_func, *args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/loader.py", line 2268, in run return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/loader.py", line 2283, in _run_as return _func_or_method(*args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/executors/direct_call.py", line 12, in execute return func(*args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/loader.py", line 1235, in __call__ return self.loader.run(run_func, *args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/loader.py", line 2268, in run return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/loader.py", line 2283, in _run_as return _func_or_method(*args, **kwargs) File "/usr/lib/python3.8/site-packages/salt/modules/state.py", line 1115, in highstate ret = st_.call_highstate( File "/usr/lib/python3.8/site-packages/salt/state.py", line 4527, in call_highstate return self.state.call_high(high, orchestration_jid) File "/usr/lib/python3.8/site-packages/salt/state.py", line 3251, in call_high high, ext_errors = self.reconcile_extend(high) File "/usr/lib/python3.8/site-packages/salt/state.py", line 1627, in reconcile_extend state_type = next(x for x in body if not x.startswith("__")) StopIteration

@ghormoon @hatifnatt Looks like these extend blocks need to handle Suse as well?

https://github.com/saltstack-formulas/zabbix-formula/search?q=extend+filename%3Arepo.sls

@ghormoon I see what the problem is in the repo.sls files (the extends haven't been set correctly). Just trying something in my fork and will link inline with the results.

Yes there is a problem with extend
https://gitlab.com/myii/zabbix-formula/-/jobs/1186085485#L1687-#L1691 it's rendering as simple {} that's does not look like correct yaml.

@hatifnatt Actually, the current code isn't even hitting the {}, it's actually splitting the RedHat block with a Suse block. I've mentioned the fixes below.

myii · 2021-04-16T11:58:03Z

zabbix/agent/repo.sls

+    {% elif salt['grains.get']('os_family') == 'Suse' -%}
+    pkgrepo:
+      - require_in:
+        - pkg: zabbix-agent


Suggested change

{% elif salt['grains.get']('os_family') == 'Suse' -%}

pkgrepo:

- require_in:

- pkg: zabbix-agent

This current method also splits the RedHat block, which will cause problems there.

All this file needs is the line above to be changed:

- {% if salt['grains.get']('os_family') == 'Debian' -%} + {% if salt['grains.get']('os_family') in ['Debian', 'Suse'] -%}

Note, this single line change is also required here:

zabbix-formula/zabbix/server/repo.sls

Line 15 in c9aeb37

{% if salt['grains.get']('os_family') == 'Debian' -%}

myii · 2021-04-16T11:58:52Z

zabbix/frontend/repo.sls

+    {% elif salt['grains.get']('os_family') == 'Suse' -%}
+    pkgrepo:
+      - require_in:
+        - pkg: zabbix-frontend-php


myii · 2021-04-16T11:59:21Z

zabbix/proxy/repo.sls

+    {% elif salt['grains.get']('os_family') == 'Suse' -%}
+    pkgrepo:
+      - require_in:
+        - pkg: zabbix-proxy


myii · 2021-04-16T12:04:17Z

@ghormoon After applying the fixes mentioned above, the good news is that this is almost passing on openSUSE Leap but will need some more work on openSUSE Tumbleweed:

https://gitlab.com/myii/zabbix-formula/-/jobs/1186140108
- Just one test needs fixing.
https://gitlab.com/myii/zabbix-formula/-/jobs/1186140109
- Repo URL would need adjusting.

hatifnatt · 2021-04-16T12:18:04Z

* https://gitlab.com/myii/zabbix-formula/-/jobs/1186140109
  
  * Repo URL would need adjusting.

That's because Tumbleweed grains['osmajorrelease'] != SLES grains['osmajorrelease'] (at least I think so, can't verify).

@ghormoon are you using openSUSE Tumbleweed or SLES?

ghormoon · 2021-04-16T12:23:33Z

I'm using SLES 15, but honestly i didn't run the kitchen test, i just tried it manually :)
the problem is not sles vs opensuse, but the version number. if you look on the log, it shows
https://repo.zabbix.com/zabbix/4.4/sles/20210414/x86_64/

my link is https://repo.zabbix.com/zabbix/5.2/sles/15/x86_64/ on my SLES 15 VM (the 5.2 is because of variable), so the problem is the tumbleweed versioning.
Either we can somehow hack it around if version>15 then version = 15, but the question is, if we want to run it on tumbleweed, as zabbix doesn't have repo for that. 99% the sles 15 repo will work, though.

leap is much closer to sles, so it makes sense to fix the path for files expected in tests

i'll have a look on the suggestions and try to apply and test them locally on sles, i just got to it and you're quite active :)

myii · 2021-04-16T12:25:05Z

That's because Tumbleweed grains['osmajorrelease'] != SLES grains['osmajorrelease'] (at least I think so, can't verify).

@hatifnatt @ghormoon I've tried a quick fix locally (using Kitchen) and there's deeper issues with Tumbleweed, for example:

       [INFO    ] Executing state pkg.installed for [zabbix-server]
       [DEBUG   ] Could not LazyLoad pkg.check_db: 'pkg.check_db' is not available.
       [DEBUG   ] targeting package: zabbix-server-mysql
       [DEBUG   ] Calling Zypper: systemd-run --scope zypper --non-interactive --no-refresh --no-refresh install --auto-agree-with-licenses --name zabbix-server-mysql
       [INFO    ] Executing command ['systemd-run', '--scope', 'zypper', '--non-interactive', '--no-refresh', '--no-refresh', 'install', '--auto-agree-with-licenses', '--name', 'zabbix-server-mysql'] in directory '/root'
       [ERROR   ] Command '['systemd-run', '--scope', 'zypper', '--non-interactive', '--no-refresh', '--no-refresh', 'install', '--auto-agree-with-licenses', '--name', 'zabbix-server-mysql']' failed with return code: 4
       [ERROR   ] stdout: Warning: The flag --no-refresh can only be used once.
       Loading repository data...
       Reading installed packages...
       Resolving package dependencies...
       
       Problem: nothing provides libevent-2.1.so.6()(64bit) needed by zabbix-server-mysql-4.4.10-1.el15.x86_64
        Solution 1: do not install zabbix-server-mysql-4.4.10-1.el15.x86_64
        Solution 2: break zabbix-server-mysql-4.4.10-1.el15.x86_64 by ignoring some of its dependencies
       
       Choose from above solutions by number or cancel [1/2/c/d/?] (c): c
       [ERROR   ] stderr: Running scope as unit: run-r0a507cfdfb0f48a7bca8fd3fdc509a86.scope
       [ERROR   ] retcode: 4
       [ERROR   ] An error was encountered while installing package(s): Zypper command failure: Running scope as unit: run-r0a507cfdfb0f48a7bca8fd3fdc509a86.scope

So let's not worry about Tumbleweed for now and just get Leap-15 up and running. In fact, I'll just fix the test and push the fixes here and then this PR can be merged.

myii · 2021-04-16T12:26:12Z

Just for reference, this was the fix I was going to suggest (but not going to apply now):

{%- elif salt['grains.get']('os_family') == 'Suse' %}
{#-   TODO: When the v5 `map.jinja` is introduced to this formula, this should be provided via. the relevant `oscodename` parameter file #}
{%-   set majorrelease = '15' if grains.get('oscodename', '') == 'openSUSE Tumbleweed' else grains['osmajorrelease'] %}
{{ id_prefix }}_repo:
  pkgrepo.managed:
    - name: zabbix
    - humanname: "Zabbix Official Repository"
    - baseurl: https://repo.zabbix.com/zabbix/{{ zabbix.version_repo }}/sles/{{ majorrelease }}/x86_64/
    - gpgcheck: 1
    - gpgkey: https://repo.zabbix.com/zabbix/{{ zabbix.version_repo }}/sles/{{ majorrelease }}/x86_64/repodata/repomd.xml.key
    - gpgautoimport: True

ghormoon · 2021-04-16T12:28:57Z

yeah, it would be asking for dependency problems like you have in the log sooner or later. even if we were lucky now and it passed, we might get and bug reported in a month or two, so i'd stick to that only leap/sles is supported.
hacking it around like this is like using debian 10 packages on testing/experimental. it might work, but the further it diverges, the higher is the chance of issues

myii · 2021-04-16T12:38:48Z

@ghormoon OK, here's the final bit then, to get Leap-15 passing in GitLab CI:

zabbix-formula/test/integration/default/controls/config_spec.rb

Lines 41 to 48 in 2c67b11

    
           case platform[:family] 
        
           when 'debian' 
        
             server_file_group = 'root' 
        
             server_file_mode = '0644' 
        
             setting_dbsocket = '/var/run/mysqld/mysqld.sock' 
        
           when 'fedora' 
        
             server_file_group = 'zabbixsrv' 
        
           end

Please add the suse block as follows:

  case platform[:family]
  when 'debian'
    server_file_group = 'root'
    server_file_mode = '0644'
    setting_dbsocket = '/var/run/mysqld/mysqld.sock'
  when 'fedora'
    server_file_group = 'zabbixsrv'
  when 'suse'
    setting_dbsocket = '/run/mysql/mysql.sock'
  end

Then this PR should be ready for merging.

Would you prefer to do it or shall I push all of the changes myself?

ghormoon · 2021-04-16T12:56:27Z

ok, i tried to apply the changes and allowing leap on ci, we'll see if it passes now :)
thanks for the help so far

myii · 2021-04-16T14:27:48Z

Thanks, @ghormoon -- all working nicely.
@hatifnatt I went ahead and merged, since you already had got your approval in -- I wouldn't usually get involved when there's a codeowner available, so please don't think you have to wait for me!

saltstack-formulas-travis · 2021-04-16T14:32:53Z

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

* saltstack-formulas/zabbix-formula#146

* Automated using myii/ssf-formula#315 * Completes #146

# [1.2.0](v1.1.0...v1.2.0) (2021-05-11) ### Continuous Integration * add `arch-master` to matrix and update `.travis.yml` [skip ci] ([95523a9](95523a9)) * **kitchen+gitlab:** adjust matrix to add `3003` [skip ci] ([1d237e8](1d237e8)) * **travis:** maintain sync with GitLab CI [skip ci] ([5c81ca1](5c81ca1)), closes [#146](#146) ### Features * **agent:** allow use of string Server and ServerActive ([59dff0a](59dff0a))

ghormoon requested a review from hatifnatt as a code owner April 15, 2021 23:12

hatifnatt approved these changes Apr 16, 2021

View reviewed changes

myii suggested changes Apr 16, 2021

View reviewed changes

hatifnatt requested changes Apr 16, 2021

View reviewed changes

myii reviewed Apr 16, 2021

View reviewed changes

ghormoon force-pushed the suse-support branch from 769515c to 53388e7 Compare April 16, 2021 12:54

feat: add suse support (applied suggestions from hatifnatt and myii)

a245ec4

ghormoon force-pushed the suse-support branch from 53388e7 to a245ec4 Compare April 16, 2021 12:55

ghormoon requested a review from a team as a code owner April 16, 2021 12:55

hatifnatt approved these changes Apr 16, 2021

View reviewed changes

myii approved these changes Apr 16, 2021

View reviewed changes

myii merged commit a8b7982 into saltstack-formulas:master Apr 16, 2021

saltstack-formulas-travis added the released label Apr 16, 2021

ghormoon mentioned this pull request Apr 16, 2021

Add suse support #140

Closed

19 tasks

myii added a commit to myii/ssf-formula that referenced this pull request Apr 16, 2021

feat(zabbix): review PR 146

a5a773f

* saltstack-formulas/zabbix-formula#146

myii mentioned this pull request Apr 16, 2021

feat(zabbix): review PR 146 myii/ssf-formula#315

Merged

myii added a commit that referenced this pull request Apr 16, 2021

ci(travis): maintain sync with GitLab CI [skip ci]

5c81ca1

* Automated using myii/ssf-formula#315 * Completes #146

	# default-opensuse-leap-152-master-py3: {extends: '.test_instance'}
	# default-opensuse-tmbl-latest-master-py3: {extends: '.test_instance'}

Add suse support (with mysql defaults this time) #146

Add suse support (with mysql defaults this time) #146

Uh oh!

Conversation

ghormoon commented Apr 15, 2021

PR progress checklist (to be filled in by reviewers)

What type of PR is this?

Primary type

Does this PR introduce a BREAKING CHANGE?

Related issues and/or pull requests

Describe the changes you're proposing

Uh oh!

myii left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

myii commented Apr 16, 2021

Uh oh!

hatifnatt commented Apr 16, 2021

Uh oh!

ghormoon commented Apr 16, 2021

Uh oh!

myii commented Apr 16, 2021

Uh oh!

myii commented Apr 16, 2021

Uh oh!

ghormoon commented Apr 16, 2021

Uh oh!

myii commented Apr 16, 2021

Uh oh!

ghormoon commented Apr 16, 2021

Uh oh!

myii commented Apr 16, 2021

Uh oh!

saltstack-formulas-travis commented Apr 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Does this PR introduce a `BREAKING CHANGE`?