applehv: return socket path from setupAPIForwarding #21264
Conversation
openshift-ci
bot
added
the
do-not-merge/release-note-label-needed
openshift-ci
bot
added
release-note-none
and removed
do-not-merge/release-note-label-needed
|
Needs a test or |
I don't know how to add a test for it, if you have pointers I can look into it. |
`applehv.Start()` has this line of code:
```
cmd.ExtraFiles = []*os.File{ioEater, ioEater, ioEater}
```
whose purpose is not clear.
The intent may have been to redirect stdin/stdout/stderr to /dev/null in
the child process.
This should be done by setting cmd.Stdin/cmd.Stdout/cmd/Stderr to nil,
which is the case by default.
The way it's done could also cause issues as
`Vfkit.VirtualMachine.Cmd()` sometimes adds files it needs to keep open
to `ExtraFiles`, so at the very least this should be an `append()`
This commit removes this code.
[NO NEW TESTS NEEDED]
Signed-off-by: Christophe Fergeau <[email protected]>
When starting podman machine with applehv, this warning is printed: WARN[0025] API socket failed ping test This is due to a bug in applehv.setupAPIForwarding which is not returning the path to the socket, which causes `WaitAndPingAPI` to be called with `""` as the socket path, triggering the warning. This commit changes setupAPIForwarding to be similar to the implementation in the other machine implementations. I don't know how to add a test for this, but this can be handled in podman-machine end to end tests by making sure that there are no warnings when running `podman machine start` with applehv. [NO NEW TESTS NEEDED] Signed-off-by: Christophe Fergeau <[email protected]>
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfergeau, 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 |
openshift-ci
bot
added
the
approved
openshift-merge-bot
bot
merged commit 597ff52
into
containers:main
|
/cherry-pick v4.9 |
|
@ashley-cui: new pull request created: #21437 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
github-actions
bot
added
the
locked - please file new issue/PR
When starting podman machine with applehv, this warning is printed:
WARN[0025] API socket failed ping test
This is due to a bug in applehv.setupAPIForwarding which is not returning
the path to the socket, which causes
WaitAndPingAPIto be called with""as the socket path, triggering the warning.This commit changes setupAPIForwarding to be similar to the implementation
in the other machine implementations.
Does this PR introduce a user-facing change?