Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Windows Container support #96
Changes from 3 commits
57ca05e
6bdd663
6e00312
b8be89e
c093c94
3dfef15
2709aae
eda7ab2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.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.
If you use
_encode_script
as per below you can skip the& { ... }
entirely and just run the command as is.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 changed the code as advised, but I receive an
invalid payload
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'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.