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

WSL machine - consider adding pipe address to machine inspect output #16860

Closed
arixmkii opened this issue Dec 15, 2022 · 7 comments · Fixed by #17303
Closed

WSL machine - consider adding pipe address to machine inspect output #16860

arixmkii opened this issue Dec 15, 2022 · 7 comments · Fixed by #17303
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@arixmkii
Copy link
Contributor

arixmkii commented Dec 15, 2022

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind feature

Description

Currently pipe address of WSL machine is not published anywhere (it is built using the way one could read in code or documentation), but it would be better to allow this to be retrievable using some interface. There is ConnectionInfo with PodmanSocket path in it, but it is not used by WSL. Code here.

If it is used then naming will be a bit confusing (as one may argue that pipe in not a 100% socket), but it will allow some more flexibility. For example Podman Desktop instead of "guessing" here the pipe address will just read using podman cli (like it already does reading list of machines).

This will also simplify things for alternative implementations (like QEMU machine #13006) as they will not have to mimic this behavior or Podman Desktop would need to become aware that alternative implementations exist.

Steps to reproduce the issue:

  1. podman machine inspect --format '{{.ConnectionInfo.PodmanSocket.Path}}'

Describe the results you received:
Outputs null

Describe the results you expected:
Outputs pipe address

Additional information you deem important (e.g. issue happens only occasionally):

Output of podman version:

4.3.1 and 4.4.0-dev

Output of podman info:

Intentionally skipped

Package info (e.g. output of rpm -q podman or apt list podman or brew info podman):

Intentionally skipped

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide?

Yes (including built from sources)

Additional environment details (AWS, VirtualBox, physical, etc.):

WSL machine

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 15, 2022
@arixmkii
Copy link
Contributor Author

The most simple implementation - just adding it to output reusing all the available methods in the same file is pretty straightforward.

The one which involves moving it to machine config structure is a more significant refactoring.

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

@arixmkii
Copy link
Contributor Author

Still valid. There should be a way to communicate API listening interface address w/o guessing on the client side.

@arixmkii
Copy link
Contributor Author

Did some experiments and due to lack of AF_UNIX support on Windows in nodejs nodejs/node#35008 it is better to extend the structure to expose 2 different endpoints - one for Unix socket and one for named pipe. And then machine inspect could populate them accordingly. This is indeed an overhead for all non-Windows versions as the data structure will always be empty there, but it is really small. Alternative implementation would be to refactor this to expose it as a list of endpoints, where path, type and possible other attributes would appear, which would potentially scale better, but doesn't look like a mandatory addition at this point.

Expected result would look like:

podman machine inspect --format {{.ConnectionInfo.PodmanSocket.Path}} my-machine
C:\Temp\my-machine.sock

podman machine inspect --format {{.ConnectionInfo.PodmanNamedPipe.Path}} my-machine
//./pipe/my-machine

@rhatdan
Copy link
Member

rhatdan commented Jan 23, 2023

Care to open a PR to make this happen?

@arixmkii
Copy link
Contributor Author

@rhatdan I will work on this. And after this one is finalized (reviewed and accepted) will submit a fix to Podman Desktop as well.

@rhatdan
Copy link
Member

rhatdan commented Jan 23, 2023

Great thanks.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 2, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants