-
Notifications
You must be signed in to change notification settings - Fork 123
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
Windows Container support #96
Changes from all commits
57ca05e
6bdd663
6e00312
b8be89e
c093c94
3dfef15
2709aae
eda7ab2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
minor_changes: | ||
- docker connection plugin - added support for Windows containers (https://github.com/ansible-collections/community.docker/pull/96). |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -81,7 +81,9 @@ def __init__(self, play_context, new_stdin, *args, **kwargs): | |
|
||
# Windows uses Powershell modules | ||
if getattr(self._shell, "_IS_WINDOWS", False): | ||
self.has_native_async = True | ||
self.module_implementation_preferences = ('.ps1', '.exe', '') | ||
self.allow_executable = False | ||
|
||
if 'docker_command' in kwargs: | ||
self.docker_cmd = kwargs['docker_command'] | ||
|
@@ -214,7 +216,11 @@ def exec_command(self, cmd, in_data=None, sudoable=False): | |
""" Run a command on the docker host """ | ||
super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable) | ||
|
||
local_cmd = self._build_exec_cmd([self._play_context.executable, '-c', cmd]) | ||
if getattr(self._shell, "_IS_WINDOWS", False): | ||
sudoable = False | ||
local_cmd = self._build_exec_cmd(["cmd", "/c", cmd]) | ||
else: | ||
local_cmd = self._build_exec_cmd([self._play_context.executable, '-c', cmd]) | ||
|
||
display.vvv(u"EXEC {0}".format(to_text(local_cmd)), host=self._play_context.remote_addr) | ||
display.debug("opening command with Popen()") | ||
|
@@ -268,6 +274,12 @@ def exec_command(self, cmd, in_data=None, sudoable=False): | |
display.debug("done communicating") | ||
|
||
display.debug("done with docker.exec_command()") | ||
|
||
# When running on Windows, stderr may contain CLIXML encoded output | ||
if getattr(self._shell, "_IS_WINDOWS", False) and stderr.startswith(b"#< CLIXML"): | ||
from ansible.plugins.shell.powershell import _parse_clixml | ||
tmeckel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
stderr = _parse_clixml(stderr) | ||
|
||
return (p.returncode, stdout, stderr) | ||
|
||
def _prefix_login_path(self, remote_path): | ||
|
@@ -288,6 +300,16 @@ def _prefix_login_path(self, remote_path): | |
remote_path = os.path.join(os.path.sep, remote_path) | ||
return os.path.normpath(remote_path) | ||
|
||
def _escape_win_path(self, path): | ||
""" converts a Windows path to one that's supported by SFTP and SCP """ | ||
# If using a root path then we need to start with / | ||
prefix = "" | ||
if re.match(r'^\w{1}:', path): | ||
prefix = "/" | ||
|
||
# Convert all '\' to '/' | ||
return "%s%s" % (prefix, path.replace("\\", "/")) | ||
|
||
def put_file(self, in_path, out_path): | ||
""" Transfer a file from local to docker container """ | ||
super(Connection, self).put_file(in_path, out_path) | ||
|
@@ -308,11 +330,32 @@ def put_file(self, in_path, out_path): | |
count = ' count=0' | ||
else: | ||
count = '' | ||
args = self._build_exec_cmd([self._play_context.executable, "-c", "dd of=%s bs=%s%s" % (out_path, BUFSIZE, count)]) | ||
if getattr(self._shell, "_IS_WINDOWS", False): | ||
cmd = '''& { $strm = [Console]::OpenStandardInput(1024) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what is happening here, the error you've shared is something that is thrown by the bootstrap_wrapper.ps1 which is used when executing a module. The code you have here is for copying a file to the docker container and shouldn't be calling the bootstrap_wrapper code. |
||
$data = [System.Array]::CreateInstance([byte],1024) | ||
$fstrm = [IO.File]::OpenWrite(%s) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You still need to have more logic to handle here. Right now at a quote glance I could have a path like:
Note the use of unicode smart quotes PowerShell (un)helpfully treats a bunch of smart quotes as the same as single quotes so even if you ran There are 2 choices I can see you moving to:
cmd = '''
...
$outPath = [Text.Encoding]::UTF8.GetString([Convert]::FromBase64String('%s'))
...
''' % (to_text(base64.b64encode(to_bytes(out_path, encoding='utf-8'))))
Both options remove the need for `shlex_quote` but I still recommend you normalize the path as you have done today. |
||
do { | ||
$read = $strm.Read($data, 0, $data.Length) | ||
if ($read -gt 0) { | ||
$fstrm.Write($data, 0, $read) | ||
} | ||
} while ($read -gt 0) | ||
$fstrm.Flush() | ||
$fstrm.Dispose() | ||
$strm.Dispose() }''' % (out_path) | ||
args = self._build_exec_cmd(["cmd", "/c", self._shell._encode_script(cmd, as_list=False, strict_mode=False, preserve_rc=False)]) | ||
tmeckel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
display.debug(u"Command to execute: {0}".format(args)) | ||
else: | ||
args = self._build_exec_cmd([self._play_context.executable, "-c", "dd of=%s bs=%s%s" % (out_path, BUFSIZE, count)]) | ||
|
||
args = [to_bytes(i, errors='surrogate_or_strict') for i in args] | ||
try: | ||
p = subprocess.Popen(args, stdin=in_file, | ||
stdout=subprocess.PIPE, stderr=subprocess.PIPE) | ||
p = subprocess.Popen( | ||
args, | ||
stdin=in_file, | ||
stdout=subprocess.PIPE, | ||
stderr=subprocess.PIPE | ||
) | ||
except OSError: | ||
raise AnsibleError("docker connection requires dd command in the container to put files") | ||
stdout, stderr = p.communicate() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be wrong but I believe using
cmd /c
here will mean anyraw
task with the connection plugin will be usingcmd
as the shell rather than PowerShell. You may want to change this based on whether theansible_shell_type
is eithercmd
orpowershell
to allow a user to change this as they desire.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jborean93 yes this is true. Bit when I change the code as follows I receive a
file not found
. So the interpreter (Powershell) cannot be found. So I suspect I didn't grasp what you meant by saying I should utilize theansible_shell_type
. Any guidance?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know what the default shell is for a Windows container, i.e. what runs when you do
docker exec ...
. Is it cmd or is it powershell.exe, or maybe no shell at all?Basically right now if someone was to do the following
Is that going to be running under cmd or powershell? With the current changes here that will be running as
cmd
which might not be ideal if someone has setansible_shell_type=powershell
. Maybe the desire is to haveraw
with this connection plugin independent to the shell plugin used, if that's the case then the PR as it is makes sense. If you wish for this to be dependent on the shell plugin then you will have to hardcodecmd.exe /c
when using the cmd shell andpowershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -Command
for the powershell shell.This might be something that @felixfontein may want to comment on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I can say something here. I've never used Docker with Windows containers, so I have zero experience what these contain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mostly mean more in general, do you expect that
raw
should match theansible_shell_type
(powershell or cmd) or should it just be the same thing on the connection plugin?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Matching
ansible_shell_type
would be nice I guess, though I'm not even sure whether Linux containers fully support that (I've never really looked at shell plugins). I also have no idea how complicated it would be to 'properly' implement this.