Skip to content

Commit

Permalink
fix podman-remote exec regression with v4.8
Browse files Browse the repository at this point in the history
Commit f48a706 added a new API endpoint to remove exec session
correctly. And the bindings try to call that endpoint for exec every
time. Now since client and server must not be the same version this
causes a problem if a new 4.8 client calls an older 4.7 server as it has
no idea about such endpoint and throws an ugly error. This is a common
scenario for podman machine setups.

The client does know the server version so it should make sure to not
call such endpoint if the server is older than 4.8.

I added a exec test to the machine tests as this can be reproduced with
podman machine as at the moment at least the VM image does not contain
podman 4.8. And it should at least make sure podman exec keeps working
for podman machine without regressions.

Fixes containers#20821

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
Luap99 authored and openshift-cherrypick-robot committed Dec 1, 2023
1 parent 1c434e3 commit 86bb910
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
7 changes: 6 additions & 1 deletion pkg/bindings/containers/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,15 @@ func ExecStart(ctx context.Context, sessionID string, options *ExecStartOptions)

// ExecRemove removes a given exec session.
func ExecRemove(ctx context.Context, sessionID string, options *ExecRemoveOptions) error {
v := bindings.ServiceVersion(ctx)
// The exec remove endpoint was added in 4.8.
if v.Major < 4 || (v.Major == 4 && v.Minor < 8) {
// Do no call this endpoint as it will not be supported on the server and throw an "NOT FOUND" error.
return nil
}
if options == nil {
options = new(ExecRemoveOptions)
}
_ = options
conn, err := bindings.GetClient(ctx)
if err != nil {
return err
Expand Down
8 changes: 7 additions & 1 deletion pkg/machine/e2e/basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,18 @@ var _ = Describe("run basic podman commands", func() {
Expect(err).ToNot(HaveOccurred())
Expect(session).To(Exit(0))

ctrName := "test"
bm := basicMachine{}
runAlp, err := mb.setCmd(bm.withPodmanCommand([]string{"run", "-dt", "-p", "62544:80", "quay.io/libpod/alpine_nginx"})).run()
runAlp, err := mb.setCmd(bm.withPodmanCommand([]string{"run", "-dt", "--name", ctrName, "-p", "62544:80", "quay.io/libpod/alpine_nginx"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(runAlp).To(Exit(0))
testHTTPServer("62544", false, "podman rulez")

// Test exec in machine scenario: https://github.com/containers/podman/issues/20821
exec, err := mb.setCmd(bm.withPodmanCommand([]string{"exec", ctrName, "true"})).run()
Expect(err).ToNot(HaveOccurred())
Expect(exec).To(Exit(0))

out, err := pgrep("gvproxy")
Expect(err).ToNot(HaveOccurred())
Expect(out).ToNot(BeEmpty())
Expand Down

0 comments on commit 86bb910

Please sign in to comment.