nodegroups not backwards compatible

[Mrten]

With a 2015.5 master on a 2014.1 minion I get this error when targeting a nodegroup:

2015-06-13 14:21:41,245 [salt.minion      ][ERROR   ] Invalid compound target: ( L@idd0010,download,htmlpdf,minify,osm-1,syslog,upload,webstats )

I don't have the braces configured:

nodegroups:
  'vm10':    'L@idd0010,download,htmlpdf,minify,osm-1,syslog,upload,webstats'

[basepi]

Does it work when you use that target as a compound match instead of nodegroup?

salt -C 'L@idd0010,download,htmlpdf,minify,osm-1,syslog,upload,webstats' test.ping

[basepi]

Additionally, is that error master side or minion side?

[Mrten]

Yes it does, that's how I worked around it. The error is minion-side (as you can see from the logline :)
If a nodegroup has mixed 12.04 and 14.04 minions, the 14.04 minions answer. It's probably the braces...

[basepi]

Did the logline always have the salt.minion? I swear when I looked at this earlier it didn't.

The problem is that 2014.1 is definitely not going to be getting any new releases, so I'm not sure what fixing this would gain us. If it's working fine for 2014.7 and 2015.5 minions, then I'm tempted to mark this as "Won't Fix"

[Mrten]

Cross my heart!

My suggestion would be to fix 2015.5; the extra braces it sends, if the logmessage from 2014 is correct, are not necessary. Extra braces also violate the 'do as I say'-expectation.

This would gain you trust that people that are upgrading more slowly because of previous stability issues aren't left behind. :) (Also, I currently cannot upgrade because of #24525)

This is 2014.1:

mrten@idd0010:/usr/share/pyshared/salt$ fgrep -R 'Invalid compound' *
minion.py:            log.error('Invalid compound target: {0}'.format(tgt))
utils/minions.py:                                log.error('Invalid compound expr (unexpected '
utils/minions.py:                log.error('Invalid compound target: {0}'.format(expr))

So 2014.1 really seems to log the sent-by-2015.5 compound target.

If I'd had to guess, I think the problem is in utils/minions.py, in the _check_compound_minions function, that one always generates outer braces for matchers. From a master debug log (salt-master -l debug):

[INFO    ] Clear payload received with command publish
[DEBUG   ] Evaluating final compound matching expr: ( set(['idd0010', 'minify', 'webstats', 'osm-1', 'upload', 'htmlpdf', 'syslog', 'download']) )
[DEBUG   ] Sending event - data = {'_stamp': '2015-06-18T07:41:26.053338', 'minions': ['idd0010', 'minify', 'webstats', 'osm-1', 'upload', 'htmlpdf', 'syslog', 'download']}
[DEBUG   ] Sending event - data = {'tgt_type': 'compound', 'jid': '20150618094126053060', 'tgt': '( L@idd0010,download,htmlpdf,minify,osm-1,syslog,upload,webstats )', '_stamp': '2015-06-18T07:41:26.053565', 'user': 'root', 'arg': [], 'fun': 'test.ping', 'minions': ['idd0010', 'minify', 'webstats', 'osm-1', 'upload', 'htmlpdf', 'syslog', 'download']}
[DEBUG   ] Evaluating final compound matching expr: ( set(['idd0010', 'minify', 'webstats', 'osm-1', 'upload', 'htmlpdf', 'syslog', 'download']) )
[INFO    ] User root Published command test.ping with jid 20150618094126053060
[DEBUG   ] Published command details {'tgt_type': 'compound', 'jid': '20150618094126053060', 'tgt': '( L@idd0010,download,htmlpdf,minify,osm-1,syslog,upload,webstats )', 'ret': '', 'user': 'root', 'arg': [], 'fun': 'test.ping'}

[basepi]

Yeah, that would probably work. Thanks for the recommendation, we'll investigate this.

[Mrten]

Thanks!

[mexicarne]

Hello, any movement on this one? This issue preventing us using 2015.x minions.

[basepi]

@mexicarne I was under the impression this prevented 2015.5 masters from talking to 2014.1 minions via nodegroup matches, not vice versa.

In any case, this one slipped through the cracks. I'm going to look into it myself sometime in the next week and see if we need to fix anything for 2015.8.2. Unfortunately, 2015.5 is EOL for open source support; there's likely not going to be any further releases on that branch, since 2015.8 is in full swing.

[ahammond]

So... the migration plan is "upgrade everything to 2015.8 all at the same
time"? Because... uh... we can do that, but it ain't gonna be fun.

On Fri, Oct 16, 2015 at 3:10 PM, Colton Myers notifications@github.com
wrote:

@mexicarne https://github.com/mexicarne I was under the impression this
prevented 2015.5 masters from talking to 2014.1 minions via nodegroup
matches, not vice versa.

In any case, this one slipped through the cracks. I'm going to look into
it myself sometime in the next week and see if we need to fix anything for
2015.8.2. Unfortunately, 2015.5 is EOL for open source support, there's
likely not going to be any further releases on that branch, since 2015.8 is
in full swing.

—
Reply to this email directly or view it on GitHub
#24660 (comment).

[basepi]

2015.5 works fine. It's just nodegroup targeting across that gap that doesn't work particularly well.

Sorry we dropped the ball here. I'll bring up the possibility of shipping a 2015.5.7.

[ahammond]

For this I will buy you all expensive craft beers (or tasty beverages of
your choice) at SaltConf. Heck, I'd do that anyway, but this would really
help. :)

On Fri, Oct 16, 2015 at 3:40 PM, Colton Myers notifications@github.com
wrote:

2015.5 works fine. It's just nodegroup targeting across that gap that
doesn't work particularly well.

Sorry we dropped the ball here. I'll bring up the possibility of shipping
a 2015.5.7.

—
Reply to this email directly or view it on GitHub
#24660 (comment).

[basepi]

Actually, the more I catch up on this, the more I feel like this should be a non-issue. I've been catching up on #26107, and it appears that 2014.7 is compatible with 2015.5 in nodegroups. So if you just upgrade to 2014.7 and then to 2015.5 everything should be fine. It's the jump from 2014.1 to 2015.5 that appears to be the issue......

/me continues to look

[basepi]

So I've been doing a lot of testing this morning. I spun up a minion for 2015.8.1, 2015.5.6, and 2014.7.6, and then tried different master versions controlling them.

• 2014.7.6 master works great with all three, including nodegroups.
• 2015.5.6 master works great with all three, including nodegroups.
• 2015.8.1 master is causing a weird error on older minions when nodegroups are used

This is good news, because if I can track down what broke between 2015.5 and 2015.8 and patch it, then 2015.8.2 should be good to go for the upgrade path.

I did not test against 2014.1, but I'm guessing it's not compatible with 2015.5 and so you should upgrade through 2014.7 and then to 2015.5.

[basepi]

Ah, 2015.8 starting converting all nodegroup matches to lists as part of the nodegroup expansion. If I just return the original string in the case of no nodegroup expansion, backwards compat works. Fix incoming.

[basepi]

As of #28148, here is the current situation for backwards compat.

• 2014.7.6 master works great with all three, including nodegroups.
• 2015.5.6 master works great with all three, including nodegroups.
• 2015.8.2 master works great with all three, including nodegroups. (but not nested nodegroups)

Once that fix is merged, I am going to close this. If you're still on 2014.1, please upgrade to 2014.7, then the upgrade path should be clear from there.