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

[v4.8] fix podman-remote exec regression with v4.8 #20865

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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