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

Explicitly use IPv4 to check if podman-machine VM is listening #13617

Merged
merged 1 commit into from
Mar 23, 2022

Conversation

holzman
Copy link
Contributor

@holzman holzman commented Mar 23, 2022

When starting a VM that has been configured with volume mounts, the podman client attempts to connect via
TCP to localhost, which runs gvproxy to proxy an ephemeral port to the VM's ssh port. Previously, gvproxy
was listening on all interfaces and IP addresses, but this behavior has changed to listening only on the
IPv4 loopback address.

Without this change, if a newer build of gvproxy is used, a podman machine configured with volume mounts
will hang forever after "podman machine start" with "Waiting for VM ...".

Signed-off-by: Burt Holzman [email protected]

@mheon
Copy link
Member

mheon commented Mar 23, 2022

LGTM

@mheon
Copy link
Member

mheon commented Mar 23, 2022

/approve

@rhatdan
Copy link
Member

rhatdan commented Mar 23, 2022

/approve
/lgtm
/hold

@Luap99
Copy link
Member

Luap99 commented Mar 23, 2022

If gvproxy only listens on 127.0.0.1, why not just replace localhost with 127.0.0.1 here.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 23, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: holzman, mheon, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 23, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 23, 2022
@holzman
Copy link
Contributor Author

holzman commented Mar 23, 2022

If gvproxy only listens on 127.0.0.1, why not just replace localhost with 127.0.0.1 here.

Either works (and is equally readable as far as I'm concerned). I can change it if reviewers prefer.

@Luap99
Copy link
Member

Luap99 commented Mar 23, 2022

I prefer 127.0.0.1 because AFAIK you could in theory change localhost to another ip but I guess in this case many things would break.

@Luap99
Copy link
Member

Luap99 commented Mar 23, 2022

You also have to add [NO NEW TESTS NEEDED] to your commit message.

When starting a VM that has been configured with volume mounts, the
podman client attempts to connect via TCP to localhost, which runs
gvproxy to proxy an ephemeral port to the VM's ssh port.  Previously,
gvproxy was listening on all interfaces and IP addresses, but this
behavior has changed to listening only on the IPv4 loopback address.

Without this change, if a newer build of gvproxy is used, a podman
machine configured with volume mounts will hang forever after "podman
machine start" with "Waiting for VM ...".

[NO NEW TESTS NEEDED]

Signed-off-by: Burt Holzman <[email protected]>
@holzman holzman force-pushed the volume-mount-ipv4 branch from 90c685d to cdda192 Compare March 23, 2022 17:49
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 23, 2022
@rhatdan
Copy link
Member

rhatdan commented Mar 23, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 23, 2022
@rhatdan
Copy link
Member

rhatdan commented Mar 23, 2022

/lgtm

@rhatdan
Copy link
Member

rhatdan commented Mar 23, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2022
@openshift-merge-robot openshift-merge-robot merged commit 1092247 into containers:main Mar 23, 2022
@holzman holzman deleted the volume-mount-ipv4 branch March 23, 2022 19:50
@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 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. 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 this pull request may close these issues.

5 participants