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

Fixed #2086 by adding support for Windows Named Pipes in volumes #2185

Merged
merged 3 commits into from
Aug 28, 2019

Conversation

ericdalling
Copy link
Contributor

Summary
This pull request fixes issue #2086, allowing Windows containers to mount Named Pipes (i.e. Mounting the .\pipe\docker_engine).

Implementation details
There is a regular expression to check if the mount path is using named pipes, is so, it bypasses the call to the filepath.Clean function that breaks the named pipes path.

Testing
I ran the unit and integration tests on a Windows machine
New tests cover the changes: yes

Description for the changelog
Enables support for mounting named pipes to Windows docker containers in ECS

Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ericdalling
Copy link
Contributor Author

ericdalling commented Aug 27, 2019

@yumex93 and @fenxiong this is the new PR that I opened. Please review and let me know your feedback. Thanks!

@fenxiong
Copy link
Contributor

@ericdalling make static-check is failing. can you run make gofmt to fix formatting? thanks.

@ericdalling
Copy link
Contributor Author

@ericdalling make static-check is failing. can you run make gofmt to fix formatting? thanks.

I ran the make gofmt and fixed the formatting issues. Thanks!

@yhlee-aws yhlee-aws merged commit ade810a into aws:dev Aug 28, 2019
@yhlee-aws
Copy link
Contributor

@ericdalling Thank you for the contribution!

@fenxiong fenxiong added this to the 1.31.0 milestone Sep 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.

4 participants