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

Breaking Change: Update default version to 3.0 LTS #123

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

Conversation

xenadmin
Copy link
Contributor

@xenadmin xenadmin commented Oct 2, 2019

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
Copy link
Member

myii commented Oct 2, 2019

@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
Copy link
Contributor Author

xenadmin commented Oct 7, 2019

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
Copy link
Member

myii commented Oct 7, 2019

@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
Copy link
Contributor Author

xenadmin commented Oct 7, 2019

@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
Copy link
Member

myii commented Oct 7, 2019

@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
Copy link
Collaborator

@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
Copy link
Member

myii commented Oct 7, 2019

@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
Copy link
Member

myii commented Oct 12, 2019

@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
Copy link
Contributor Author

@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
Copy link
Member

myii commented Oct 13, 2019

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:

lookup:
version_repo: '4.4'

version =
case platform[:name]
when 'debian'
if os[:release].start_with?('10')
'1:4.4.0-1+buster'
elsif os[:release].start_with?('9')
'1:4.4.0-1+stretch'
elsif os[:release].start_with?('8')
'1:4.4.0-1+jessie'
end
when 'ubuntu'
if os[:release].start_with?('18')
'1:4.4.0-1+bionic'
elsif os[:release].start_with?('16')
'1:4.4.0-1+xenial'
end
when 'centos'
if os[:release].start_with?('8')
'4.4.0-1.el8'
elsif os[:release].start_with?('7')
'4.4.0-1.el7'
elsif os[:release].start_with?('6')
'4.4.0-1.el6'
end
when 'fedora'
if os[:release].start_with?('30')
'4.0.11-1.fc30'
elsif os[:release].start_with?('29')
'3.0.28-1.fc29'
end
end

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

@myii
Copy link
Member

myii commented Mar 22, 2020

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
Copy link
Collaborator

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
Copy link
Contributor

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
Copy link
Contributor

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 myii mentioned this pull request Mar 31, 2020
@myii
Copy link
Member

myii commented Apr 10, 2020

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

@myii
Copy link
Member

myii commented May 12, 2020

@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
Copy link
Collaborator

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

@hatifnatt hatifnatt mentioned this pull request Jun 16, 2020
19 tasks
@xenadmin
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

xenadmin commented Nov 1, 2020

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?).

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.

4 participants