saltstack-formulas / zabbix-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
50 stars 121 forks source link

Breaking Change: Update default version to 3.0 LTS #123

Open xenadmin opened 4 years ago

xenadmin commented 4 years ago

As suggested by @hatifnatt in #87 we should finally switch to 3.0 LTS, as 4.0 LTS is already available.

What do the others think? =)

myii commented 4 years ago

@xenadmin @hatifnatt We've found dealing with breaking changes much easier after implementing semantic-release in formulas (we have around 45 done now). For example, promoting nginx.ng to be nginx was not a trivial task and it was made much easier by having the tags and changelog updated automatically. Obviously this PRs changeset isn't as radical but it's still a breaking change; it would be good to have that clearly defined. Are you ready for that to be implemented for this formula as well? It can be rolled out before this PR is merged, giving a clean before and after for end users.

Another benefit to this process is that it will introduce Kitchen + InSpec + Travis, so automatic testing will be introduced as well.

xenadmin commented 4 years ago

I have a suggestion: Would you @myii mind opening a new issue for your suggestion? There is a discussion here here and it was in PR #113 and I would believe it doesn't fit there. Isn't it more of an issue with this formula? I'm just some random guy who started using salt a while back, to solve his challenges with an increasing number of Zabbix Proxies. During the adoption of this formula I was able to offer various improvements, but I would still call myself a novice concerning salt formulas and GitHub. But I would say, that semantic-releases sound like a great way to support rapid PR approvement and while also allowing breaking changes. I have no idea what we are up to, but I could try to help, as I plan to use this formula even more in the future and I'm very interested in it's development and stability.

myii commented 4 years ago

@xenadmin When there's a formula with specific active contributors, it's useful to get their opinions before rolling this out. To be honest, with 45 formulas done, zabbix-formula is one of the biggest formulas that hasn't been converted yet. I could start a new issue but it seemed especially pertinent to this PR's discussion, since it involves a breaking change.

xenadmin commented 4 years ago

@myii It was just a suggestion, we can gladly talk here about it. ;-) As I'm a contributor and not a maintainer, I appreciate your suggestion. Can you give me a link to a guide where it's detailed what I should have to do different in the future?

myii commented 4 years ago

@xenadmin This is pretty much it:

When we first started rolling this out, I was concerned that it would become problematic for contributors and maintainers. However, it's working out really well, where maintainers are able to make the final adjustments if necessary.

hatifnatt commented 4 years ago

@myii I already expressed my thoughts here. Generally I have nothing against semantic-release if somebody take burden of supporting all that stuff :)

And about this breaking change I may be wrong, but I can hardly imagine someone who is still using Zabbix 2.2 and in the same time uses this formula 'defaults' to determine which version will be used on this person production servers.

Also general approach of using public formulas - always assume that everything can be broken after each update, so make a fork and use it instead of directly using formula repository and blindly making git pull or using gitfs.

myii commented 4 years ago

@hatifnatt Even if the contributors do nothing at all, it doesn't prevent merging to the repo. All it does is not automate another release and update the changelog. But the team have shown they are happy to help and they are able to make changes at the time of merging, to ensure a release is produced.

myii commented 4 years ago

@xenadmin semantic-release including InSpec-based testing has been implemented in this formula now (#130). Please rebase your changes onto the latest version so that this PR can benefit from it.

Also have a look at the section explaining breaking changes. The rest of the document is useful, too.

xenadmin commented 4 years ago

@myii @hatifnatt It’s funny and positive to see, that this formula gets so much positive attention, I really like that and I would really like to help, but ironically I’m at Zabbix Summit 2019 in Riga right now xD No chance to follow on GitHub during the conference.

But to let you know how much I love Saltstack and this formula, yesterday I finally upgraded 27 Zabbix Proxies at the same time from 4.0.13 to 4.2.7 without any errors. 5 Proxies still not allow 4505-4506 TCP sadly. Yes the formula is clunky, yes there are several strange workarounds and the formula is complicated to adopt, but that doesn’t matter, because after 1yr of learning and reading the docs of Salt and finally starting to commit to this formula to help, I’m finally on 4.2 without any issues!

I will write back in my other issues and PRs as soon as I’m back in Germany. Because being on 4.2 just means we have to prepare for 4.4, am I right?!

myii commented 4 years ago

I will write back in my other issues and PRs as soon as I’m back in Germany. Because being on 4.2 just means we have to prepare for 4.4, am I right?!

@xenadmin Good news for you: the automated testing is already running 4.4:

https://github.com/saltstack-formulas/zabbix-formula/blob/8eca128182bf82aa076866505b3ce33c50854847/test/salt/pillar/default.sls#L6-L7

https://github.com/saltstack-formulas/zabbix-formula/blob/4facac64df15283870ec5d57df4e862c39cf2b8d/test/integration/default/controls/packages_spec.rb#L12-L42

While it doesn't test everything, it looks like a good platform for getting 4.4 out there. Looking forward to your PRs!

myii commented 4 years ago

Thanks to @absmith82, we've now split the monolithic map.jinja into multiple YAML files, similar to many of the other formulas. That makes it much easier to modify the versions across the various platforms. Looking at the link that @hatifnatt shared in the linked issue, wouldn't it be better to move straight to 4.0 as the basis now?

hatifnatt commented 4 years ago

Maybe it worth to wait for next LTS version

Zabbix 5.0 LTS Expected Q1, 2020

v 4.0 is really lacking a lot of sweet features in comparsion with v 4.4. Generally I think that change of default version in formula can't be considered as "breaking chanage" users of the formula must not rely on defaults in case of version parameter.

absmith82 commented 4 years ago

The best part about the yaml maps is that it would be easy to set the default version to the maximum or minimum supported version for each version of os. So we could make the default version work for any system that supports zabbix with some version.

absmith82 commented 4 years ago

I would like to change the default verison to the latest LTS version available. Then we also will have the issue that some OS versions will not support the latest LTS, in those cases I would like to propose we set the OS finger version to match the latest possible version. The question there becomes where do we cut off on OS Finger versions? Do we use only currently supported versions, LTS versions etc.. let me know what you think.

myii commented 4 years ago

Useful resource here: https://www.zabbix.com/release_notes. Currently up to 5.0.0alpha4.

myii commented 4 years ago

@xenadmin @hatifnatt @absmith82 Just saw this in Telegram a short while back:

https://t.me/ZabbixTech/14417

Alexei Vladishev, [12.05.20 08:24] Zabbix 5.0 LTS was just released! Learn more at https://www.zabbix.com/whats_new_5_0 😀

So, who fancies taking this on?!

hatifnatt commented 4 years ago

@myii I think I'll wait for the first minor update :) But in the context of the formula - it's time to move on.

xenadmin commented 3 years ago

We should get this discussion back to life. Zabbix-Formula is still on 2.2 which is a shame, while 3.0 LTS is old by now, 4.0 LTS is rather cool, but 5.0 LTS is a big step and is already stable.

hatifnatt commented 3 years ago

I can't see any problems preventing update of the default version number. I don't believe that anybody is really running 2.2 by now, relying on formulas default version. On the other hand, it's also required to update configuration templates, for agent, server and proxy, and that's a much bigger deal.

Few words about my personal experience. I have updated one of my servers to 5.0 and I don't like some parts of the new interface. Overall it's not bad, but they removed "Graphs" section which I personally used a lot. See ZBXNEXT-6031, ZBXNEXT-5947

xenadmin commented 3 years ago

On the other hand, it's also required to update configuration templates, for agent, server and proxy, and that's a much bigger deal.

I got permission from my boss to spent more time with our Zabbix installation in the upcoming weeks, now that other projects are finished. Which in return means, that I can update the three .jinja files to 5.0 (or even 5.2?).