mirror of
https://github.com/89luca89/distrobox.git
synced 2026-08-19 01:14:49 -05:00
fix(enter): propagate errors when entering containers (#2070)
* fix(providers): propagate container start errors before enter * fix(providers): return errors from enter command execution * fix(providers): improve setup and inspect error reporting * test(providers): cover enter error propagation paths
This commit is contained in:
committed by
Alessio Biancalana
parent
b581ace4c7
commit
1981ca41e7
@@ -534,7 +534,9 @@ func (d *Docker) Enter(
|
||||
if err != nil || inspectResult.ContainerStatus != RunningStatus {
|
||||
logTimestamp := timestampNow()
|
||||
|
||||
_ = d.startContainer(ctx, options.ContainerName, progress)
|
||||
if err := d.startContainer(ctx, options.ContainerName, progress); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Monitor logs for setup completion
|
||||
if err := d.waitForSetup(ctx, options.ContainerName, logTimestamp, progress, printer); err != nil {
|
||||
@@ -544,7 +546,9 @@ func (d *Docker) Enter(
|
||||
progress.Finalize("Container Setup Complete!")
|
||||
}
|
||||
|
||||
_, _ = d.run(ctx, append(command, commandArgs...), runOptions{Interactive: !options.NoTTY})
|
||||
if _, err := d.run(ctx, append(command, commandArgs...), runOptions{Interactive: !options.NoTTY}); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
@@ -669,7 +673,7 @@ func (d *Docker) InspectContainer(ctx context.Context, containerName string) (*c
|
||||
|
||||
var inspects []inspectOutput
|
||||
if err := json.Unmarshal([]byte(output), &inspects); err != nil {
|
||||
return nil, errors.New("error marshaling json into containerInspect")
|
||||
return nil, fmt.Errorf("error unmarshaling json into containerInspect: %w", err)
|
||||
}
|
||||
|
||||
if len(inspects) == 0 {
|
||||
@@ -1026,10 +1030,17 @@ func (d *Docker) waitForSetup(
|
||||
for {
|
||||
// Check container is still running
|
||||
inspectResult, err := d.InspectContainer(ctx, containerName)
|
||||
if err != nil || inspectResult.ContainerStatus != RunningStatus {
|
||||
if err != nil {
|
||||
printer.PrintError("\nContainer Setup Failure!")
|
||||
return fmt.Errorf("container stopped during setup: %w", err)
|
||||
}
|
||||
if inspectResult.ContainerStatus != RunningStatus {
|
||||
printer.PrintError("\nContainer Setup Failure!")
|
||||
return fmt.Errorf(
|
||||
"container stopped during setup: status=%s",
|
||||
inspectResult.ContainerStatus,
|
||||
)
|
||||
}
|
||||
|
||||
// Get logs
|
||||
nextSince := timestampNow()
|
||||
|
||||
@@ -1,12 +1,18 @@
|
||||
package providers
|
||||
|
||||
import (
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/89luca89/distrobox/internal/userenv"
|
||||
"github.com/89luca89/distrobox/pkg/containermanager"
|
||||
"github.com/89luca89/distrobox/pkg/ui"
|
||||
)
|
||||
|
||||
func TestDocker_makeCreateCommand(t *testing.T) {
|
||||
@@ -483,3 +489,98 @@ func hasSubstring(s, substr string) bool {
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func TestDockerEnterPropagatesStartError(t *testing.T) {
|
||||
installFakeDockerRuntime(t)
|
||||
t.Setenv("FAKE_INSPECT_STDOUT", dockerFakeInspectJSON("exited"))
|
||||
t.Setenv("FAKE_START_EXIT", "9")
|
||||
t.Setenv("FAKE_START_STDERR", "start failed")
|
||||
|
||||
err := NewDocker(false, "sudo", false).Enter(
|
||||
t.Context(),
|
||||
containermanager.EnterOptions{
|
||||
ContainerName: "box",
|
||||
NoTTY: true,
|
||||
NoWorkDir: true,
|
||||
},
|
||||
ui.NewDevNullProgress(),
|
||||
ui.NewPrinter(io.Discard, false),
|
||||
)
|
||||
|
||||
require.Error(t, err)
|
||||
assert.ErrorContains(t, err, "failed to start container")
|
||||
}
|
||||
|
||||
func TestDockerEnterPropagatesExecError(t *testing.T) {
|
||||
installFakeDockerRuntime(t)
|
||||
t.Setenv("FAKE_INSPECT_STDOUT", dockerFakeInspectJSON("running"))
|
||||
t.Setenv("FAKE_EXEC_EXIT", "7")
|
||||
t.Setenv("FAKE_EXEC_STDERR", "exec failed")
|
||||
|
||||
err := NewDocker(false, "sudo", false).Enter(
|
||||
t.Context(),
|
||||
containermanager.EnterOptions{
|
||||
ContainerName: "box",
|
||||
NoTTY: true,
|
||||
NoWorkDir: true,
|
||||
},
|
||||
ui.NewDevNullProgress(),
|
||||
ui.NewPrinter(io.Discard, false),
|
||||
)
|
||||
|
||||
require.Error(t, err)
|
||||
assert.ErrorContains(t, err, "exec failed")
|
||||
}
|
||||
|
||||
func installFakeDockerRuntime(t *testing.T) {
|
||||
t.Helper()
|
||||
|
||||
tmpDir := t.TempDir()
|
||||
runtimePath := filepath.Join(tmpDir, "docker")
|
||||
|
||||
const script = `#!/bin/sh
|
||||
cmd="$1"
|
||||
shift
|
||||
case "$cmd" in
|
||||
inspect)
|
||||
if [ -n "$FAKE_INSPECT_STDOUT" ]; then
|
||||
printf "%s" "$FAKE_INSPECT_STDOUT"
|
||||
fi
|
||||
exit "${FAKE_INSPECT_EXIT:-0}"
|
||||
;;
|
||||
start)
|
||||
if [ -n "$FAKE_START_STDERR" ]; then
|
||||
printf "%s" "$FAKE_START_STDERR" >&2
|
||||
fi
|
||||
exit "${FAKE_START_EXIT:-0}"
|
||||
;;
|
||||
logs)
|
||||
if [ -n "$FAKE_LOGS_STDOUT" ]; then
|
||||
printf "%s" "$FAKE_LOGS_STDOUT"
|
||||
fi
|
||||
exit "${FAKE_LOGS_EXIT:-0}"
|
||||
;;
|
||||
exec)
|
||||
if [ -n "$FAKE_EXEC_STDERR" ]; then
|
||||
printf "%s" "$FAKE_EXEC_STDERR" >&2
|
||||
fi
|
||||
exit "${FAKE_EXEC_EXIT:-0}"
|
||||
;;
|
||||
*)
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
`
|
||||
|
||||
err := os.WriteFile(runtimePath, []byte(script), 0o755)
|
||||
require.NoError(t, err)
|
||||
|
||||
t.Setenv("PATH", tmpDir+":"+os.Getenv("PATH"))
|
||||
t.Setenv("USER", "testuser")
|
||||
t.Setenv("HOME", "/home/testuser")
|
||||
t.Setenv("SHELL", "/bin/sh")
|
||||
}
|
||||
|
||||
func dockerFakeInspectJSON(status string) string {
|
||||
return `[{"Id":"container-id","State":{"Status":"` + status + `"},"Config":{"Labels":{"distrobox.unshare_groups":"0"},"Env":["HOME=/home/testuser","PATH=/usr/bin:/bin"]}}]`
|
||||
}
|
||||
|
||||
@@ -529,7 +529,9 @@ func (p *Podman) Enter(
|
||||
if err != nil || inspectResult.ContainerStatus != RunningStatus {
|
||||
logTimestamp := timestampNow()
|
||||
|
||||
_ = p.startContainer(ctx, options.ContainerName, progress)
|
||||
if err := p.startContainer(ctx, options.ContainerName, progress); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Monitor logs for setup completion
|
||||
if err := p.waitForSetup(ctx, options.ContainerName, logTimestamp, progress, printer); err != nil {
|
||||
@@ -539,7 +541,9 @@ func (p *Podman) Enter(
|
||||
progress.Finalize("Container Setup Complete!")
|
||||
}
|
||||
|
||||
_, _ = p.run(ctx, append(command, commandArgs...), runOptions{Interactive: !options.NoTTY})
|
||||
if _, err := p.run(ctx, append(command, commandArgs...), runOptions{Interactive: !options.NoTTY}); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
@@ -749,7 +753,7 @@ func (p *Podman) InspectContainer(ctx context.Context, containerName string) (*c
|
||||
|
||||
var inspects []inspectOutput
|
||||
if err := json.Unmarshal([]byte(output), &inspects); err != nil {
|
||||
return nil, errors.New("error marshaling json into containerInspect")
|
||||
return nil, fmt.Errorf("error unmarshaling json into containerInspect: %w", err)
|
||||
}
|
||||
|
||||
if len(inspects) == 0 {
|
||||
@@ -909,10 +913,17 @@ func (p *Podman) waitForSetup(
|
||||
for {
|
||||
// Check container is still running
|
||||
inspectResult, err := p.InspectContainer(ctx, containerName)
|
||||
if err != nil || inspectResult.ContainerStatus != RunningStatus {
|
||||
if err != nil {
|
||||
printer.PrintError("\nContainer Setup Failure!")
|
||||
return fmt.Errorf("container stopped during setup: %w", err)
|
||||
}
|
||||
if inspectResult.ContainerStatus != RunningStatus {
|
||||
printer.PrintError("\nContainer Setup Failure!")
|
||||
return fmt.Errorf(
|
||||
"container stopped during setup: status=%s",
|
||||
inspectResult.ContainerStatus,
|
||||
)
|
||||
}
|
||||
|
||||
// Get logs
|
||||
nextSince := timestampNow()
|
||||
|
||||
@@ -2,7 +2,9 @@ package providers
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -10,6 +12,8 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/89luca89/distrobox/internal/userenv"
|
||||
"github.com/89luca89/distrobox/pkg/containermanager"
|
||||
"github.com/89luca89/distrobox/pkg/ui"
|
||||
)
|
||||
|
||||
func TestPodman_makeCreateCommand(t *testing.T) {
|
||||
@@ -672,3 +676,98 @@ func TestCommandExists(t *testing.T) {
|
||||
// Test with a command that should not exist
|
||||
assert.False(t, commandExists("this-command-definitely-does-not-exist-12345"), "Did not expect non-existent command to be found")
|
||||
}
|
||||
|
||||
func TestPodmanEnterPropagatesStartError(t *testing.T) {
|
||||
installFakePodmanRuntime(t)
|
||||
t.Setenv("FAKE_INSPECT_STDOUT", podmanFakeInspectJSON("exited"))
|
||||
t.Setenv("FAKE_START_EXIT", "9")
|
||||
t.Setenv("FAKE_START_STDERR", "start failed")
|
||||
|
||||
err := NewPodman(false, "sudo", false).Enter(
|
||||
t.Context(),
|
||||
containermanager.EnterOptions{
|
||||
ContainerName: "box",
|
||||
NoTTY: true,
|
||||
NoWorkDir: true,
|
||||
},
|
||||
ui.NewDevNullProgress(),
|
||||
ui.NewPrinter(io.Discard, false),
|
||||
)
|
||||
|
||||
require.Error(t, err)
|
||||
assert.ErrorContains(t, err, "failed to start container")
|
||||
}
|
||||
|
||||
func TestPodmanEnterPropagatesExecError(t *testing.T) {
|
||||
installFakePodmanRuntime(t)
|
||||
t.Setenv("FAKE_INSPECT_STDOUT", podmanFakeInspectJSON("running"))
|
||||
t.Setenv("FAKE_EXEC_EXIT", "7")
|
||||
t.Setenv("FAKE_EXEC_STDERR", "exec failed")
|
||||
|
||||
err := NewPodman(false, "sudo", false).Enter(
|
||||
t.Context(),
|
||||
containermanager.EnterOptions{
|
||||
ContainerName: "box",
|
||||
NoTTY: true,
|
||||
NoWorkDir: true,
|
||||
},
|
||||
ui.NewDevNullProgress(),
|
||||
ui.NewPrinter(io.Discard, false),
|
||||
)
|
||||
|
||||
require.Error(t, err)
|
||||
assert.ErrorContains(t, err, "exec failed")
|
||||
}
|
||||
|
||||
func installFakePodmanRuntime(t *testing.T) {
|
||||
t.Helper()
|
||||
|
||||
tmpDir := t.TempDir()
|
||||
runtimePath := filepath.Join(tmpDir, "podman")
|
||||
|
||||
const script = `#!/bin/sh
|
||||
cmd="$1"
|
||||
shift
|
||||
case "$cmd" in
|
||||
inspect)
|
||||
if [ -n "$FAKE_INSPECT_STDOUT" ]; then
|
||||
printf "%s" "$FAKE_INSPECT_STDOUT"
|
||||
fi
|
||||
exit "${FAKE_INSPECT_EXIT:-0}"
|
||||
;;
|
||||
start)
|
||||
if [ -n "$FAKE_START_STDERR" ]; then
|
||||
printf "%s" "$FAKE_START_STDERR" >&2
|
||||
fi
|
||||
exit "${FAKE_START_EXIT:-0}"
|
||||
;;
|
||||
logs)
|
||||
if [ -n "$FAKE_LOGS_STDOUT" ]; then
|
||||
printf "%s" "$FAKE_LOGS_STDOUT"
|
||||
fi
|
||||
exit "${FAKE_LOGS_EXIT:-0}"
|
||||
;;
|
||||
exec)
|
||||
if [ -n "$FAKE_EXEC_STDERR" ]; then
|
||||
printf "%s" "$FAKE_EXEC_STDERR" >&2
|
||||
fi
|
||||
exit "${FAKE_EXEC_EXIT:-0}"
|
||||
;;
|
||||
*)
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
`
|
||||
|
||||
err := os.WriteFile(runtimePath, []byte(script), 0o755)
|
||||
require.NoError(t, err)
|
||||
|
||||
t.Setenv("PATH", tmpDir+":"+os.Getenv("PATH"))
|
||||
t.Setenv("USER", "testuser")
|
||||
t.Setenv("HOME", "/home/testuser")
|
||||
t.Setenv("SHELL", "/bin/sh")
|
||||
}
|
||||
|
||||
func podmanFakeInspectJSON(status string) string {
|
||||
return `[{"Id":"container-id","State":{"Status":"` + status + `"},"Config":{"Labels":{"distrobox.unshare_groups":"0"},"Env":["HOME=/home/testuser","PATH=/usr/bin:/bin"]}}]`
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user