Add suse support #140

ricardoklein · 2020-05-25T13:27:47Z

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

[build] Changes related to the build system
[chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
[ci] Changes to the continuous integration configuration
[feat] A new feature
[fix] A bug fix
[perf] A code change that improves performance
[refactor] A code change that neither fixes a bug nor adds a feature
[revert] A change used to revert a previous commit
[style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

[docs] Documentation changes
[test] Adding missing or correcting existing tests

Does this PR introduce a `BREAKING CHANGE`?

No.

Related issues and/or pull requests

Describe the changes you're proposing

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

Updated the README (e.g. Available states).
Updated pillar.example.

Testing checklist

Included in Kitchen (i.e. under state_top).
Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
Updated the relevant test pillar.

Additional context

If you want, we can also use this pillar example to show how to use this with other formulas to have a complete zabbix server setup: https://gist.github.com/kleinstuff/65929dd3023fba8cc7e7c995a2d6bbf5

* osfinger, osfamily and all maps * Added another condition on repo.sls to properly set the repository for (open)SUSE (leap)15 and 12.

* Forgot to set the zabbix-web-<dbtype> to pgsql

* Changed defaults and osfammilymap to support SUSE * Tested against openSUSE 15.1 and openSUSE 15.2

pull-assistant · 2020-05-25T13:27:50Z

Best reviewed: commit by commit

Optimal code review plan

feat(SUSE): Added (open)SUSE support on base files

bugfix(osfamilymap): Set everything to pgsql

fix(SUSE-support): SLE and Leap working on 15.1 and 15.2 beta

Powered by Pull Assistant. Last update 52ded9d ... d343b29. Read the comment docs.

ricardoklein · 2020-05-25T13:30:03Z

About the lint error... I think this may be caused by using SUSE and SLE in upper case, but, well, this is how it is...

myii · 2020-06-16T13:57:34Z

@kleinstuff Apologies for the delay. Let's ask if any of the regular contributors here can have a look at this PR: @hatifnatt @xenadmin @absmith82 -- can any of you help out here?

hatifnatt

This PR looks more like a new feature than a bug fix. Overall I think this PR need some improvements before it can be merged.

hatifnatt · 2020-06-16T16:05:43Z

zabbix/defaults.yaml

 ---
 zabbix:
-  version_repo: 2.2
+  version_repo: 5.0


Discussion about setting v 5 as default is going on here #123

I disagree with this as part of the merge. While I agree it needs to happen, this should be in a different pull request.

hatifnatt · 2020-06-16T16:10:40Z

zabbix/defaults.yaml

  server:
    pkgs:
-      - zabbix-server-mysql
+      - zabbix-server-pgsql


You can't change global defaults and call it 'non breaking change'. Also I can't understand why you ever touch global defaults if you have Suse section in osfamilymap.yaml.

I disagree with changing this portion of the defaults file altogether. There is a whole portion of this formula to deal with postgres instead of mysql. Mysql is still valid, and there is no reason to change this in the defaults.

hatifnatt · 2020-06-16T16:11:20Z

zabbix/osfamilymap.yaml

+      - /etc/zabbix/zabbix_agentd.d/*.conf
+  server:
+    pkgs:
+      - zabbix-server-pgsql


You set package to PostgreSQL version.

unless mysql is not available in Suse, which I imagine it is, these changes need to be made in the zabbix.pgsql portion of the formula, or can be modified in your own pillar files.

hatifnatt · 2020-06-16T16:13:59Z

zabbix/osfamilymap.yaml

+      - zabbix-server-pgsql
+    service: zabbix-server
+    config: /etc/zabbix/zabbix_server.conf
+    dbsocket: /var/lib/mysql/mysql.sock


But you don't set dbsocket to empty string as required for PostgreSQL DB type. Also you don't set dbtype: POSTGRESQL in frontend section below.

unless mysql is not available in Suse, which I imagine it is, these changes need to be made in the zabbix.pgsql portion of the formula, or can be modified in your own pillar files.

hatifnatt · 2020-06-16T16:21:01Z

zabbix/repo.sls

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

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


id_prefix must be used in state id, otherwise there will be conflicts between id-s i.e. when Zabbix server and Zabbix agent will be installed simultaneously.

ricardoklein · 2020-06-16T19:24:58Z

OK, it's my second PR for formulas, and, I need to admit that it was indeed not a "clean" work.
Thank you for your time, I will fix this and reopen it.

myii · 2020-06-16T23:36:26Z

Thanks for all of the feedback.

@kleinstuff For the PostgreSQL stuff, I reckon we should put your stuff in a test suite specific to PostgreSQL -- we don't have that right now. So don't feel like you have to throw out the baby with that bathwater! It's still useful stuff, we just need to use it the right way. If you need a hand with that, feel free to ping me and I'll give you some assistance with that.

ghormoon · 2021-04-16T14:37:31Z

I suggest closing this and consider it resolved by #146

hatifnatt · 2021-04-16T14:51:41Z

As suggested closed in favor of #146

Ricardo Klein added 3 commits May 5, 2020 13:21

feat(SUSE): Added (open)SUSE support on base files

52ded9d

* osfinger, osfamily and all maps * Added another condition on repo.sls to properly set the repository for (open)SUSE (leap)15 and 12.

bugfix(osfamilymap): Set everything to pgsql

8be5857

* Forgot to set the zabbix-web-<dbtype> to pgsql

fix(SUSE-support): SLE and Leap working on 15.1 and 15.2 beta

d343b29

* Changed defaults and osfammilymap to support SUSE * Tested against openSUSE 15.1 and openSUSE 15.2

hatifnatt reviewed Jun 16, 2020

View reviewed changes

ghormoon mentioned this pull request Apr 15, 2021

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

Merged

4 tasks

hatifnatt closed this Apr 16, 2021

Add suse support #140

Add suse support #140

Uh oh!

Conversation

ricardoklein commented May 25, 2020

PR progress checklist (to be filled in by reviewers)

What type of PR is this?

Primary type

Secondary type

Does this PR introduce a BREAKING CHANGE?

Related issues and/or pull requests

Describe the changes you're proposing

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

Testing checklist

Additional context

Uh oh!

pull-assistant bot commented May 25, 2020

Uh oh!

ricardoklein commented May 25, 2020

Uh oh!

myii commented Jun 16, 2020

Uh oh!

hatifnatt 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!

ricardoklein commented Jun 16, 2020

Uh oh!

myii commented Jun 16, 2020

Uh oh!

ghormoon commented Apr 16, 2021

Uh oh!

hatifnatt commented Apr 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Does this PR introduce a `BREAKING CHANGE`?