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.

Sign up for GitHub

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

bug: fix leaking logs #2912

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

bug: fix leaking logs #2912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 28 additions & 3 deletions wait/host_port.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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...)
}
Loading