Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix docker shell 2 #1262

Closed
wants to merge 2 commits into from
Closed

Conversation

msosyak
Copy link
Contributor

@msosyak msosyak commented Dec 6, 2019

Description of PR

In this PR #1253 I have tested docker plugin by running dip_sib and lag2 (lldp task run in lldp container) test cases and everything was fine.
But after regression, I have noticed that some tests failed due to this plugin with error:
KeyError: 'Requested entry (plugin_type: shell plugin: docker setting: ansible_python_interpreter ) was not defined in configuration.' on copy task.

Here is the fix for the error described above.

Summary:
Fixed docker shell plugin part 2
Fixes # (issue)

Type of change

  • Bug fix
  • [] Testbed and Framework(new/improvement)
  • [] Test case(new/improvement)

Approach

How did you do it?

Defined mandatory container_name 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
    container_name: swss

How did you verify/test it?

run ecn_wred, coop and dip_sib test_cases

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Myron Sosyak added 2 commits December 5, 2019 08:27
Replace ansible_python_interpreter: docker exec -i <container name>
with container_name: <container name>
@msosyak
Copy link
Contributor Author

msosyak commented Dec 6, 2019

@qiluo-msft could you have a look at this?

@qiluo-msft
Copy link
Contributor

I suspect you are not fixing the root issue. I did a while and found one missing in docker.py, you may need to override ShellBase.append_command().

Please help open an issue, and provide the verbose error message in order to better communicate.

@msosyak
Copy link
Contributor Author

msosyak commented Dec 9, 2019

Related issue #1268
@qiluo-msft I am not sure that append_command() will really help us as it just called to concatenate two commands. I go deep and have made this PR #1269 where I used native docker connection plugin.

@msosyak
Copy link
Contributor Author

msosyak commented Dec 10, 2019

Closed In favor of #1269

@msosyak msosyak closed this Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants