From 069f7248cc14b9f8edfcb31c56e1481d1332170e Mon Sep 17 00:00:00 2001 From: Vivek Chandela <107602480+vchandela@users.noreply.github.com> Date: Thu, 19 Sep 2024 01:42:54 +0530 Subject: [PATCH] fix: handle 127 error code for podman compatibility (#2778) * fix: handle 127 error code for podman compatibility * fix: remove magic numbers; handle 126 and 127 error code separately * chore: handle comments for removing exitStatus type; handle returned error and switch case instead of if-else-if * feat: add unit test for exit code 127 * chore: add internalCheck() tests for exit code = 126/127 * chore: replace t.fatal() with require.NoError() * chore: merge internal check tests with existing tests; revert mockStrategyTarget to MockStrategyTarget to avoid export errors; run make lint --- wait/host_port.go | 33 ++++++++++++----- wait/host_port_test.go | 81 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 99 insertions(+), 15 deletions(-) diff --git a/wait/host_port.go b/wait/host_port.go index a3e9137006..9360517a04 100644 --- a/wait/host_port.go +++ b/wait/host_port.go @@ -13,13 +13,21 @@ import ( "github.com/docker/go-connections/nat" ) +const ( + exitEaccess = 126 // container cmd can't be invoked (permission denied) + exitCmdNotFound = 127 // container cmd not found/does not exist or invalid bind-mount +) + // Implement interface var ( _ Strategy = (*HostPortStrategy)(nil) _ StrategyTimeout = (*HostPortStrategy)(nil) ) -var errShellNotExecutable = errors.New("/bin/sh command not executable") +var ( + errShellNotExecutable = errors.New("/bin/sh command not executable") + errShellNotFound = errors.New("/bin/sh command not found") +) type HostPortStrategy struct { // Port is a string containing port number and protocol in the format "80/tcp" @@ -151,12 +159,16 @@ func (hp *HostPortStrategy) WaitUntilReady(ctx context.Context, target StrategyT } if err = internalCheck(ctx, internalPort, target); err != nil { - if errors.Is(errShellNotExecutable, err) { + switch { + case errors.Is(err, errShellNotExecutable): log.Println("Shell not executable in container, only external port validated") return nil + case errors.Is(err, errShellNotFound): + log.Println("Shell not found in container") + return nil + default: + return fmt.Errorf("internal check: %w", err) } - - return fmt.Errorf("internal check: %w", err) } return nil @@ -207,13 +219,18 @@ func internalCheck(ctx context.Context, internalPort nat.Port, target StrategyTa return fmt.Errorf("%w, host port waiting failed", err) } - if exitCode == 0 { - break - } else if exitCode == 126 { + // Docker has a issue which override exit code 127 to 126 due to: + // https://github.com/moby/moby/issues/45795 + // Handle both to ensure compatibility with Docker and Podman for now. + switch exitCode { + case 0: + return nil + case exitEaccess: return errShellNotExecutable + case exitCmdNotFound: + return errShellNotFound } } - return nil } func buildInternalCheckCommand(internalPort int) string { diff --git a/wait/host_port_test.go b/wait/host_port_test.go index bf349f05a7..18a15aed82 100644 --- a/wait/host_port_test.go +++ b/wait/host_port_test.go @@ -1,8 +1,10 @@ package wait import ( + "bytes" "context" "io" + "log" "net" "strconv" "testing" @@ -456,16 +458,12 @@ func TestHostPortStrategyFailsWhileInternalCheckingDueToUnexpectedContainerStatu func TestHostPortStrategySucceedsGivenShellIsNotInstalled(t *testing.T) { listener, err := net.Listen("tcp", "localhost:0") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) defer listener.Close() rawPort := listener.Addr().(*net.TCPAddr).Port port, err := nat.NewPort("tcp", strconv.Itoa(rawPort)) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) target := &MockStrategyTarget{ HostImpl: func(_ context.Context) (string, error) { @@ -497,7 +495,67 @@ func TestHostPortStrategySucceedsGivenShellIsNotInstalled(t *testing.T) { }, ExecImpl: func(_ context.Context, _ []string, _ ...exec.ProcessOption) (int, io.Reader, error) { // This is the error that would be returned if the shell is not installed. - return 126, nil, nil + return exitEaccess, nil, nil + }, + } + + wg := NewHostPortStrategy("80"). + WithStartupTimeout(5 * time.Second). + WithPollInterval(100 * time.Millisecond) + + oldWriter := log.Default().Writer() + var buf bytes.Buffer + log.Default().SetOutput(&buf) + t.Cleanup(func() { + log.Default().SetOutput(oldWriter) + }) + + err = wg.WaitUntilReady(context.Background(), target) + require.NoError(t, err) + + require.Contains(t, buf.String(), "Shell not executable in container, only external port validated") +} + +func TestHostPortStrategySucceedsGivenShellIsNotFound(t *testing.T) { + listener, err := net.Listen("tcp", "localhost:0") + require.NoError(t, err) + defer listener.Close() + + rawPort := listener.Addr().(*net.TCPAddr).Port + port, err := nat.NewPort("tcp", strconv.Itoa(rawPort)) + require.NoError(t, err) + + target := &MockStrategyTarget{ + HostImpl: func(_ context.Context) (string, error) { + return "localhost", nil + }, + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "80": []nat.PortBinding{ + { + HostIP: "0.0.0.0", + HostPort: port.Port(), + }, + }, + }, + }, + }, + }, nil + }, + MappedPortImpl: func(_ context.Context, _ nat.Port) (nat.Port, error) { + return port, nil + }, + StateImpl: func(_ context.Context) (*types.ContainerState, error) { + return &types.ContainerState{ + Running: true, + }, nil + }, + ExecImpl: func(_ context.Context, _ []string, _ ...exec.ProcessOption) (int, io.Reader, error) { + // This is the error that would be returned if the shell is not found. + return exitCmdNotFound, nil, nil }, } @@ -505,6 +563,15 @@ func TestHostPortStrategySucceedsGivenShellIsNotInstalled(t *testing.T) { WithStartupTimeout(5 * time.Second). WithPollInterval(100 * time.Millisecond) + oldWriter := log.Default().Writer() + var buf bytes.Buffer + log.Default().SetOutput(&buf) + t.Cleanup(func() { + log.Default().SetOutput(oldWriter) + }) + err = wg.WaitUntilReady(context.Background(), target) require.NoError(t, err) + + require.Contains(t, buf.String(), "Shell not found in container") }