fix(minion): Fix version compare in minion.sls

[sticky-note]

No description provided.

[noelmcloughlin]

LGTM, thanks @sticky-note

[daks]

LGTM too, but Salt versions < 2017 are not supported anymore (cf https://s.saltstack.com/product-support-lifecycle/) so I think this patch is not useful.

[sticky-note]

@daks, @noelmcloughlin .. So we can also remove this entire test.
Without this patch, and with 2019.2.0, we are not matching to the
grains['saltversioninfo'][0] >= 2016 and grains['saltversioninfo'][1] >= 3 portion
which is not desirable according to the comments # old style, pre 2016.3. fork and disown the process and by the way, it doesn't work on FreeBSD 12 minions, version 2019.2.0.
What is the best thing to do ?

[myii]

Already covered by #416.

[sticky-note]

@myii @noelmcloughlin. I'll close this when #416 get merged but if it'll take time, can we merge this and rebase #416 with this modification ?

[noelmcloughlin]

This is approved - Yes we should merge - comments can be addressed in #416

[myii]

@sticky-note Let's improve this implementation and get this merged:

-  {%- if grains['saltversioninfo'][0] >= 2016 and grains['saltversioninfo'][1] >= 3 %}
+  {%- if grains['saltversioninfo'] > [2016, 3] %}

CC: @noelmcloughlin @daks.

[daks]

@sticky-note Let's improve this implementation and get this merged:

-  {%- if grains['saltversioninfo'][0] >= 2016 and grains['saltversioninfo'][1] >= 3 %}
+  {%- if grains['saltversioninfo'] > [2016, 3] %}

CC: @noelmcloughlin @daks.

Do we want to support Salt < 2017? which itself is not supported par Saltstack itself?
If not, then no need to make any verification.

[myii]

@daks I've actually removed the verification in #416 but that is stuck for merging right now. Others want this PR merged so I'm suggesting a cleaner solution for the meantime, even though it references older Salt versions for the time being.

[sticky-note]

supported par Saltstack

@daks You es French ? ^^

@myii Rebase done.
I've left the >= operator because we want 2016.3.0 to be matched by the test.
Is grains['saltversioninfo'] >= [ 2016, 3 ] equivalent with grains['saltversioninfo'][0] >= 2016 and grains['saltversioninfo'][1] >= 3

[myii]

@sticky-note The following is sufficient by itself, no need for the 2017 clause:

-  {%- if grains['saltversioninfo'][0] >= 2017
-      or grains['saltversioninfo'] >= [ 2016, 3 ] %}
+  {%- if grains['saltversioninfo'] >= [2016, 3] %}

[sticky-note]

The PR is about that
-> version 2019.2.0 don't enter in the the condition grains['saltversioninfo'][0] >= 2016 and grains['saltversioninfo'][1] >= 3 because grains['saltversioninfo'][1] is less than 3. It is why i put this first condition.

[myii]

The PR is about that
-> version 2019.2.0 don't enter in the the condition grains['saltversioninfo'][0] >= 2016 and grains['saltversioninfo'][1] >= 3 because grains['saltversioninfo'][1] is less than 3. It is why i put this first condition.

@sticky-note Forget the old method and try this by itself:

{%- if grains['saltversioninfo'] >= [2016, 3] %}

You'll find it works for 2019.2, etc.

[daks]

@myii ok then.

@sticky-note yes

[myii]

Thanks, @sticky-note. I would have preferred it as [2016, 3] (without the extra spaces) but this should be fine for now.

[saltstack-formulas-travis]

🎉 This PR is included in version 0.58.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀