Skip to content

Commit

Permalink
fix: handle 127 error code for podman compatibility (#2778)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
vchandela authored Sep 18, 2024
1 parent 31a033c commit 069f724
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 15 deletions.
33 changes: 25 additions & 8 deletions wait/host_port.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
81 changes: 74 additions & 7 deletions wait/host_port_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package wait

import (
"bytes"
"context"
"io"
"log"
"net"
"strconv"
"testing"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -497,14 +495,83 @@ 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
},
}

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 found in container")
}

0 comments on commit 069f724

Please sign in to comment.