Skip to content

Commit

Permalink
[bug] fix leaking logs
Browse files Browse the repository at this point in the history
  • Loading branch information
KenxinKun committed Dec 6, 2024
1 parent f884d5b commit fb9eeda
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 3 deletions.
31 changes: 28 additions & 3 deletions wait/host_port.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,23 @@ var (
errShellNotFound = errors.New("/bin/sh command not found")
)

type Logging interface {
Printf(format string, v ...interface{})
Println(v ...interface{})
}

var _ Logging = defaultLogger{}

type defaultLogger struct{}

func (defaultLogger) Printf(format string, v ...interface{}) {
log.Printf(format, v...)
}

func (defaultLogger) Println(v ...interface{}) {
log.Println(v...)
}

type HostPortStrategy struct {
// Port is a string containing port number and protocol in the format "80/tcp"
// which
Expand All @@ -41,6 +58,7 @@ type HostPortStrategy struct {
// a shell is not available in the container or when the container doesn't bind
// the port internally until additional conditions are met.
skipInternalCheck bool
logger Logging
}

// NewHostPortStrategy constructs a default host port strategy that waits for the given
Expand All @@ -49,6 +67,7 @@ func NewHostPortStrategy(port nat.Port) *HostPortStrategy {
return &HostPortStrategy{
Port: port,
PollInterval: defaultPollInterval(),
logger: defaultLogger{},
}
}

Expand Down Expand Up @@ -78,6 +97,12 @@ func (hp *HostPortStrategy) SkipInternalCheck() *HostPortStrategy {
return hp
}

func (hp *HostPortStrategy) WithLogger(logger Logging) *HostPortStrategy {
hp.logger = logger

return hp
}

// WithStartupTimeout can be used to change the default startup timeout
func (hp *HostPortStrategy) WithStartupTimeout(startupTimeout time.Duration) *HostPortStrategy {
hp.timeout = &startupTimeout
Expand Down Expand Up @@ -145,7 +170,7 @@ func (hp *HostPortStrategy) WaitUntilReady(ctx context.Context, target StrategyT
}
port, err = target.MappedPort(ctx, internalPort)
if err != nil {
log.Printf("mapped port: retries: %d, port: %q, err: %s\n", i, port, err)
hp.logger.Printf("mapped port: retries: %d, port: %q, err: %s\n", i, port, err)
}
}
}
Expand All @@ -161,10 +186,10 @@ func (hp *HostPortStrategy) WaitUntilReady(ctx context.Context, target StrategyT
if err = internalCheck(ctx, internalPort, target); err != nil {
switch {
case errors.Is(err, errShellNotExecutable):
log.Println("Shell not executable in container, only external port validated")
hp.logger.Println("Shell not executable in container, only external port validated")
return nil
case errors.Is(err, errShellNotFound):
log.Println("Shell not found in container")
hp.logger.Println("Shell not found in container")
return nil
default:
return fmt.Errorf("internal check: %w", err)
Expand Down
160 changes: 160 additions & 0 deletions wait/host_port_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,54 @@ func TestWaitForExposedPortSucceeds(t *testing.T) {
require.NoError(t, err)
}

func TestWaitForListeningPortSucceedsWithRetriesAndCustomLogger(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)

var mappedPortCount, execCount int
target := &MockStrategyTarget{
HostImpl: func(_ context.Context) (string, error) {
return "localhost", nil
},
MappedPortImpl: func(_ context.Context, _ nat.Port) (nat.Port, error) {
defer func() { mappedPortCount++ }()
if mappedPortCount < 2 {
return port, ErrPortNotFound
}
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) {
defer func() { execCount++ }()
if execCount == 0 {
return 1, nil, nil
}
return 0, nil, nil
},
}

wg := ForListeningPort("80").
WithStartupTimeout(5 * time.Second).
WithPollInterval(100 * time.Millisecond).WithLogger(MockLogger{
PrintfImpl: func(format string, v ...interface{}) {
require.Equal(t, "mapped port: retries: %d, port: %q, err: %s\n", format)
require.Equal(t, []interface{}{1, port, ErrPortNotFound}, v)
},
})

err = wg.WaitUntilReady(context.Background(), target)
require.NoError(t, err)
}

func TestHostPortStrategyFailsWhileGettingPortDueToOOMKilledContainer(t *testing.T) {
var mappedPortCount int
target := &MockStrategyTarget{
Expand Down Expand Up @@ -467,6 +515,62 @@ func TestHostPortStrategySucceedsGivenShellIsNotInstalled(t *testing.T) {
require.Contains(t, buf.String(), "Shell not executable in container, only external port validated")
}

func TestHostPortStrategySucceedsGivenShellIsNotInstalledWithCustomLogger(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 installed.
return exitEaccess, nil, nil
},
}

wg := NewHostPortStrategy("80").
WithStartupTimeout(5 * time.Second).
WithPollInterval(100 * time.Millisecond).
WithLogger(MockLogger{
PrintlnImpl: func(v ...interface{}) {
require.Equal(t, []interface{}{"Shell not executable in container, only external port validated"}, v)
},
})

err = wg.WaitUntilReady(context.Background(), target)
require.NoError(t, err)
}

func TestHostPortStrategySucceedsGivenShellIsNotFound(t *testing.T) {
listener, err := net.Listen("tcp", "localhost:0")
require.NoError(t, err)
Expand Down Expand Up @@ -526,3 +630,59 @@ func TestHostPortStrategySucceedsGivenShellIsNotFound(t *testing.T) {

require.Contains(t, buf.String(), "Shell not found in container")
}

func TestHostPortStrategySucceedsGivenShellIsNotFoundWithCustomLogger(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).
WithLogger(MockLogger{
PrintlnImpl: func(v ...interface{}) {
require.Equal(t, []interface{}{"Shell not found in container"}, v)
},
})

err = wg.WaitUntilReady(context.Background(), target)
require.NoError(t, err)
}
14 changes: 14 additions & 0 deletions wait/wait_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,17 @@ func (st MockStrategyTarget) State(ctx context.Context) (*types.ContainerState,
func (st MockStrategyTarget) CopyFileFromContainer(ctx context.Context, filePath string) (io.ReadCloser, error) {
return st.CopyFileFromContainerImpl(ctx, filePath)
}

type MockLogger struct {
PrintfImpl func(format string, v ...interface{})
PrintlnImpl func(v ...interface{})
}

var _ Logging = MockLogger{}

func (l MockLogger) Printf(format string, v ...interface{}) {
l.PrintfImpl(format, v...)
}
func (l MockLogger) Println(v ...interface{}) {
l.PrintlnImpl(v...)
}

0 comments on commit fb9eeda

Please sign in to comment.