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

[Windows] Fix tracing listener #3544

Merged
merged 3 commits into from
Mar 8, 2023

Conversation

gabriel-samfira
Copy link
Collaborator

  • The tracing socket is a named pipe on Windows
  • Sets proper permissions on the named pipe
  • Sets proper environment variables inside the executor
  • Defines proper mount for the tracing socket

Signed-off-by: Gabriel Adrian Samfira gsamfira@cloudbasesolutions.com

Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
executor/oci/spec.go Outdated Show resolved Hide resolved
executor/oci/spec_unix.go Outdated Show resolved Hide resolved
@gabriel-samfira
Copy link
Collaborator Author

@tonistiigi I updated the PR. PTAL when you have some time.

Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
@crazy-max
Copy link
Member

crazy-max commented Feb 9, 2023

Ah was going to say the unix:// protocol was missing redundant, good catch.

@gabriel-samfira
Copy link
Collaborator Author

Ah was going to say the unix:// protocol was missing, good catch.

Accidentally omitted the npipe:// variant on Windows when I moved code around. My eye twitched as I looked at that line, and didn't know why. 😄

@crazy-max
Copy link
Member

Ah was going to say the unix:// protocol was missing, good catch.

Accidentally omitted the npipe:// variant on Windows when I moved code around. My eye twitched as I looked at that line, and didn't know why. 😄

Yeah this npipe:////./pipe/foo is confusing though.

@gabriel-samfira
Copy link
Collaborator Author

Ah was going to say the unix:// protocol was missing, good catch.

Accidentally omitted the npipe:// variant on Windows when I moved code around. My eye twitched as I looked at that line, and didn't know why. smile

Yeah this npipe:////./pipe/foo is confusing though.

You should see what it looks like when \ are used and go escapes them when they get printed 😄. But as funky as that looks, it is correct. Named pipes on Windows start with \\hostname\path\to\pipe. Where, if we're referencing the local host, a dot can be used instead of the hostname. Usernames are the same. You can use hostname\Administrator or .\Administrator. If you're loggin in as an AD user, that hostname can be replaced with the forest name.

Windows is fun sometimes 😄.

Co-authored-by: Akihiro Suda <suda.kyoto@gmail.com>
Signed-off-by: Gabriel <samfiragabriel@gmail.com>
@tonistiigi tonistiigi merged commit 9ea86dd into moby:master Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants