Cloud roster fixes #40201
Cloud roster fixes #40201
Conversation
A path to salt-cloud config file is hardcoded into the cloud roster, so it won't work with custom conf dir. Remove the hardcoded path and construct it using __opts__['conf_file'].
The cloud roster expects an additional nest level when extracting an
instance's info from the result of the show_instance action. It looks
for (which is what show_instance of EC2 driver returned prior to
2016.3):
info[provider][driver][name][name]
whereas the correct location is
info[provider][driver][name]
Fix it.
The cloud roster places an instance's info under 'tgt' key in the dictionary it returns. Because of this salt-ssh reports host name 'tgt' instead of the specified in the command line. Fix it.
Pass the vm_ variable with provider and profile info to salt.utils.cloud.ssh_usernames in order to lookup ssh_username not only in the main salt-cloud config file but in provider and profile configuration as well. It is the same way all other settings are retrieved in the cloud roster.
If the show_instance action failed on a node, the node's name would be added to a list located at 'Not Actioned/Not Running' key of a dictionary returned by the action. Ensure that a target node isn't included into that list.
The cloud roster retrieves a minions's profile, provider and driver names from the cloud cache index and then issues the show_instance action to get the minions's IP address. If the minion wasn't found in the cloud cache index, the cloud roster stops. However, the cloud cache index is only maintained by the EC2 driver, making the cloud roster only usable with EC2. Don't stop if a minion wasn't found in the cloud cache index, trying show_instance anyway. In this way, although it's impossible to get the minion's profile, the cloud roster is applicable to cloud providers other than EC2.
|
@sergeizv, thanks for your PR! By analyzing the history of the files in this pull request, we identified @techhat, @flowhamster and @rallytime to be potential reviewers. |
|
Please check the cloud roster in develop. I have already made some of these changes to use the cache.cloud runner to do the matching instead of reading the files directly. |
|
@gtmanfred I plan another PR on develop, but need some time for it as there are more issues to be addressed (like maintaining the cache index by cloud providers). This PR however does the job on the release branches. |
|
@sergeizv 2016.3 and 2016.11 are bugfix-only branches. Does this address an open GitHub issue? |
|
@cachedout Each commit from this pull request is a bugfix, and together they aim to address #31005 on the release branches. I didn't get why #31005 was closed before this is merged, because the issues are still there. If the commit messages don't contain enough information on the problems they fix, I might provide more detailed descriptions of the issues in this PR or open another GitHub issue(s) for this (or #31005 might be reopened). What would you suggest in this case? |
|
Can you try the roster in develop? If that one works, i think the better option would be to backport the changes made there, instead of making extra changes here. It will also require the Thanks, |
|
@cachedout ok, I opened #40251 @gtmanfred Yes, I checked it. The cloud roster in develop is buggy as well together with the cloud cache runner and today I'm going to submit a PR on that with some bits of this PR forward-ported. This PR targets 2016.3 and 2016.11 release branches and is limited to bugfixes. Back-porting the roster from develop (but it firstly needs to be fixed there) from my opinion is not in spirit of maintaining stable branches, as the roster has been rewritten there and does things in different way. But that's up to maintainers :) |
|
@sergeizv Thanks for the detailed explanation! Thanks for helping us get up to speed and for filing the requested issue. I completely agree with your position. :] |
What does this PR do?
Makes the cloud roster alive on 2016.3 and 2016.11. Tested on EC2 and GCE.
What issues does this PR fix or reference?
#31005, #40251
Tests written?
No