Merge pull request #20831 from Luap99/remote-exec-rm

fix podman-remote exec regression with v4.8

Author: openshift-merge-bot[bot]

pkg/bindings/containers/exec.go

@@ -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 bindings.NewAPIVersionError("/exec/{id}/remove", v, "4.8.0")
+	}
 	if options == nil {
 		options = new(ExecRemoveOptions)
 	}
-	_ = options
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return err

pkg/bindings/errors.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 
+	"github.com/blang/semver/v4"
 	"github.com/containers/podman/v4/pkg/errorhandling"
 )
 
@@ -61,3 +62,26 @@ func CheckResponseCode(inError error) (int, error) {
 		return -1, errors.New("is not type ErrorModel")
 	}
 }
+
+type APIVersionError struct {
+	endpoint        string
+	serverVersion   *semver.Version
+	requiredVersion string
+}
+
+// NewAPIVersionError create bindings error when the endpoint on the server is not supported
+// because the version is to old.
+//   - endpoint is the name fo the endpoint (e.g. /containers/json)
+//   - version is the server API version
+//   - requiredVersion is the server version need to use said endpoint
+func NewAPIVersionError(endpoint string, version *semver.Version, requiredVersion string) *APIVersionError {
+	return &APIVersionError{
+		endpoint:        endpoint,
+		serverVersion:   version,
+		requiredVersion: requiredVersion,
+	}
+}
+
+func (e *APIVersionError) Error() string {
+	return fmt.Sprintf("API server version is %s, need at least %s to call %s", e.serverVersion.String(), e.requiredVersion, e.endpoint)
+}

pkg/domain/infra/tunnel/containers.go

@@ -17,6 +17,7 @@ import (
 	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/libpod/events"
 	"github.com/containers/podman/v4/pkg/api/handlers"
+	"github.com/containers/podman/v4/pkg/bindings"
 	"github.com/containers/podman/v4/pkg/bindings/containers"
 	"github.com/containers/podman/v4/pkg/bindings/images"
 	"github.com/containers/podman/v4/pkg/domain/entities"
@@ -588,6 +589,11 @@ func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrID string, o
 	}
 	defer func() {
 		if err := containers.ExecRemove(ic.ClientCtx, sessionID, nil); err != nil {
+			apiErr := new(bindings.APIVersionError)
+			if errors.As(err, &apiErr) {
+				// if the API is to old do not throw an error
+				return
+			}
 			if retErr == nil {
 				exitCode = -1
 				retErr = err

pkg/machine/e2e/basic_test.go

@@ -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())