More granular master job targeting#10374
Conversation
|
Test FAILed. |
|
Test FAILed. |
|
I like where you are going with this, but it does involve sending targeting data in the clear which is why I did not do it this way in the beginning. This is one of the things that we hope to address with RAET |
|
What is the concern with targeting data in the clear? If you send data to
|
|
Also, we could hash or encrypt the target if that's a major concern.
|
|
Test FAILed. |
|
@thatch45 are there any specific concerns about sending the targeting data in the clear? |
|
Yes, targets in the clear means that we compromise the predictability of future packets, as well as leaking what could be sensitive data, especially if the target is a pillar target |
|
The only targeting data in the clear in this implementation is the minion Tldr; as long as minion I'd isn't 'secret' this implementation should be
|
|
Ok, I am in, please rebase and we will give this a spin. |
|
Alright, i'll try to get this in tonight-- otherwise it'll be tomorrow :) |
|
Test FAILed. |
|
The rebase is going to be complicated ;) I'll get it working tomorrow. |
Salt targeting is done using the zmq pub/sub mechanism. No filtering is done so all requests go to all minions. This is an attempt to make the requests more target-able. ZeroMQ supports filters on these sockets which are processed publisher side. So the thought is to set it to the minion id and a shared "broadcast" one. Only requests of tgt_type == list will use this. The thought is if we like this we can use other data (mine etc) to determine a more specific target on the master (such as a list) from a glob match etc. Syndication masters do not have zmq filters applied, but will send out their syndicated requests with a filter as well. So all targeted messages aren't gauranteed to go to only the minion, its just best effort
|
K, lets wait on the jenkins run :) |
|
Test FAILed. |
|
Test FAILed. |
|
Test PASSed. |
|
Test FAILed. |
|
Says build finished, but the details link seems to be empty. |
More granular master job targeting
|
Since I assume you will be testing this more as well and it is not in until Helium I think we should be ok :) |
|
Looks like the commit was reverted, so we shouldnt be causing any more breakage. Do we know exactly what happened? In my testing updating the master then the minion is completely safe with this new feature. If thats not acceptable then I can wrap the minion filtering in a config and we can enable that after a few releases, just want to know whats the plan forward :) |
|
@jacksontj if you want to investigate, the jenkins master is always a little behind on the bootstrapped minions(commit wise), we'll even be enforcing the jenkins master to be on latest stable to find backwards incompatible changes. |
|
Does that mean "backwards compatibility" means it needs to work both ways (new minion old master AND old minion new master)? |
|
@jacksontj now that's really a question for @thatch45... |
|
@jacksontj We don't ever guarantee that new minions will be able to talk to an old master. We do our best to make new masters talk to old minions, however. |
|
@basepi If that's the case then the pull req should have succeeded. Do we guarantee
|
|
@s0undt3ch So, i want to sync up with you before we try this again. Sounds like the CI tooling doesn't enforce the same compatibility model-- if that's correct I'll just commit the master side (which is completely backwards compatible both ways) and then config wrap the minion change and commit it later. IMO we should probably update the CI stack to enforce the same compatibility model that we want to follow for release, but I'm not sure how hard that is. |
|
@jacksontj sorry for my slow response but work has caught me up a bit. I'll get back to this when I have the necessary time. Again, sorry for my slow response. |
|
Too much time has passed without me replying to you. Sorry! The salt-master used by Jenkins is now "locked" on latest stable release, 2014.1.0. |
|
That master is seperate from the one actually running the tests right? If i understand that master just spawns the other box which runs the tests? If so, then that would work fine for me :) |
|
The jenkins master spawns new vms, and uses the bootstrapped minion(which is based on the commit being tests) to run the tests. Cool? |
|
That makes it sound like we only run the minion tests? Do we ever run tests with the master code as well? Also, if that is the case (new minion old master) isn't that the exact opposite of our support model? |
|
This is not the ideal situation, as you already know. We should cover all use cases. New master, old minion, the other way around, etc... To do that, you'd need to orchestrate both deployments and run some tests on those deployments. We'd need to bootstrap, an older master, a newer minion, a newer master, an older minion, etc. Have them communicate and run some tests on all of these. Such orchestration is an awesome use case for salt. I've been implementing an orchestration strategy in order to test Halite on our jenkins server, you can have a look here, under The same could be achieved for this specific use case, and it will be somewhere in the future. So in conclusion. Yes, we are testing the opposite of our model(upgrade master first) but we're actually getting a heads up way earlier if we break backwards communication compatibility allowing us to properly warn the user on the release notes, ie, "this release it's specifically important that the master be upgraded first". |
|
So with this as the case how do I ever have the Jenkins stack test the
|
|
@s0undt3ch Any update on how to move forward? |
|
About Jenkins, the only, current, option is to wait until we setup orchestration, at which time we will be able to test against specific versions. |
|
Even if i do the config switch we'll have to get the new master done first... |
|
@s0undt3ch any update on this? |
|
Not yet, still on the TODO. |
|
@s0undt3ch ping :) |
|
@jacksontj sorry for the looong time to reply... Still no news, still preparing the LXC jenkins setup which will simplify this. |
|
OK, let me know when we are ready to give this another shot. This is going
|
Salt targeting is done using the zmq pub/sub mechanism. No filtering is done so all requests go to all minions. This is an attempt to make the requests more target-able. ZeroMQ supports filters on these sockets which are processed publisher side. So the thought is to set it to the minion id and a shared "broadcast" one. Only requests of tgt_type == list will use this. The thought is if we like this we can use other data (mine etc) to determine a more specific target on the master (such as a list) from a glob match etc.
This is more or less backwards compatible, during my testing it still works but the old minion will throw an exception like:
[CRITICAL] An exception occurred while polling the minion
Traceback (most recent call last):
File "/home/thjackso/src/salt/salt/minion.py", line 1324, in tune_in
payload = self.serial.loads(self.socket.recv(zmq.NOBLOCK))
File "/home/thjackso/src/salt/salt/payload.py", line 95, in loads
return msgpack.loads(msg, use_list=True)
File "_unpacker.pyx", line 114, in msgpack._unpacker.unpackb (msgpack/_unpacker.cpp:114)
ExtraData: unpack(b) recieved extra data.
So if we decide this is a good idea we'll probably want to wrap the new master side logic in a config option, and just merge in the minion side changes for the first release.
This is another attempt of saltstack#10374.
4 participants
Please DO NOT MERGE IMMEDIATELY.
Salt targeting is done using the zmq pub/sub mechanism. No filtering is done so all requests go to all minions. This is an attempt to make the requests more target-able. ZeroMQ supports filters on these sockets which are processed publisher side. So the thought is to set it to the minion id and a shared "broadcast" one. Only requests of tgt_type == list will use this. The thought is if we like this we can use other data (mine etc) to determine a more specific target on the master (such as a list) from a glob match etc.
This is more or less backwards compatible, during my testing it still works but the old minion will throw an exception like:
So if we decide this is a good idea we'll probably want to wrap the new master side logic in a config option, and just merge in the minion side changes for the first release.