Skip to content

Commit

Permalink
Merge pull request #20865 from openshift-cherrypick-robot/cherry-pick…
Browse files Browse the repository at this point in the history
…-20831-to-v4.8

[v4.8] fix podman-remote exec regression with v4.8
  • Loading branch information
openshift-merge-bot[bot] authored Dec 1, 2023
2 parents 1c434e3 + a8b8dc5 commit 4635f40
Show file tree
Hide file tree
Showing 4 changed files with 43 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 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
Expand Down
24 changes: 24 additions & 0 deletions pkg/bindings/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"

"github.com/blang/semver/v4"
"github.com/containers/podman/v4/pkg/errorhandling"
)

Expand Down Expand Up @@ -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)
}
6 changes: 6 additions & 0 deletions pkg/domain/infra/tunnel/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
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 4635f40

Please sign in to comment.