[ansible] Fix docker shell plugin #1253
Conversation
|
@lguohan @qiluo-msft Please review. This is the fix for #1243 |
msosyak
force-pushed
the
fix_docker_shell_plugin
branch
from
1ab7a04
to
102d385
Compare
msosyak
force-pushed
the
fix_docker_shell_plugin
branch
from
102d385
to
d4968fb
Compare
ansible/shell_plugins/docker.py
Outdated
| def build_module_command(self, env_string, shebang, cmd, arg_path=None, rm_tmp=None): | ||
| def remove(self, path, recurse=False): | ||
| if not self.container: | ||
| self.container = self.get_option('ansible_python_interpreter').split()[3] |
There was a problem hiding this comment.
self.get_option('ansible_python_interpreter').split()[3] [](start = 29, length = 56)
This logic is not robust.
If this case not self.container happens in your test? If not, we can just fallback to base class function. #Closed
There was a problem hiding this comment.
If I am right this module was made just for one thing, to execute tasks inside the container on the host. And it retrieves the container name from shebang in build_module_command which is not anything else as ansible_python_interpreter. And for this plugin, it should look like docker exec -i <container> python what is not obvious. So I just put get_option('ansible_python_interpreter') under if to minimize get_option calls.
For any case changed this to the same approach as in build_module_command.
For me, it looks like is better to define mandatory ansible_container option for this plugin, change remove and build_module_command to use it as a container name source and replace
vars:
ansible_shell_type: docker
ansible_python_interpreter: docker exec -i swss python
to
vars:
ansible_shell_type: docker
ansible_container: swss
I could do this in separate PR if you accept.
Or maybe it is the time to look to the docker connection plugin.
Description of PR
Update docker shell plugin to be compatible with ansible 2.8.7
Summary:
Fixes #1243
Type of change
Approach
How did you do it?
Add a description of the plugin. https://docs.ansible.com/ansible/latest/porting_guides/porting_guide_2.4.html#configuration
Fixed base method call
How did you verify/test it?
Run few testcases
Any platform specific information?
N/A
Supported testbed topology if it's a new test case?
N/A
Documentation