Skip to content

Commit

Permalink
fix: remove magic numbers; handle 126 and 127 error code separately
Browse files Browse the repository at this point in the history
  • Loading branch information
vchandela committed Sep 12, 2024
1 parent 18cc4bc commit 9a6847f
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
16 changes: 15 additions & 1 deletion 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"
)

type exitStatus = int

const (
exitEaccess exitStatus = 126 // container cmd can't be invoked (permission denied)
exitCmdNotFound exitStatus = 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 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 @@ -207,10 +215,16 @@ func internalCheck(ctx context.Context, internalPort nat.Port, target StrategyTa
return fmt.Errorf("%w, host port waiting failed", err)
}

/*
Docker maps exit code 127 to 126, while Podman treats them separately.
Handle both to ensure compatibility with Docker and Podman.
*/
if exitCode == 0 {
break
} else if exitCode == 126 || exitCode == 127 {
} else if exitCode == exitEaccess {
return errShellNotExecutable
} else if exitCode == exitCmdNotFound {
return errShellNotFound
}
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion wait/host_port_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ 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
},
}

Expand Down

0 comments on commit 9a6847f

Please sign in to comment.