mirror of
https://github.com/89luca89/distrobox.git
synced 2026-08-17 16:34:42 -05:00
feat(migrate): detect outdated containers via distrobox.version label
This commit is contained in:
@@ -75,6 +75,9 @@ func (s *spyContainerManager) InspectContainer(_ context.Context, _ string) (*co
|
||||
return &containermanager.InspectResult{}, nil
|
||||
}
|
||||
func (s *spyContainerManager) Commit(_ context.Context, _, _ string) error { return nil }
|
||||
func (s *spyContainerManager) NeedsMigration(_ context.Context, _ string) (bool, error) {
|
||||
return false, nil
|
||||
}
|
||||
func (s *spyContainerManager) ImageExists(_ context.Context, _ string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -20,6 +20,11 @@ type migrateSpyContainerManager struct {
|
||||
commits []string
|
||||
|
||||
inspectResult *containermanager.InspectResult
|
||||
|
||||
// needsMigrationResult, when non-nil, overrides the default return
|
||||
// of NeedsMigration. The default is true (migrate), matching a v1
|
||||
// container with no version label.
|
||||
needsMigrationResult *bool
|
||||
}
|
||||
|
||||
func (s *migrateSpyContainerManager) Name() string { return "spy" }
|
||||
@@ -57,6 +62,12 @@ func (s *migrateSpyContainerManager) ImageExists(_ context.Context, _ string) bo
|
||||
func (s *migrateSpyContainerManager) PullImage(_ context.Context, _ string, _ string, _ bool) error {
|
||||
return nil
|
||||
}
|
||||
func (s *migrateSpyContainerManager) NeedsMigration(_ context.Context, _ string) (bool, error) {
|
||||
if s.needsMigrationResult != nil {
|
||||
return *s.needsMigrationResult, nil
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// runMigrate runs the migrate subcommand with the given argv (starting from
|
||||
// "migrate") against a spy container manager. It returns the spy so the
|
||||
@@ -178,6 +189,7 @@ func TestMigrateAction_V2Container_Skipped(t *testing.T) {
|
||||
v2ScriptDir := t.TempDir()
|
||||
t.Setenv("DBX_SCRIPTS_DIR", v2ScriptDir)
|
||||
|
||||
notNeeded := false
|
||||
spy := &migrateSpyContainerManager{
|
||||
inspectResult: &containermanager.InspectResult{
|
||||
ContainerID: "abc123",
|
||||
@@ -187,10 +199,11 @@ func TestMigrateAction_V2Container_Skipped(t *testing.T) {
|
||||
IpcMode: "host",
|
||||
PidMode: "host",
|
||||
Env: []string{"HOME=/home/testuser"},
|
||||
Mounts: []containermanager.MountInfo{
|
||||
{Source: v2ScriptDir + "/distrobox-init", Destination: "/usr/bin/entrypoint"},
|
||||
Labels: map[string]string{
|
||||
containermanager.VersionLabelKey: "2",
|
||||
},
|
||||
},
|
||||
needsMigrationResult: ¬Needed,
|
||||
}
|
||||
|
||||
runMigrate(t, spy, "migrate", "my-box")
|
||||
|
||||
+16
-25
@@ -5,7 +5,6 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -86,6 +85,15 @@ func (c *MigrateCommand) Execute(ctx context.Context, opts MigrateOptions) error
|
||||
return ErrMigrateNoContainerSpecified
|
||||
}
|
||||
|
||||
// Provision v2 scripts once, up front: they don't depend on which
|
||||
// container is being migrated and the same files are reused for all.
|
||||
// Skipped in dry-run to avoid host-side file writes.
|
||||
if !opts.DryRun {
|
||||
if _, err := insidedistrobox.ProvisionScripts(insidedistrobox.ScriptsDir()); err != nil {
|
||||
return fmt.Errorf("failed to provision v2 scripts: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
var lastErr error
|
||||
for _, name := range containerNames {
|
||||
if err := c.migrateContainer(ctx, name, opts); err != nil {
|
||||
@@ -109,9 +117,14 @@ func (c *MigrateCommand) migrateContainer(ctx context.Context, name string, opts
|
||||
return fmt.Errorf("failed to inspect container %s: %w", name, err)
|
||||
}
|
||||
|
||||
// Check if migration is needed
|
||||
// Check if migration is needed. The container manager owns the
|
||||
// version-label rule, so we just ask it.
|
||||
if !opts.Force {
|
||||
if c.isAlreadyMigrated(inspectResult) {
|
||||
needs, err := c.containerManager.NeedsMigration(ctx, name)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to determine if %s needs migration: %w", name, err)
|
||||
}
|
||||
if !needs {
|
||||
return ErrMigrateAlreadyMigrated
|
||||
}
|
||||
}
|
||||
@@ -175,28 +188,6 @@ func (c *MigrateCommand) migrateContainer(ctx context.Context, name string, opts
|
||||
return nil
|
||||
}
|
||||
|
||||
// isAlreadyMigrated checks whether the container's entrypoint mount source
|
||||
// already points to the v2 scripts directory.
|
||||
func (c *MigrateCommand) isAlreadyMigrated(inspect *containermanager.InspectResult) bool {
|
||||
v2Dir := insidedistrobox.ScriptsDir()
|
||||
for _, mount := range inspect.Mounts {
|
||||
if mount.Destination == "/usr/bin/entrypoint" {
|
||||
// The source should be <v2Dir>/distrobox-init
|
||||
expected := filepath.Join(v2Dir, "distrobox-init")
|
||||
if mount.Source == expected {
|
||||
return true
|
||||
}
|
||||
// Also accept if the source is within the v2 directory (e.g., symlink resolution)
|
||||
if strings.HasPrefix(mount.Source, v2Dir) {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
}
|
||||
// If we can't find the entrypoint mount at all, conservatively say not migrated
|
||||
return false
|
||||
}
|
||||
|
||||
// recoverCreateOptions reconstructs the CreateOptions from the container's
|
||||
// inspect data, so the recreated container matches the original as closely as
|
||||
// possible.
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"context"
|
||||
"io"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -127,20 +128,19 @@ func TestMigrate_V1Container_TriggersStopCommitRemoveCreate(t *testing.T) {
|
||||
func TestMigrate_V2Container_Skipped(t *testing.T) {
|
||||
migrateCmd, mock := newMigrateTestSetup(t)
|
||||
|
||||
// Simulate a v2 container: entrypoint mount source points to v2 dir
|
||||
v2Dir := insidedistrobox.ScriptsDir()
|
||||
v2EntrypointPath := filepath.Join(v2Dir, "distrobox-init")
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Simulate a v2 container: tagged with the current schema version.
|
||||
mock.InspectContainerResult = &containermanager.InspectResult{
|
||||
ContainerID: "abc123",
|
||||
ContainerStatus: "exited",
|
||||
ContainerImage: "alpine:latest",
|
||||
Mounts: []containermanager.MountInfo{
|
||||
{Source: v2EntrypointPath, Destination: "/usr/bin/entrypoint"},
|
||||
Labels: map[string]string{
|
||||
containermanager.VersionLabelKey: strconv.Itoa(containermanager.SchemaVersion),
|
||||
},
|
||||
}
|
||||
needs := false
|
||||
mock.NeedsMigrationResult = &needs
|
||||
|
||||
opts := commands.MigrateOptions{
|
||||
ContainerNames: []string{"my-box"},
|
||||
@@ -228,6 +228,69 @@ func TestMigrate_V2Container_ForceRecreates(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestMigrate_UnparseableLabel_TreatedAsV1 covers the case where a
|
||||
// container has a distrobox.version label that isn't a valid integer.
|
||||
// The shared rule in NeedsMigrationFromLabels treats this as version 0,
|
||||
// so the container is migrated.
|
||||
func TestMigrate_UnparseableLabel_TreatedAsV1(t *testing.T) {
|
||||
migrateCmd, mock := newMigrateTestSetup(t)
|
||||
|
||||
ctx := context.Background()
|
||||
mock.InspectContainerResult = &containermanager.InspectResult{
|
||||
ContainerID: "abc123",
|
||||
ContainerStatus: "exited",
|
||||
ContainerImage: "alpine:latest",
|
||||
Labels: map[string]string{
|
||||
containermanager.VersionLabelKey: "not-a-number",
|
||||
},
|
||||
}
|
||||
|
||||
opts := commands.MigrateOptions{
|
||||
ContainerNames: []string{"my-box"},
|
||||
NonInteractive: true,
|
||||
}
|
||||
|
||||
if err := migrateCmd.Execute(ctx, opts); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
if len(mock.Spy.Commit) != 1 || len(mock.Spy.Remove) != 1 || len(mock.Spy.Create) != 1 {
|
||||
t.Errorf("expected unparseable-label container to be migrated, got stop=%d commit=%d remove=%d create=%d",
|
||||
len(mock.Spy.Stop), len(mock.Spy.Commit), len(mock.Spy.Remove), len(mock.Spy.Create))
|
||||
}
|
||||
}
|
||||
|
||||
// TestMigrate_DowngradedLabel_StillMigrates covers a container tagged
|
||||
// with a strictly older schema version (e.g. label=1 against the
|
||||
// current SchemaVersion=2). It must be migrated.
|
||||
func TestMigrate_DowngradedLabel_StillMigrates(t *testing.T) {
|
||||
migrateCmd, mock := newMigrateTestSetup(t)
|
||||
|
||||
ctx := context.Background()
|
||||
mock.InspectContainerResult = &containermanager.InspectResult{
|
||||
ContainerID: "abc123",
|
||||
ContainerStatus: "exited",
|
||||
ContainerImage: "alpine:latest",
|
||||
Labels: map[string]string{
|
||||
containermanager.VersionLabelKey: "1",
|
||||
},
|
||||
}
|
||||
|
||||
opts := commands.MigrateOptions{
|
||||
ContainerNames: []string{"my-box"},
|
||||
NonInteractive: true,
|
||||
}
|
||||
|
||||
if err := migrateCmd.Execute(ctx, opts); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
if len(mock.Spy.Commit) != 1 || len(mock.Spy.Remove) != 1 || len(mock.Spy.Create) != 1 {
|
||||
t.Errorf("expected downgraded-label container to be migrated, got stop=%d commit=%d remove=%d create=%d",
|
||||
len(mock.Spy.Stop), len(mock.Spy.Commit), len(mock.Spy.Remove), len(mock.Spy.Create))
|
||||
}
|
||||
}
|
||||
|
||||
func TestMigrate_DryRun_NoSideEffects(t *testing.T) {
|
||||
migrateCmd, mock := newMigrateTestSetup(t)
|
||||
|
||||
|
||||
@@ -26,6 +26,7 @@ import (
|
||||
"regexp"
|
||||
"runtime"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -63,6 +64,15 @@ func ReadOnlyBindPropagation() string {
|
||||
|
||||
const (
|
||||
RunningStatus = "running"
|
||||
|
||||
// SchemaVersion is the distrobox container schema version that the
|
||||
// currently running binary is compatible with. Containers tagged with
|
||||
// a strictly older version (or no version at all) need to be migrated.
|
||||
SchemaVersion = 2
|
||||
|
||||
// VersionLabelKey is the container label used to record the distrobox
|
||||
// schema version a container was created with.
|
||||
VersionLabelKey = "distrobox.version"
|
||||
)
|
||||
|
||||
type Container struct {
|
||||
@@ -96,6 +106,8 @@ type InspectResult struct {
|
||||
// Env is the full list of environment variables set on the container
|
||||
// (e.g., "HOME=/home/user", "HOSTNAME=...").
|
||||
Env []string
|
||||
// Labels is the full set of container labels, as key/value pairs.
|
||||
Labels map[string]string
|
||||
}
|
||||
|
||||
// MountInfo represents a single bind mount of a container.
|
||||
@@ -134,6 +146,11 @@ type CreateOptions struct {
|
||||
// by config.Values.ScriptsDir and passed through so the provider can
|
||||
// mount them into the container.
|
||||
ScriptsDir string
|
||||
// Labels are extra container labels to set at creation time. The
|
||||
// provider may add its own labels (e.g. distrobox.version); entries
|
||||
// here are merged in and the provider's defaults win on conflict for
|
||||
// keys the provider manages.
|
||||
Labels map[string]string
|
||||
}
|
||||
|
||||
type EnterOptions struct {
|
||||
@@ -182,6 +199,24 @@ func (c Container) IsRunning() bool {
|
||||
return strings.Contains(s, "up") || strings.Contains(s, "running")
|
||||
}
|
||||
|
||||
// NeedsMigrationFromLabels is the shared rule for whether a container's
|
||||
// labels indicate it predates the current distrobox schema. A missing or
|
||||
// unparseable distrobox.version label is treated as version 0, which means
|
||||
// the container always needs migration. This is the one-time cost of
|
||||
// rolling out the version-label scheme: pre-existing v2 containers get
|
||||
// recreated once to be tagged, then permanently skipped.
|
||||
func NeedsMigrationFromLabels(labels map[string]string) bool {
|
||||
raw, ok := labels[VersionLabelKey]
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
ver, err := strconv.Atoi(raw)
|
||||
if err != nil {
|
||||
return true
|
||||
}
|
||||
return ver < SchemaVersion
|
||||
}
|
||||
|
||||
//nolint:revive // ContainerManagerType is intentionally named for clarity despite the stutter
|
||||
type ContainerManagerType string
|
||||
|
||||
@@ -200,6 +235,11 @@ type ContainerManager interface {
|
||||
InspectContainer(ctx context.Context, containerName string) (*InspectResult, error)
|
||||
PullImage(ctx context.Context, imageName string, platform string, dryRun bool) error
|
||||
Commit(ctx context.Context, containerID string, imageTag string) error
|
||||
// NeedsMigration reports whether the named container was created with a
|
||||
// distrobox schema version older than the one this binary supports. A
|
||||
// container with no distrobox.version label is treated as schema
|
||||
// version 0, so it always needs migration.
|
||||
NeedsMigration(ctx context.Context, containerName string) (bool, error)
|
||||
}
|
||||
|
||||
func PathExists(path string) bool {
|
||||
|
||||
@@ -242,6 +242,11 @@ func (d *Docker) makeCreateCommand(
|
||||
"--label",
|
||||
fmt.Sprintf("distrobox.unshare_groups=%d", containermanager.Btoi(unshareGroups)),
|
||||
)
|
||||
options = append(
|
||||
options,
|
||||
"--label",
|
||||
fmt.Sprintf("%s=%d", containermanager.VersionLabelKey, containermanager.SchemaVersion),
|
||||
)
|
||||
options = append(options, "--env", fmt.Sprintf("SHELL=%s", shellFilepath))
|
||||
options = append(options, "--env", fmt.Sprintf("HOME=%s", containerUserHome))
|
||||
options = append(options, "--env", fmt.Sprintf("container=%s", containerManager))
|
||||
@@ -642,6 +647,18 @@ func (d *Docker) Commit(ctx context.Context, containerID string, tag string) err
|
||||
return err
|
||||
}
|
||||
|
||||
// NeedsMigration reports whether the named container was created with a
|
||||
// distrobox schema version older than the one this binary supports. A
|
||||
// container with no distrobox.version label (or with an unparseable one)
|
||||
// is treated as schema version 0, so it always needs migration.
|
||||
func (d *Docker) NeedsMigration(ctx context.Context, containerName string) (bool, error) {
|
||||
inspect, err := d.InspectContainer(ctx, containerName)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to inspect container %s: %w", containerName, err)
|
||||
}
|
||||
return containermanager.NeedsMigrationFromLabels(inspect.Labels), nil
|
||||
}
|
||||
|
||||
func parseContainerList(output string) ([]containermanager.Container, error) {
|
||||
var containers []containermanager.Container
|
||||
|
||||
@@ -715,14 +732,8 @@ func (d *Docker) InspectContainer(ctx context.Context, containerName string) (*c
|
||||
config.PidMode = inspect.HostConfig.PidMode
|
||||
config.Env = inspect.Config.Env
|
||||
|
||||
// Docker exposes the entrypoint command (distrobox-init args) as Config.Cmd,
|
||||
// not top-level Args (which is the parsed entrypoint path array). Prefer
|
||||
// top-level Args if present (podman compatibility), else use Config.Cmd.
|
||||
if len(inspect.Args) > 0 {
|
||||
config.Cmd = inspect.Args
|
||||
} else {
|
||||
config.Cmd = inspect.Config.Cmd
|
||||
}
|
||||
// Docker exposes the entrypoint command (distrobox-init args) as Config.Cmd.
|
||||
config.Cmd = inspect.Config.Cmd
|
||||
|
||||
// Populate mount info
|
||||
config.Mounts = make([]containermanager.MountInfo, 0, len(inspect.Mounts))
|
||||
@@ -739,6 +750,10 @@ func (d *Docker) InspectContainer(ctx context.Context, containerName string) (*c
|
||||
config.UnshareGroups = true
|
||||
}
|
||||
|
||||
// Expose the full label set so callers (e.g. migrate) can read
|
||||
// distrobox.version and other distrobox-managed labels.
|
||||
config.Labels = inspect.Config.Labels
|
||||
|
||||
// Extract HOME and PATH from container env
|
||||
for _, env := range inspect.Config.Env {
|
||||
if strings.HasPrefix(env, "HOME=") {
|
||||
|
||||
@@ -105,6 +105,7 @@ func TestDocker_makeCreateCommand(t *testing.T) {
|
||||
--pid host
|
||||
--label manager=distrobox
|
||||
--label distrobox.unshare_groups=0
|
||||
--label distrobox.version=2
|
||||
--env SHELL=sh
|
||||
--env HOME=/home/user
|
||||
--env container=docker
|
||||
|
||||
@@ -233,6 +233,11 @@ func (p *Podman) makeCreateCommand(
|
||||
"--label",
|
||||
fmt.Sprintf("distrobox.unshare_groups=%d", containermanager.Btoi(unshareGroups)),
|
||||
)
|
||||
options = append(
|
||||
options,
|
||||
"--label",
|
||||
fmt.Sprintf("%s=%d", containermanager.VersionLabelKey, containermanager.SchemaVersion),
|
||||
)
|
||||
options = append(options, "--env", fmt.Sprintf("SHELL=%s", shellFilepath))
|
||||
options = append(options, "--env", fmt.Sprintf("HOME=%s", containerUserHome))
|
||||
options = append(options, "--env", "container=podman")
|
||||
@@ -788,6 +793,18 @@ func (p *Podman) Commit(ctx context.Context, containerID string, tag string) err
|
||||
return err
|
||||
}
|
||||
|
||||
// NeedsMigration reports whether the named container was created with a
|
||||
// distrobox schema version older than the one this binary supports. A
|
||||
// container with no distrobox.version label (or with an unparseable one)
|
||||
// is treated as schema version 0, so it always needs migration.
|
||||
func (p *Podman) NeedsMigration(ctx context.Context, containerName string) (bool, error) {
|
||||
inspect, err := p.InspectContainer(ctx, containerName)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to inspect container %s: %w", containerName, err)
|
||||
}
|
||||
return containermanager.NeedsMigrationFromLabels(inspect.Labels), nil
|
||||
}
|
||||
|
||||
func (p *Podman) InspectContainer(ctx context.Context, containerName string) (*containermanager.InspectResult, error) {
|
||||
config := containermanager.InspectResult{}
|
||||
args := []string{"inspect", "--type", "container", "--format", "json", containerName}
|
||||
@@ -838,6 +855,10 @@ func (p *Podman) InspectContainer(ctx context.Context, containerName string) (*c
|
||||
config.UnshareGroups = true
|
||||
}
|
||||
|
||||
// Expose the full label set so callers (e.g. migrate) can read
|
||||
// distrobox.version and other distrobox-managed labels.
|
||||
config.Labels = inspect.Config.Labels
|
||||
|
||||
// Extract HOME and PATH from container env
|
||||
for _, env := range inspect.Config.Env {
|
||||
if strings.HasPrefix(env, "HOME=") {
|
||||
|
||||
@@ -41,6 +41,7 @@ type ContainerManagerSpy struct {
|
||||
Commit [][]any
|
||||
ImageExists [][]any
|
||||
PullImage [][]any
|
||||
NeedsMigration [][]any
|
||||
}
|
||||
|
||||
// MockContainerManager is a no-op container manager for testing.
|
||||
@@ -57,6 +58,11 @@ type ContainerManagerSpy struct {
|
||||
// ListContainersResult and InspectContainerResult, when non-nil, override
|
||||
// the default zero-value return values of ListContainers and
|
||||
// InspectContainer respectively.
|
||||
//
|
||||
// NeedsMigrationResult, when non-nil, overrides the default return value
|
||||
// of NeedsMigration. The default is true, matching how a freshly
|
||||
// inspected mock container looks like a v1 container (no version label
|
||||
// in InspectContainerResult.Labels).
|
||||
type MockContainerManager struct {
|
||||
Spy ContainerManagerSpy
|
||||
Root bool
|
||||
@@ -64,6 +70,7 @@ type MockContainerManager struct {
|
||||
ExistsFn func(containerName string) bool
|
||||
ListContainersResult []containermanager.Container
|
||||
InspectContainerResult *containermanager.InspectResult
|
||||
NeedsMigrationResult *bool
|
||||
}
|
||||
|
||||
func (m *MockContainerManager) Name() string {
|
||||
@@ -82,6 +89,7 @@ func (m *MockContainerManager) CloneAsRoot() containermanager.ContainerManager {
|
||||
ExistsFn: m.ExistsFn,
|
||||
ListContainersResult: m.ListContainersResult,
|
||||
InspectContainerResult: m.InspectContainerResult,
|
||||
NeedsMigrationResult: m.NeedsMigrationResult,
|
||||
}
|
||||
}
|
||||
return m.RootClone
|
||||
@@ -136,6 +144,16 @@ func (m *MockContainerManager) Commit(_ context.Context, containerID string, tag
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *MockContainerManager) NeedsMigration(_ context.Context, containerName string) (bool, error) {
|
||||
m.Spy.NeedsMigration = append(m.Spy.NeedsMigration, []any{containerName})
|
||||
if m.NeedsMigrationResult != nil {
|
||||
return *m.NeedsMigrationResult, nil
|
||||
}
|
||||
// Default: a fresh mock container looks like it needs migration
|
||||
// (no version label set on InspectContainerResult).
|
||||
return true, nil
|
||||
}
|
||||
|
||||
func (m *MockContainerManager) ImageExists(_ context.Context, imageName string) bool {
|
||||
m.Spy.ImageExists = append(m.Spy.ImageExists, []any{imageName})
|
||||
return false
|
||||
|
||||
Reference in New Issue
Block a user