LXC cloud fixes #23488
LXC cloud fixes #23488
Conversation
…stance without profile For this purpose `self.opts['providers']` is as good as `mapper.map_providers_parallel()`
At the end of call chain lxc.init returns boolean result, exactly what we need and not vice verse.
Otherwise user is not able to find if LXC container was created when using cloud.profile state. No changes were reported and output was always showing container as already existing and not created because of state applied.
Allows to set LXC template specific options in salt-cloud profile.
For example:
wheezy-profile:
provider: lxc-provider
image: debian
options:
release: wheezy
lxc.run_cmd was shadowing `stdout` and `stderr` arguments with local variables. Those arguments allow to change format of returned value and shadowing resulted in lxc.cmd_run returning value in wrong format when command didn't produce any output or errors.
For some reason loader overwrites __opts__['test'] with default value of False when performing action and after executing `show_instance` action cloud.create always got test=False. Obviously this resulted in unexpected creation of new instance when test=True was requested. I'm not sure it's a correct solution, more like a workaround.
lxc-attach doesn't understand commands with environment variables prepended (i.e. "FOO=bar baz") and tells command is not found. Use configured shell to set PATH environment variables and call bootstrap script. Backported from 2015.2.
This allows for LXC cloud implementation to properly return active instance(s) from `list_nodes`. `list_nodes` uses tricky filtering to avoid showing some running LXC containers when in creation mode. However sometimes it filters too much hence the need to pass container names via opts. Probably the better solution would be to change filtering algorithm in list_nodes but I'm not confident enough to do that. This change is safe for all the rest of cloud implementations because they don't ever use __opts__['names'].
This allows for LXC cloud implementation to properly return instance(s) from `list_nodes` to check if instance exists and can be destroyed. `list_nodes` uses tricky filtering to avoid showing some running LXC containers when in creation mode. However sometimes it filters too much hence the need to pass destroy=True via opts. Additionally LXC cloud implementation will refuse to destroy container if destroy=True is not present in opts. This change is safe for all the rest of cloud implementations because they don't use __opts__['destroy'].
When using cloud states to manage LXC containers __opts__['profile'] is not implicitly reset between operations on different containers. However list_nodes will hide container if profile is set in opts assuming that it have to be created. But in cloud state we do want to check at first if it really exists hence the need to remove profile from global opts once current container is created. Before this change salt would try to bootstrap other LXC containers configured with help of LXC cloud state after legitimately creating new container, even if those other containers were already up and running. Probably the better solution would be to change filtering algorithm in list_nodes but I'm not confident enough to do that.
|
@terminalmage do you see any issues in here? |
|
I've also applied some of those changes on top of 2015.5.0 and added another new fix for it. How to better proceed with that? May be just rebase this on 2015.5 since it seems I'm the only one who wants to use salt-cloud with LXC? :) Or should I create new pull request after (and if) this one is merged? |
|
@cellscape Thanks very much for contributing! As far as the 2015.5 branch goes, we merge the 2014.7 branch into 2015.5, and then 2015.5 into develop every day or two to help keep fixes consistent and applied to all applicable branches, so a new pull request for 2015.5 won't be needed. This fix, once merged, will propagate through to the other branches as we merge forward. You can read more information about that here. |
|
In most cases, what @rallytime said would be true; however, LXC support was rewritten in 2015.5 and so I will not be merging any of the salt/modules/lxc changes forward. I'm still evaluating the changes to salt/states/cloud.py in my merge forward. |
|
Final merge forward can be seen here: #23783 |
|
@basepi I still would like to see cellscape/salt@fb24f0c merged in. I would also like to still be able to pass LXC template options in What do you think? |
This pull request includes changes that I had to do in order to make salt-cloud with LXC and cloud state management work on my side.
Changes include mostly fixes for minor bugs like using incorrect variables and proper cloud state handling with changes reported back and test=True honoured.
I don't yet know salt internals good enough to make proper changes however I believe those can be a good start for review. I've tried hard to not break other clouds functionality but tested only with LXC. Please let me know if there is something wrong, I'll fix it.
I'm especially not confident of last 3 commits, they are workarounds and not proper fixes. Still work for me but I'll be happy to do it right way if someone shows the direction.
As for 2015.2 and develop branches, couple of changes were backported from 2015.2, few are not applicable anymore because of LXC revamp but some changes can still be applied.