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.

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

chore: improve error wrapping in some docker container methods and compose module #2720

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,12 +508,12 @@ func (c *DockerContainer) Exec(ctx context.Context, cmd []string, options ...tce

response, err := cli.ContainerExecCreate(ctx, c.ID, processOptions.ExecConfig)
if err != nil {
return 0, nil, err
return 0, nil, fmt.Errorf("container exec create: %w", err)
}

hijack, err := cli.ContainerExecAttach(ctx, response.ID, container.ExecAttachOptions{})
if err != nil {
return 0, nil, err
return 0, nil, fmt.Errorf("container exec attach: %w", err)
}

processOptions.Reader = hijack.Reader
Expand All @@ -528,7 +528,7 @@ func (c *DockerContainer) Exec(ctx context.Context, cmd []string, options ...tce
for {
execResp, err := cli.ContainerExecInspect(ctx, response.ID)
if err != nil {
return 0, nil, err
return 0, nil, fmt.Errorf("container exec inspect: %w", err)
}

if !execResp.Running {
Expand Down Expand Up @@ -1129,7 +1129,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque

resp, err := p.client.ContainerCreate(ctx, dockerInput, hostConfig, networkingConfig, platform, req.Name)
if err != nil {
return nil, err
return nil, fmt.Errorf("container create: %w", err)
}

// #248: If there is more than one network specified in the request attach newly created container to them one by one
Expand All @@ -1144,7 +1144,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque
}
err = p.client.NetworkConnect(ctx, nw.ID, resp.ID, &endpointSetting)
if err != nil {
return nil, err
return nil, fmt.Errorf("network connect: %w", err)
}
}
}
Expand Down Expand Up @@ -1245,7 +1245,7 @@ func (p *DockerProvider) ReuseOrCreateContainer(ctx context.Context, req Contain
if !p.config.RyukDisabled {
r, err := reuseOrCreateReaper(context.WithValue(ctx, core.DockerHostContextKey, p.host), sessionID, p)
if err != nil {
return nil, fmt.Errorf("%w: creating reaper failed", err)
return nil, fmt.Errorf("reaper: %w", err)
}
termSignal, err = r.Connect()
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func GenericContainer(ctx context.Context, req GenericContainerRequest) (Contain
}
provider, err := req.ProviderType.GetProvider(WithLogger(logging))
if err != nil {
return nil, err
return nil, fmt.Errorf("get provider: %w", err)
}
defer provider.Close()

Expand All @@ -79,7 +79,7 @@ func GenericContainer(ctx context.Context, req GenericContainerRequest) (Contain

if req.Started && !c.IsRunning() {
if err := c.Start(ctx); err != nil {
return c, fmt.Errorf("failed to start container: %w", err)
return c, fmt.Errorf("start container: %w", err)
}
}
return c, nil
Expand Down
6 changes: 3 additions & 3 deletions modules/compose/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func NewDockerComposeWith(opts ...ComposeStackOption) (*dockerCompose, error) {

for i := range opts {
if err := opts[i].applyToComposeStack(&composeOptions); err != nil {
return nil, err
return nil, fmt.Errorf("apply compose stack option: %w", err)
}
}

Expand All @@ -139,11 +139,11 @@ func NewDockerComposeWith(opts ...ComposeStackOption) (*dockerCompose, error) {

dockerCli, err := command.NewDockerCli()
if err != nil {
return nil, err
return nil, fmt.Errorf("new docker client: %w", err)
}

if err = dockerCli.Initialize(flags.NewClientOptions(), command.WithInitializeClient(makeClient)); err != nil {
return nil, err
return nil, fmt.Errorf("initialize docker client: %w", err)
}

reaperProvider, err := testcontainers.NewDockerProvider()
Expand Down
20 changes: 10 additions & 10 deletions modules/compose/compose_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,19 +120,19 @@ func (r ComposeStackReaders) applyToComposeStack(o *composeStackOptions) error {
tmp = filepath.Join(tmp, strconv.FormatInt(time.Now().UnixNano(), 10))
err := os.MkdirAll(tmp, 0o755)
if err != nil {
return fmt.Errorf("failed to create temporary directory: %w", err)
return fmt.Errorf("create temporary directory: %w", err)
}

name := fmt.Sprintf(baseName, i)

bs, err := io.ReadAll(reader)
if err != nil {
return fmt.Errorf("failed to read from reader: %w", err)
return fmt.Errorf("read from reader: %w", err)
}

err = os.WriteFile(filepath.Join(tmp, name), bs, 0o644)
if err != nil {
return fmt.Errorf("failed to write to temporary file: %w", err)
return fmt.Errorf("write to temporary file: %w", err)
}

f[i] = filepath.Join(tmp, name)
Expand Down Expand Up @@ -311,7 +311,7 @@ func (d *dockerCompose) Up(ctx context.Context, opts ...StackUpOption) error {
},
})
if err != nil {
return err
return fmt.Errorf("compose up: %w", err)
}

err = d.lookupNetworks(ctx)
Expand Down Expand Up @@ -442,7 +442,7 @@ func (d *dockerCompose) lookupContainer(ctx context.Context, svcName string) (*t
),
})
if err != nil {
return nil, err
return nil, fmt.Errorf("container list: %w", err)
}

if len(containers) == 0 {
Expand All @@ -458,7 +458,7 @@ func (d *dockerCompose) lookupContainer(ctx context.Context, svcName string) (*t

dockerProvider, err := testcontainers.NewDockerProvider(testcontainers.WithLogger(d.logger))
if err != nil {
return nil, err
return nil, fmt.Errorf("new docker provider: %w", err)
}

dockerProvider.SetClient(d.dockerClient)
Expand All @@ -479,7 +479,7 @@ func (d *dockerCompose) lookupNetworks(ctx context.Context) error {
),
})
if err != nil {
return err
return fmt.Errorf("network list: %w", err)
}

for _, n := range networks {
Expand All @@ -504,12 +504,12 @@ func (d *dockerCompose) compileProject(ctx context.Context) (*types.Project, err

compiledOptions, err := cli.NewProjectOptions(d.configs, projectOptions...)
if err != nil {
return nil, err
return nil, fmt.Errorf("new project options: %w", err)
}

proj, err := compiledOptions.LoadProject(ctx)
if err != nil {
return nil, err
return nil, fmt.Errorf("load project: %w", err)
}

for i, s := range proj.Services {
Expand Down Expand Up @@ -568,7 +568,7 @@ func withEnv(env map[string]string) func(*cli.ProjectOptions) error {
func makeClient(*command.DockerCli) (client.APIClient, error) {
dockerClient, err := testcontainers.NewDockerClientWithOpts(context.Background())
if err != nil {
return nil, err
return nil, fmt.Errorf("new docker client: %w", err)
}
return dockerClient, nil
}
25 changes: 13 additions & 12 deletions modules/compose/compose_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (dc *LocalDockerCompose) containerNameFromServiceName(service, separator st
func (dc *LocalDockerCompose) applyStrategyToRunningContainer() error {
cli, err := testcontainers.NewDockerClientWithOpts(context.Background())
if err != nil {
return err
return fmt.Errorf("new docker client: %w", err)
}
defer cli.Close()

Expand All @@ -150,22 +150,22 @@ func (dc *LocalDockerCompose) applyStrategyToRunningContainer() error {
containerListOptions := container.ListOptions{Filters: f, All: true}
containers, err := cli.ContainerList(context.Background(), containerListOptions)
if err != nil {
return fmt.Errorf("error %w occurred while filtering the service %s: %d by name and published port", err, k.service, k.publishedPort)
return fmt.Errorf("container list service %q: %w", k.service, err)
}

if len(containers) == 0 {
return fmt.Errorf("service with name %s not found in list of running containers", k.service)
return fmt.Errorf("service with name %q not found in list of running containers", k.service)
}

// The length should always be a list of 1, since we are matching one service name at a time
if l := len(containers); l > 1 {
return fmt.Errorf("expecting only one running container for %s but got %d", k.service, l)
return fmt.Errorf("expecting only one running container for %q but got %d", k.service, l)
}
container := containers[0]
strategy := dc.WaitStrategyMap[k]
dockerProvider, err := testcontainers.NewDockerProvider(testcontainers.WithLogger(dc.Logger))
if err != nil {
return fmt.Errorf("unable to create new Docker Provider: %w", err)
return fmt.Errorf("new docker provider: %w", err)
}
defer dockerProvider.Close()

Expand All @@ -175,7 +175,7 @@ func (dc *LocalDockerCompose) applyStrategyToRunningContainer() error {

err = strategy.WaitUntilReady(context.Background(), dockercontainer)
if err != nil {
return fmt.Errorf("unable to apply wait strategy %v to service %s due to %w", strategy, k.service, err)
return fmt.Errorf("wait until ready %v to service %q due: %w", strategy, k.service, err)
}
}
return nil
Expand Down Expand Up @@ -223,7 +223,6 @@ func (dc *LocalDockerCompose) WithExposedService(service string, port int, strat
// depending on the version services names are composed in a different way
func (dc *LocalDockerCompose) determineVersion() error {
execErr := executeCompose(dc, []string{"version", "--short"})

if err := execErr.Error; err != nil {
return err
}
Expand All @@ -235,7 +234,7 @@ func (dc *LocalDockerCompose) determineVersion() error {

majorVersion, err := strconv.ParseInt(string(components[0]), 10, 8)
if err != nil {
return err
return fmt.Errorf("parsing major version: %w", err)
}

switch majorVersion {
Expand Down Expand Up @@ -263,11 +262,11 @@ func (dc *LocalDockerCompose) validate() error {

yamlFile, err := os.ReadFile(abs)
if err != nil {
return err
return fmt.Errorf("read compose file %q: %w", abs, err)
}
err = yaml.Unmarshal(yamlFile, &c)
if err != nil {
return err
return fmt.Errorf("unmarshalling file %q: %w", abs, err)
}

if dc.Services == nil {
Expand Down Expand Up @@ -448,7 +447,9 @@ func (w *capturingPassThroughWriter) Bytes() []byte {

// Which checks if a binary is present in PATH
func which(binary string) error {
_, err := exec.LookPath(binary)
if _, err := exec.LookPath(binary); err != nil {
return fmt.Errorf("lookup: %w", err)
}

return err
return nil
}