Tell what a container inherits from what it overrides

Checked against a real stalwartlabs/stalwart image, `docker inspect` on an
ordinary container reports User "stalwart", Entrypoint
["/usr/local/bin/stalwart"] and Cmd ["--config",
"/etc/stalwart/config.json"] — all three inherited, none of them given.

Two things followed from reading those as the operator's.

A container user was listed as configuration a recreate would drop, so
every container off the official image was refused as unrecreatable. That
refusal lived in cutover, downstream of the stop, the settings conversion
and the store migration: it arrived with mail down and data already
moved, which is the failure issue #1 was filed for. Each of the three is
now compared against `docker image inspect` of the image the container is
on. Inherited values are left to the new image, whose own defaults are
the ones that go with it. Overrides are carried: --user, --entrypoint,
and the rest of an entrypoint as leading argv. Cmd and Entrypoint were
not being read at all, so an overridden one was silently dropped — the
exact loss the unsupported list exists to prevent.

The recreatability question also moved into preflight, while the server
is still running. Cutover asks it again, since the two are separated by
the whole migration, but only one of them can refuse without cost.

The other half: the recreated container is now started with `--config`
pointing at the migrated config in the data volume. Left to the image's
default command it came up on /etc/stalwart/config.json — a different
volume, holding whatever the old version left there — so cutover would
have produced a running server with nothing to do with the migration that
preceded it. An overridden command and that --config are the same argv
and cannot be merged honestly, so a container with one is refused and
told why.

The config is also chowned to whatever owns the data directory, before
the recovery cycle opens it. The image runs as uid 2000 and this tool
writes as root; §4.8 is the standing reminder that byte-perfect and
unreadable is a way to report success.

Found while checking @kaya-eu's field report in #1 against a real image.
Their three manual migrations are where the config step comes from.
This commit is contained in:
2026-08-29 17:45:23 -07:00
parent 964b61bb47
commit e96e72bf79
8 changed files with 654 additions and 25 deletions
+29 -3
View File
@@ -300,7 +300,21 @@ against an already-migrated store.
it carries across, naming what it found. The list is conservative and it carries across, naming what it found. The list is conservative and
deliberately not exhaustive; docker's HostConfig has far more fields than deliberately not exhaustive; docker's HostConfig has far more fields than
it checks, and one it does not know about is a reason not to be it checks, and one it does not know about is a reason not to be
recreating that container at all. recreating that container at all. That question is asked in preflight,
while the server is still running, and again at cutover: the answer does
not change between them, and only one of the two points can refuse
without having already cost an outage.
- What a container *inherits* from its image is not what it *overrides*,
and only the override is the operator's. `docker inspect` reports
`User`, `Cmd` and `Entrypoint` either way - a container off the official
image reports user `stalwart` and command
`--config /etc/stalwart/config.json` having been given neither - so each
is compared against `docker image inspect` of the image the container is
actually on. An inherited value is left to the new image, whose own
defaults are the ones that go with it; an override is carried onto the
recreate. Reading an inherited value as an override is not a harmless
over-refusal: before this distinction existed, every ordinary container
off the official image was refused at cutover, after the stop.
- The old container is renamed rather than removed, and the old image is - The old container is renamed rather than removed, and the old image is
never pruned. Together they are the container's manual restore path, the never pruned. Together they are the container's manual restore path, the
nearest equivalent to the preserved binary of §4.2: one command starts nearest equivalent to the preserved binary of §4.2: one command starts
@@ -852,8 +866,20 @@ happens to need them. `preflight.DeploymentKind` is a type alias for
`run` writes it under the host side of whichever mount covers `run` writes it under the host side of whichever mount covers
`--data-dir` and names it on the container side, because cutover recreates `--data-dir` and names it on the container side, because cutover recreates
a container with the mounts it had and cannot invent a new one for a a container with the mounts it had and cannot invent a new one for a
config file. That is an inference from how the mounts must line up rather config file. Cutover then starts the new container with `--config` at
than something a real deployment has confirmed. that path, because the image's own default command points at
`/etc/stalwart/config.json` - a different volume, holding whatever the
old version left there. An overridden command and that `--config` are
the same argv and cannot be merged honestly, so a container with one is
refused rather than guessed at.
The config is also chowned to whatever owns the data directory before
anything reads it: the official image runs as uid 2000, and a
root-owned 0640 config is one the server cannot open - a failure that
arrives as a recovery boot that never comes up rather than as a
permission error anyone would recognise. §4.8 is why that is not left to
chance.
The path itself remains an inference from how the mounts must line up
rather than something a real deployment has confirmed.
- **Cutover ignores systemd drop-ins.** It rewrites only the main unit - **Cutover ignores systemd drop-ins.** It rewrites only the main unit
file, so an `ExecStart` or `Environment` override in file, so an `ExecStart` or `Environment` override in
`/etc/systemd/system/stalwart.service.d/*.conf` is invisible to it - `/etc/systemd/system/stalwart.service.d/*.conf` is invisible to it -
+49
View File
@@ -0,0 +1,49 @@
// SPDX-FileCopyrightText: 2026 LINUXexpert-org
// SPDX-License-Identifier: GPL-3.0-or-later
package main
import (
"fmt"
"os"
"syscall"
)
// matchOwnership gives paths the uid and gid of reference.
//
// It exists for the container path, where this tool writes the migrated
// config onto the host side of a volume and something inside the container
// has to read it. That something is not root: the official Stalwart image
// runs as uid 2000, so a config written root-owned and 0640 is a config
// the server cannot open, and the failure arrives as a recovery boot that
// will not come up rather than as a permissions error anyone would
// recognise.
//
// The reference is the data directory itself, whose ownership is already
// whatever the container writes as. Copying it is more reliable than
// resolving a user out of the image, and it is the same move
// cutover.installConfig makes for the systemd path with
// ConfigOwnerReference.
//
// ARCHITECTURE.md §4.8 is the reason this is not left to chance: the
// rollback that was removed from this tool restored every byte correctly
// and lost ownership, and reported success.
func matchOwnership(reference string, paths ...string) (string, error) {
info, err := os.Stat(reference)
if err != nil {
return "", fmt.Errorf("stat %s to copy its ownership: %w", reference, err)
}
sys, ok := info.Sys().(*syscall.Stat_t)
if !ok {
return "ownership unchanged (this platform does not report it)", nil
}
uid, gid := int(sys.Uid), int(sys.Gid)
for _, p := range paths {
if err := os.Chown(p, uid, gid); err != nil {
return "", fmt.Errorf(
"set ownership on %s to %d:%d, which is what owns %s - the server in the container runs as that user and "+
"cannot read a config it does not own: %w", p, uid, gid, reference, err)
}
}
return fmt.Sprintf("uid %d, gid %d, copied from %s", uid, gid, reference), nil
}
+34 -3
View File
@@ -240,6 +240,7 @@ func runRun(args []string) (err error) {
// side. // side.
convertedConfig := filepath.Join(runWorkDir, "config.json") convertedConfig := filepath.Join(runWorkDir, "config.json")
containerConfigPath := "" containerConfigPath := ""
containerConfigDir, containerConfigOwner := "", ""
if isContainer { if isContainer {
mount, ok := containerFacts.MountFor(*dataDir) mount, ok := containerFacts.MountFor(*dataDir)
if !ok { if !ok {
@@ -254,6 +255,8 @@ func runRun(args []string) (err error) {
} }
convertedConfig = filepath.Join(hostDir, "config.json") convertedConfig = filepath.Join(hostDir, "config.json")
containerConfigPath = path.Join(mount.Destination, "stalwart-migrate", "config.json") containerConfigPath = path.Join(mount.Destination, "stalwart-migrate", "config.json")
containerConfigOwner = mount.Source
containerConfigDir = hostDir
} }
convertedExport := filepath.Join(runWorkDir, "export.json") convertedExport := filepath.Join(runWorkDir, "export.json")
unmigratedPath := filepath.Join(runWorkDir, "unmigrated.txt") unmigratedPath := filepath.Join(runWorkDir, "unmigrated.txt")
@@ -378,6 +381,17 @@ func runRun(args []string) (err error) {
if len(tenantFix.Adoptions) > 0 { if len(tenantFix.Adoptions) > 0 {
detail += " - " + tenantFix.String() detail += " - " + tenantFix.String()
} }
// The container reads this config, and it does not run as
// root. Done here rather than at cutover because the recovery
// cycle is the first thing to open it, and a config it cannot
// read surfaces there as a boot that never comes up.
if containerConfigOwner != "" {
owner, err := matchOwnership(containerConfigOwner, containerConfigDir, convertedConfig)
if err != nil {
return checkpoint.StepOutcome{}, err
}
detail += fmt.Sprintf("; %s is owned %s", convertedConfig, owner)
}
return checkpoint.StepOutcome{Detail: detail}, nil return checkpoint.StepOutcome{Detail: detail}, nil
}); err != nil { }); err != nil {
return fmt.Errorf("convert settings: %w", err) return fmt.Errorf("convert settings: %w", err)
@@ -446,7 +460,7 @@ func runRun(args []string) (err error) {
ServiceUnitPath: *serviceUnitPath, ConfigPath: *newConfigPath, ServiceUnitPath: *serviceUnitPath, ConfigPath: *newConfigPath,
ConfigSource: configSource, ConfigOwnerReference: *configPath, ConfigSource: configSource, ConfigOwnerReference: *configPath,
Deployment: service.Options{Kind: service.Kind(rs.Topology.DeploymentKind), UnitName: *unitName, ContainerName: *containerName}, Deployment: service.Options{Kind: service.Kind(rs.Topology.DeploymentKind), UnitName: *unitName, ContainerName: *containerName},
Container: containerCutover(isContainer, *containerName, stagedImage, runStateDir), Container: containerCutover(isContainer, *containerName, stagedImage, runStateDir, containerConfigOf(p, containerConfigPath)),
RecoveryPointConfirmed: *recoveryConfirmed, RecoveryPointConfirmed: *recoveryConfirmed,
AdminURL: *adminURL, AdminUser: *adminUser, AdminPassword: *adminPassword, AdminURL: *adminURL, AdminUser: *adminUser, AdminPassword: *adminPassword,
HTTPClient: httpClient, RecalculateQuotas: *recalcQuotas && p.CrossesMajorBoundary, HTTPClient: httpClient, RecalculateQuotas: *recalcQuotas && p.CrossesMajorBoundary,
@@ -524,9 +538,26 @@ func buildSupplement(settingsPath, principalsPath, unmigratedPath, outPath strin
// containerCutover is the cutover options for a container deployment, or // containerCutover is the cutover options for a container deployment, or
// nil for a binary one. Nil is what keeps cutover refusing a container it // nil for a binary one. Nil is what keeps cutover refusing a container it
// was given no image to recreate from. // was given no image to recreate from.
func containerCutover(isContainer bool, name, image, preserveDir string) *cutover.ContainerOptions { // containerConfigOf is the container-side config path cutover should start
// the new container with, which is none at all for a patch bump: nothing
// was converted, so the container keeps the command its image gives it and
// the configuration it was already running under.
func containerConfigOf(p *plan.Plan, configPath string) string {
if !p.CrossesMajorBoundary {
return ""
}
return configPath
}
func containerCutover(isContainer bool, name, image, preserveDir, configPath string) *cutover.ContainerOptions {
if !isContainer { if !isContainer {
return nil return nil
} }
return &cutover.ContainerOptions{ContainerName: name, StagedImage: image, PreserveDir: preserveDir} // configPath is the container-side path to the migrated config, and is
// empty for a patch bump that converted nothing - there the container
// keeps whatever command its image gives it, which is what it was
// already running under.
return &cutover.ContainerOptions{
ContainerName: name, StagedImage: image, PreserveDir: preserveDir, ConfigPath: configPath,
}
} }
+59 -1
View File
@@ -36,6 +36,18 @@ type ContainerOptions struct {
// PreserveDir is where the inspected definition is written. // PreserveDir is where the inspected definition is written.
PreserveDir string PreserveDir string
// ConfigPath is the migrated v0.16 config, named as the *container*
// sees it - the path inside a mount the container already has, since a
// recreate carries the mounts it had and cannot invent a new one.
//
// Without this the recreated container falls back to its image's own
// default command, which on the official image is
// `--config /etc/stalwart/config.json`. That path is a different
// volume from the data directory and holds whatever the old version
// left there, so the new container would come up on a config that has
// nothing to do with the migration that just happened.
ConfigPath string
DockerBinary string DockerBinary string
} }
@@ -96,6 +108,11 @@ func runContainerCutover(ctx context.Context, rs *checkpoint.RunState, step step
return err return err
} }
// Preflight asked this too, before anything stopped (§4.1). It is
// asked again here because the two are separated by the whole
// migration, and a container can be reconfigured in between - but by
// the time this refuses, the answer has cost an outage, which is why
// preflight is where it is meant to be caught.
if err := step("container-is-recreatable", func() (checkpoint.StepOutcome, error) { if err := step("container-is-recreatable", func() (checkpoint.StepOutcome, error) {
if len(facts.Unsupported) > 0 { if len(facts.Unsupported) > 0 {
return checkpoint.StepOutcome{}, fmt.Errorf( return checkpoint.StepOutcome{}, fmt.Errorf(
@@ -104,6 +121,19 @@ func runContainerCutover(ctx context.Context, rs *checkpoint.RunState, step step
"the definition is preserved as %s, and the staged image is %s", "the definition is preserved as %s, and the staged image is %s",
strings.Join(facts.Unsupported, "; "), rs.Artifacts[ArtifactContainerDefinition].Path, opts.StagedImage) strings.Join(facts.Unsupported, "; "), rs.Artifacts[ArtifactContainerDefinition].Path, opts.StagedImage)
} }
// A command of the operator's own and a config this tool has to
// hand over are the same argv, and there is no honest way to merge
// them: their command may point at another config, or at something
// that is not the server at all. Refusing names both rather than
// picking one and being quietly wrong about which server came up.
if opts.ConfigPath != "" && len(facts.Cmd) > 0 {
return checkpoint.StepOutcome{}, fmt.Errorf(
"this container overrides its image's command (%s), and cutting over has to start the new one with "+
"`--config %s` - the migrated configuration. Both are the container's argv and this tool will not guess at "+
"a merge. Recreate it by hand from the preserved definition at %s, on image %s, with your command adjusted "+
"to that config",
strings.Join(facts.Cmd, " "), opts.ConfigPath, rs.Artifacts[ArtifactContainerDefinition].Path, opts.StagedImage)
}
return checkpoint.StepOutcome{Detail: "the container's definition is entirely within what a recreate carries across"}, nil return checkpoint.StepOutcome{Detail: "the container's definition is entirely within what a recreate carries across"}, nil
}); err != nil { }); err != nil {
return err return err
@@ -130,6 +160,17 @@ func runContainerCutover(ctx context.Context, rs *checkpoint.RunState, step step
if facts.RestartPolicy != "" && facts.RestartPolicy != "no" { if facts.RestartPolicy != "" && facts.RestartPolicy != "no" {
args = append(args, "--restart", facts.RestartPolicy) args = append(args, "--restart", facts.RestartPolicy)
} }
// Only what the container overrode on its old image is carried.
// An inherited value belongs to the image, and the new image's own
// default is the one that goes with the new image - pinning the
// old image's USER or ENTRYPOINT onto it would be carrying across
// a decision nobody made.
if facts.User != "" {
args = append(args, "--user", facts.User)
}
if len(facts.Entrypoint) > 0 {
args = append(args, "--entrypoint", facts.Entrypoint[0])
}
for _, e := range facts.Env { for _, e := range facts.Env {
// Recovery-mode variables must never survive into a normal // Recovery-mode variables must never survive into a normal
// start: leaving STALWART_RECOVERY_MODE set would recovery-boot // start: leaving STALWART_RECOVERY_MODE set would recovery-boot
@@ -164,12 +205,29 @@ func runContainerCutover(ctx context.Context, rs *checkpoint.RunState, step step
} }
args = append(args, opts.StagedImage) args = append(args, opts.StagedImage)
// Everything after the image is the container's argv. `docker run`
// takes only the first word of an entrypoint as --entrypoint, so
// the rest of it leads here.
if len(facts.Entrypoint) > 1 {
args = append(args, facts.Entrypoint[1:]...)
}
switch {
case opts.ConfigPath != "":
args = append(args, "--config", opts.ConfigPath)
case len(facts.Cmd) > 0:
args = append(args, facts.Cmd...)
}
if out, err := dockerOut(ctx, opts.docker(), args...); err != nil { if out, err := dockerOut(ctx, opts.docker(), args...); err != nil {
return checkpoint.StepOutcome{}, fmt.Errorf( return checkpoint.StepOutcome{}, fmt.Errorf(
"create %s from %s: %w (%s). The previous container is still here as %s", "create %s from %s: %w (%s). The previous container is still here as %s",
opts.ContainerName, opts.StagedImage, err, out, retired) opts.ContainerName, opts.StagedImage, err, out, retired)
} }
return checkpoint.StepOutcome{Detail: fmt.Sprintf("recreated %s on image %s", opts.ContainerName, opts.StagedImage)}, nil detail := fmt.Sprintf("recreated %s on image %s", opts.ContainerName, opts.StagedImage)
if opts.ConfigPath != "" {
detail += ", started with --config " + opts.ConfigPath
}
return checkpoint.StepOutcome{Detail: detail}, nil
}) })
} }
+178 -1
View File
@@ -37,6 +37,12 @@ func inspectJSON(t *testing.T, extraHost map[string]any, networks map[string]any
"Config": map[string]any{ "Config": map[string]any{
"Image": "stalwartlabs/stalwart:v0.15.5", "Image": "stalwartlabs/stalwart:v0.15.5",
"Env": []string{"TZ=UTC", "STALWART_RECOVERY_MODE=1"}, "Env": []string{"TZ=UTC", "STALWART_RECOVERY_MODE=1"},
// Matching imageJSON's defaults, so nothing here reads as an
// override. A container off the official image reports all
// three having been given none of them.
"User": imageUser,
"Entrypoint": imageEntrypoint,
"Cmd": imageCmd,
}, },
"State": map[string]any{"Running": false}, "State": map[string]any{"Running": false},
"Mounts": []map[string]any{{"Type": "volume", "Name": "stalwart-data", "Destination": "/opt/stalwart", "RW": true}}, "Mounts": []map[string]any{{"Type": "volume", "Name": "stalwart-data", "Destination": "/opt/stalwart", "RW": true}},
@@ -50,6 +56,28 @@ func inspectJSON(t *testing.T, extraHost map[string]any, networks map[string]any
return string(b) return string(b)
} }
// The defaults the official Stalwart image gives every container made
// from it. They are here rather than inline because the whole point of
// the image comparison is that a container reporting exactly these has
// overridden nothing.
var (
imageUser = "stalwart"
imageEntrypoint = []string{"/usr/local/bin/stalwart"}
imageCmd = []string{"--config", "/etc/stalwart/config.json"}
)
// imageJSON is `docker image inspect` for the image a container is on.
func imageJSON(t *testing.T) string {
t.Helper()
b, err := json.Marshal([]map[string]any{{
"Config": map[string]any{"User": imageUser, "Entrypoint": imageEntrypoint, "Cmd": imageCmd},
}})
if err != nil {
t.Fatal(err)
}
return string(b)
}
// fakeDockerCutover installs a docker that records arguments and serves the // fakeDockerCutover installs a docker that records arguments and serves the
// given inspect document. // given inspect document.
func fakeDockerCutover(t *testing.T, doc string) (log string) { func fakeDockerCutover(t *testing.T, doc string) (log string) {
@@ -60,14 +88,21 @@ func fakeDockerCutover(t *testing.T, doc string) (log string) {
if err := os.WriteFile(inspectFile, []byte(doc), 0o644); err != nil { if err := os.WriteFile(inspectFile, []byte(doc), 0o644); err != nil {
t.Fatal(err) t.Fatal(err)
} }
imageFile := filepath.Join(dir, "image.json")
if err := os.WriteFile(imageFile, []byte(imageJSON(t)), 0o644); err != nil {
t.Fatal(err)
}
script := fmt.Sprintf(`#!/bin/sh script := fmt.Sprintf(`#!/bin/sh
echo "$@" >> %q echo "$@" >> %q
case "$1 $2" in
"image inspect") cat %q ; exit 0 ;;
esac
case "$1" in case "$1" in
inspect) cat %q ;; inspect) cat %q ;;
rename) exit 0 ;; rename) exit 0 ;;
run) echo newcontainerid ;; run) echo newcontainerid ;;
esac esac
`, log, inspectFile) `, log, imageFile, inspectFile)
if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0o755); err != nil { if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0o755); err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -95,10 +130,15 @@ func runContainerFor(t *testing.T, doc string) (*checkpoint.RunState, Report, er
} }
err = runContainerCutover(context.Background(), rs, step, ContainerOptions{ err = runContainerCutover(context.Background(), rs, step, ContainerOptions{
ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(), ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(),
ConfigPath: containerConfig,
}) })
return rs, report, err return rs, report, err
} }
// containerConfig is the container-side path to the migrated config that
// run passes to cutover, mirroring run.go's <data mount>/stalwart-migrate.
const containerConfig = "/opt/stalwart/stalwart-migrate/config.json"
func TestContainerCutoverPreservesTheDefinitionFirst(t *testing.T) { func TestContainerCutoverPreservesTheDefinitionFirst(t *testing.T) {
rs, _, err := runContainerFor(t, inspectJSON(t, nil, nil)) rs, _, err := runContainerFor(t, inspectJSON(t, nil, nil))
if err != nil { if err != nil {
@@ -252,3 +292,140 @@ func readLog(t *testing.T, path string) string {
} }
return string(b) return string(b)
} }
// The recreated container has to be started on the config the migration
// just produced. Left to the image's own default command it would come up
// on /etc/stalwart/config.json - a different volume from the data
// directory, holding whatever the old version left there - and be a
// server with nothing to do with the migration that preceded it.
// @kaya-eu did this step by hand on three real migrations.
func TestContainerCutoverStartsOnTheMigratedConfig(t *testing.T) {
log := fakeDockerCutover(t, inspectJSON(t, nil, nil))
store := checkpoint.NewStore(t.TempDir())
rs, err := store.Create("0.15.5", "0.16.14")
if err != nil {
t.Fatal(err)
}
if err := runContainerCutover(context.Background(), rs, noopStep(store, rs), ContainerOptions{
ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(),
ConfigPath: containerConfig,
}); err != nil {
t.Fatalf("runContainerCutover: %v", err)
}
run := runLine(t, log)
if !strings.Contains(run, "--config "+containerConfig) {
t.Errorf("run should start the container on the migrated config, got: %s", run)
}
// After the image, not before: everything past it is the argv.
if strings.Index(run, "sha256:new") > strings.Index(run, "--config "+containerConfig) {
t.Errorf("--config must come after the image, got: %s", run)
}
}
// An inherited USER belongs to the image. Passing --user stalwart to the
// new image would be carrying across a decision nobody made, and would
// break outright on an image that named its user differently.
func TestContainerCutoverDoesNotCarryInheritedDefaults(t *testing.T) {
log := fakeDockerCutover(t, inspectJSON(t, nil, nil))
store := checkpoint.NewStore(t.TempDir())
rs, _ := store.Create("0.15.5", "0.16.14")
if err := runContainerCutover(context.Background(), rs, noopStep(store, rs), ContainerOptions{
ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(),
ConfigPath: containerConfig,
}); err != nil {
t.Fatalf("runContainerCutover: %v", err)
}
run := runLine(t, log)
for _, unwanted := range []string{"--user", "--entrypoint"} {
if strings.Contains(run, unwanted) {
t.Errorf("run carried %s across from the old image's defaults: %s", unwanted, run)
}
}
}
// What the operator did override is theirs, and a recreate that drops it
// starts cleanly as a different server - the failure Unsupported exists to
// prevent, for two settings that were not being read at all.
func TestContainerCutoverCarriesTheOperatorsOverrides(t *testing.T) {
doc := inspectJSON(t, nil, nil)
doc = strings.Replace(doc, `"User":"stalwart"`, `"User":"1500:1500"`, 1)
doc = strings.Replace(doc, `"Entrypoint":["/usr/local/bin/stalwart"]`,
`"Entrypoint":["/usr/local/bin/wrapper","--trace"]`, 1)
log := fakeDockerCutover(t, doc)
store := checkpoint.NewStore(t.TempDir())
rs, _ := store.Create("0.15.5", "0.16.14")
if err := runContainerCutover(context.Background(), rs, noopStep(store, rs), ContainerOptions{
ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(),
ConfigPath: containerConfig,
}); err != nil {
t.Fatalf("runContainerCutover: %v", err)
}
run := runLine(t, log)
if !strings.Contains(run, "--user 1500:1500") {
t.Errorf("run should carry the overridden user: %s", run)
}
// docker run takes one word as --entrypoint; the rest is argv.
if !strings.Contains(run, "--entrypoint /usr/local/bin/wrapper") {
t.Errorf("run should carry the overridden entrypoint: %s", run)
}
if !strings.Contains(run, "sha256:new --trace") {
t.Errorf("the rest of the entrypoint should lead the argv: %s", run)
}
}
// A command of the operator's own and the config this tool has to hand
// over are the same argv. Their command may point at another config, or
// at something that is not the server - so this refuses rather than
// merging and being quietly wrong about which server came up.
func TestContainerCutoverRefusesToMergeACommandWithTheConfig(t *testing.T) {
doc := strings.Replace(inspectJSON(t, nil, nil),
`"Cmd":["--config","/etc/stalwart/config.json"]`, `"Cmd":["--config","/srv/mine.toml"]`, 1)
_, _, err := runContainerFor(t, doc)
if err == nil {
t.Fatal("want a refusal when an overridden command collides with the migrated config")
}
for _, want := range []string{"/srv/mine.toml", containerConfig, "by hand"} {
if !strings.Contains(err.Error(), want) {
t.Errorf("refusal should name %q, got: %v", want, err)
}
}
}
// A patch bump converts nothing, so there is no config to point at and
// the container keeps the command its image gives it.
func TestContainerCutoverKeepsTheImageCommandWithoutAConfig(t *testing.T) {
log := fakeDockerCutover(t, inspectJSON(t, nil, nil))
store := checkpoint.NewStore(t.TempDir())
rs, _ := store.Create("0.16.14", "0.16.19")
if err := runContainerCutover(context.Background(), rs, noopStep(store, rs), ContainerOptions{
ContainerName: "stalwart", StagedImage: "sha256:new", PreserveDir: t.TempDir(),
}); err != nil {
t.Fatalf("runContainerCutover: %v", err)
}
if run := runLine(t, log); !strings.HasSuffix(strings.TrimSpace(run), "sha256:new") {
t.Errorf("run should end at the image, with no argv of its own: %s", run)
}
}
func noopStep(store *checkpoint.Store, rs *checkpoint.RunState) stepFunc {
return func(name string, fn func() (checkpoint.StepOutcome, error)) error {
_, err := store.RunStep(rs, checkpoint.PhaseCutover, name, fn)
return err
}
}
// runLine is the `docker run` the fake recorded.
func runLine(t *testing.T, log string) string {
t.Helper()
data, err := os.ReadFile(log)
if err != nil {
t.Fatal(err)
}
for _, line := range strings.Split(string(data), "\n") {
if strings.HasPrefix(line, "run ") {
return line
}
}
t.Fatalf("no `docker run` in the recorded arguments:\n%s", data)
return ""
}
+55 -1
View File
@@ -474,6 +474,14 @@ func withFakeDocker(t *testing.T) {
// dockerPreflight runs a minimal but real preflight against a fake 0.15.5 // dockerPreflight runs a minimal but real preflight against a fake 0.15.5
// install with a fake docker on PATH. // install with a fake docker on PATH.
func dockerPreflight(t *testing.T, advisory bool) Report { func dockerPreflight(t *testing.T, advisory bool) Report {
t.Helper()
return dockerPreflightOn(t, advisory, nil)
}
// dockerPreflightOn runs preflight against a container whose inspect
// document has been rewritten by edit, for the cases that need it to be
// something other than an ordinary one.
func dockerPreflightOn(t *testing.T, advisory bool, edit func(string) string) Report {
t.Helper() t.Helper()
// disk-space stats DataDir on this host, and container-data-volume // disk-space stats DataDir on this host, and container-data-volume
// wants it covered by a mount, so it has to be both: a real directory, // wants it covered by a mount, so it has to be both: a real directory,
@@ -484,7 +492,11 @@ func dockerPreflight(t *testing.T, advisory bool) Report {
t.Skipf("host has %s, which detection prefers over docker", p) t.Skipf("host has %s, which detection prefers over docker", p)
} }
} }
fakeInspect(t, inspectDoc(t, nil, []Mount{dataVolume(dataDir)})) doc := inspectDoc(t, nil, []Mount{dataVolume(dataDir)})
if edit != nil {
doc = edit(doc)
}
fakeInspect(t, doc)
counterPath := filepath.Join(t.TempDir(), "invocations") counterPath := filepath.Join(t.TempDir(), "invocations")
binaryPath := writeFakeBinary(t, "0.15.5", counterPath) binaryPath := writeFakeBinary(t, "0.15.5", counterPath)
@@ -553,3 +565,45 @@ func TestRehearseStillRunsAgainstADockerDeployment(t *testing.T) {
t.Fatalf("advisory mode should not block on docker, got:\n%s", report.String()) t.Fatalf("advisory mode should not block on docker, got:\n%s", report.String())
} }
} }
// Cutover already refused a container it could not recreate, but cutover
// is downstream of the stop, the settings conversion and the store
// migration - so that refusal arrived with the mail down and the data
// already moved, which is the shape of failure issue #1 was filed for.
// The answer never changes between the two points, so it is asked here,
// while the server is still running.
func TestPreflightRefusesAContainerItCouldNotRecreate(t *testing.T) {
report := dockerPreflightOn(t, false, func(doc string) string {
return strings.Replace(doc, `"State":`, `"HostConfig":{"Privileged":true},"State":`, 1)
})
if !report.Blocking() {
t.Fatalf("a container preflight cannot recreate should block before anything stops:\n%s", report.String())
}
var found bool
for _, res := range report.Results {
if res.Name == "container-recreatable" {
found = true
if res.Status != StatusFail {
t.Errorf("container-recreatable status = %q, want %q", res.Status, StatusFail)
}
if !strings.Contains(res.Detail, "privileged") {
t.Errorf("detail should name what would be dropped, got %q", res.Detail)
}
}
}
if !found {
t.Fatalf("no container-recreatable result in report:\n%s", report.String())
}
}
// rehearse never stops or recreates anything, so the same finding is a
// warning there: an operator migrating by hand needs to know it more than
// an automated run does.
func TestRehearseWarnsRatherThanBlocksOnAnUnrecreatableContainer(t *testing.T) {
report := dockerPreflightOn(t, true, func(doc string) string {
return strings.Replace(doc, `"State":`, `"HostConfig":{"Privileged":true},"State":`, 1)
})
if report.Blocking() {
t.Fatalf("advisory mode should not block, got:\n%s", report.String())
}
}
+140 -11
View File
@@ -55,7 +55,23 @@ type ContainerFacts struct {
Ports map[string][]PortBinding Ports map[string][]PortBinding
RestartPolicy string RestartPolicy string
NetworkMode string NetworkMode string
Unsupported []string // populated by UnsupportedForRecreate Unsupported []string // populated by unsupportedForRecreate
// User, Entrypoint and Cmd are set only when the container overrides
// what its image already says.
//
// The distinction is the whole point. `docker inspect` reports these
// three whether the operator set them or the image did - a container
// off the official image reports User "stalwart" and Cmd
// ["--config", "/etc/stalwart/config.json"] having been given
// neither. Treating an inherited value as the operator's would either
// refuse every ordinary container or pin the new image to the old
// image's defaults, and the new image's defaults are the ones that go
// with the new image. Only a genuine override is the operator's
// decision, and only that has to survive a recreate.
User string
Entrypoint []string
Cmd []string
} }
// PortBinding is one published port. // PortBinding is one published port.
@@ -107,15 +123,10 @@ func (f ContainerFacts) MountFor(path string) (Mount, bool) {
// separately from ContainerFacts because docker's shape is docker's to // separately from ContainerFacts because docker's shape is docker's to
// change, and the rest of this package should not have to know it. // change, and the rest of this package should not have to know it.
type inspectOutput struct { type inspectOutput struct {
Name string `json:"Name"` Name string `json:"Name"`
Image string `json:"Image"` Image string `json:"Image"`
Config struct { Config containerConfig `json:"Config"`
Image string `json:"Image"` State struct {
Labels map[string]string `json:"Labels"`
Env []string `json:"Env"`
User string `json:"User"`
} `json:"Config"`
State struct {
Running bool `json:"Running"` Running bool `json:"Running"`
} `json:"State"` } `json:"State"`
Mounts []Mount `json:"Mounts"` Mounts []Mount `json:"Mounts"`
@@ -145,6 +156,24 @@ type inspectOutput struct {
} `json:"NetworkSettings"` } `json:"NetworkSettings"`
} }
// containerConfig is the part of a container's or an image's Config this
// reasons about. Both docker objects carry the same shape here, which is
// what makes comparing them possible.
type containerConfig struct {
Image string `json:"Image"`
Labels map[string]string `json:"Labels"`
Env []string `json:"Env"`
User string `json:"User"`
Entrypoint []string `json:"Entrypoint"`
Cmd []string `json:"Cmd"`
}
// imageInspectOutput is `docker image inspect`, which reports the defaults
// a container inherits when it was given none of its own.
type imageInspectOutput struct {
Config containerConfig `json:"Config"`
}
// InspectContainer reads the facts about containerName. An error here is // InspectContainer reads the facts about containerName. An error here is
// an error, not an absent container: callers reach this only after // an error, not an absent container: callers reach this only after
// DetectDeploymentKind has already established that a container answers to // DetectDeploymentKind has already established that a container answers to
@@ -178,10 +207,66 @@ func InspectContainer(ctx context.Context, containerName string) (ContainerFacts
RestartPolicy: c.HostConfig.RestartPolicy.Name, RestartPolicy: c.HostConfig.RestartPolicy.Name,
NetworkMode: c.HostConfig.NetworkMode, NetworkMode: c.HostConfig.NetworkMode,
} }
// The image the container is actually on, by ID rather than by the tag
// it was started from: a tag can have moved since, and then this would
// be comparing the container against something it never inherited
// from.
base, err := inspectImage(ctx, c.Image)
if err != nil {
return ContainerFacts{}, err
}
if c.Config.User != base.User {
f.User = c.Config.User
}
if !sameArgs(c.Config.Entrypoint, base.Entrypoint) {
f.Entrypoint = c.Config.Entrypoint
}
if !sameArgs(c.Config.Cmd, base.Cmd) {
f.Cmd = c.Config.Cmd
}
f.Unsupported = unsupportedForRecreate(c) f.Unsupported = unsupportedForRecreate(c)
return f, nil return f, nil
} }
// inspectImage reads the defaults an image gives the containers made from
// it. A failure here is an error for the same reason a failed container
// inspect is: without it there is no way to tell an operator's --user from
// the image's own USER, and the difference decides what a recreate has to
// carry.
func inspectImage(ctx context.Context, imageID string) (containerConfig, error) {
if imageID == "" {
return containerConfig{}, fmt.Errorf("preflight: container reports no image to compare its configuration against")
}
out, err := exec.CommandContext(ctx, "docker", "image", "inspect", imageID).Output()
if err != nil {
return containerConfig{}, fmt.Errorf("preflight: docker image inspect %s: %w", imageID, err)
}
var got []imageInspectOutput
if err := json.Unmarshal(out, &got); err != nil {
return containerConfig{}, fmt.Errorf("preflight: parsing docker image inspect %s: %w", imageID, err)
}
if len(got) == 0 {
return containerConfig{}, fmt.Errorf("preflight: docker image inspect %s returned no image", imageID)
}
return got[0].Config, nil
}
// sameArgs compares two argv slices, treating nil and empty as the same
// thing - docker reports an absent Cmd either way depending on version.
func sameArgs(a, b []string) bool {
if len(a) != len(b) {
return false
}
for i := range a {
if a[i] != b[i] {
return false
}
}
return true
}
// unsupportedForRecreate names every piece of this container's // unsupportedForRecreate names every piece of this container's
// configuration that recreating it would not carry across. // configuration that recreating it would not carry across.
// //
@@ -217,7 +302,6 @@ func unsupportedForRecreate(c inspectOutput) []string {
add(len(h.SecurityOpt) > 0, "security options (--security-opt)") add(len(h.SecurityOpt) > 0, "security options (--security-opt)")
add(len(h.Tmpfs) > 0, "tmpfs mounts (--tmpfs)") add(len(h.Tmpfs) > 0, "tmpfs mounts (--tmpfs)")
add(h.LogConfig.Type != "" && h.LogConfig.Type != "json-file", "a non-default log driver (--log-driver "+h.LogConfig.Type+")") add(h.LogConfig.Type != "" && h.LogConfig.Type != "json-file", "a non-default log driver (--log-driver "+h.LogConfig.Type+")")
add(c.Config.User != "", "a container user (--user "+c.Config.User+")")
// A user-defined network is a name in NetworkSettings.Networks that is // A user-defined network is a name in NetworkSettings.Networks that is
// not one of docker's built-ins. Recreating without it puts the server // not one of docker's built-ins. Recreating without it puts the server
@@ -274,6 +358,31 @@ func (c *Checker) runContainerChecks(ctx context.Context, runCheck checkFunc) er
return err return err
} }
if _, err := runCheck("container-recreatable", func() (CheckResult, string) {
// Asked here, while the server is still running, rather than at
// cutover where the answer was first needed. Cutover is downstream
// of the stop, the settings conversion and the store migration, so
// a refusal there is a refusal with the mail already down and the
// data already moved - the shape of failure issue #1 was filed for.
// Nothing about this answer changes between the two points.
if len(facts.Unsupported) > 0 {
status := StatusFail
if c.opts.DeploymentCheckAdvisory {
status = StatusWarn
}
return CheckResult{Status: status, Detail: fmt.Sprintf(
"this container uses configuration that recreating it would not carry across: %s. A container is replaced "+
"rather than edited, so those would be silently dropped and the result would start cleanly without being "+
"the server it was. Migrate this one by hand",
strings.Join(facts.Unsupported, "; "))}, strings.Join(facts.Unsupported, "; ")
}
carried := describeOverrides(facts)
return CheckResult{Status: StatusOK, Detail: "the container's definition is entirely within what a recreate " +
"carries across" + carried}, ""
}); err != nil {
return err
}
_, err := runCheck("container-data-volume", func() (CheckResult, string) { _, err := runCheck("container-data-volume", func() (CheckResult, string) {
writable := facts.WritableMounts() writable := facts.WritableMounts()
if len(writable) == 0 { if len(writable) == 0 {
@@ -307,6 +416,26 @@ func (c *Checker) runContainerChecks(ctx context.Context, runCheck checkFunc) er
return err return err
} }
// describeOverrides names the settings a container holds that its image
// does not, so an operator reading a green check can see what a recreate
// is being trusted to carry rather than taking "entirely within" on faith.
func describeOverrides(f ContainerFacts) string {
var parts []string
if f.User != "" {
parts = append(parts, "--user "+f.User)
}
if len(f.Entrypoint) > 0 {
parts = append(parts, "--entrypoint "+strings.Join(f.Entrypoint, " "))
}
if len(f.Cmd) > 0 {
parts = append(parts, "a command ("+strings.Join(f.Cmd, " ")+")")
}
if len(parts) == 0 {
return ""
}
return ", including what it overrides on its image: " + strings.Join(parts, ", ")
}
func shortID(id string) string { func shortID(id string) string {
id = strings.TrimPrefix(id, "sha256:") id = strings.TrimPrefix(id, "sha256:")
if len(id) > 12 { if len(id) > 12 {
+110 -5
View File
@@ -10,33 +10,72 @@ import (
"net/http" "net/http"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"testing" "testing"
"github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint"
) )
// The defaults the official Stalwart image gives every container made from
// it. A container reporting exactly these has overridden nothing, which is
// the case the image comparison exists to recognise - `docker inspect`
// reports all three either way.
var (
imageUser = "stalwart"
imageEntrypoint = []string{"/usr/local/bin/stalwart"}
imageCmd = []string{"--config", "/etc/stalwart/config.json"}
)
// fakeInspect writes a `docker` that answers `inspect` with the given JSON // fakeInspect writes a `docker` that answers `inspect` with the given JSON
// document, so the container checks can be exercised without a container. // document, so the container checks can be exercised without a container.
// `image inspect` answers with the official image's own defaults.
func fakeInspect(t *testing.T, doc string) { func fakeInspect(t *testing.T, doc string) {
t.Helper()
fakeInspectOn(t, doc, imageDoc(t, imageUser, imageEntrypoint, imageCmd))
}
// fakeInspectOn is fakeInspect with the image's defaults named, for the
// tests that need the container and its image to disagree.
func fakeInspectOn(t *testing.T, containerDoc, imgDoc string) {
t.Helper() t.Helper()
dir := t.TempDir() dir := t.TempDir()
out := filepath.Join(dir, "inspect.json") out := filepath.Join(dir, "inspect.json")
if err := os.WriteFile(out, []byte(doc), 0o644); err != nil { if err := os.WriteFile(out, []byte(containerDoc), 0o644); err != nil {
t.Fatal(err) t.Fatal(err)
} }
script := fmt.Sprintf("#!/bin/sh\ncase \"$1\" in inspect) cat %q ;; *) exit 1 ;; esac\n", out) img := filepath.Join(dir, "image.json")
if err := os.WriteFile(img, []byte(imgDoc), 0o644); err != nil {
t.Fatal(err)
}
script := fmt.Sprintf("#!/bin/sh\n"+
"case \"$1 $2\" in \"image inspect\") cat %q ; exit 0 ;; esac\n"+
"case \"$1\" in inspect) cat %q ;; *) exit 1 ;; esac\n", img, out)
if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0o755); err != nil { if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0o755); err != nil {
t.Fatal(err) t.Fatal(err)
} }
t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
} }
func imageDoc(t *testing.T, user string, entrypoint, cmd []string) string {
t.Helper()
b, err := json.Marshal([]map[string]any{{
"Config": map[string]any{"User": user, "Entrypoint": entrypoint, "Cmd": cmd},
}})
if err != nil {
t.Fatal(err)
}
return string(b)
}
func inspectDoc(t *testing.T, labels map[string]string, mounts []Mount) string { func inspectDoc(t *testing.T, labels map[string]string, mounts []Mount) string {
t.Helper() t.Helper()
doc := []map[string]any{{ doc := []map[string]any{{
"Name": "/stalwart", "Name": "/stalwart",
"Image": "sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", "Image": "sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
"Config": map[string]any{"Image": "stalwartlabs/stalwart:v0.15.5", "Labels": labels}, "Config": map[string]any{
"Image": "stalwartlabs/stalwart:v0.15.5", "Labels": labels,
"User": imageUser, "Entrypoint": imageEntrypoint, "Cmd": imageCmd,
},
"State": map[string]any{"Running": true}, "State": map[string]any{"Running": true},
"Mounts": mounts, "Mounts": mounts,
}} }}
@@ -218,3 +257,69 @@ func TestRehearseReportsContainerProblemsWithoutBlocking(t *testing.T) {
} }
} }
} }
// docker reports Config.User, Cmd and Entrypoint whether the operator set
// them or the image did. A container off the official image reports user
// "stalwart" having been given no --user, and reading that as an operator
// override made this tool refuse to recreate every ordinary Stalwart
// container - at cutover, with the mail already down. Found while checking
// @kaya-eu's field report against a real image.
func TestInspectContainerIgnoresWhatItInheritedFromItsImage(t *testing.T) {
fakeInspect(t, inspectDoc(t, nil, []Mount{dataVolume("/var/lib/stalwart")}))
facts, err := InspectContainer(context.Background(), "stalwart")
if err != nil {
t.Fatalf("InspectContainer: %v", err)
}
if facts.User != "" {
t.Errorf("User = %q, want empty: it is the image's own USER, not an override", facts.User)
}
if len(facts.Cmd) != 0 {
t.Errorf("Cmd = %v, want none: it is the image's own CMD", facts.Cmd)
}
if len(facts.Entrypoint) != 0 {
t.Errorf("Entrypoint = %v, want none: it is the image's own ENTRYPOINT", facts.Entrypoint)
}
if len(facts.Unsupported) != 0 {
t.Errorf("Unsupported = %v, want none for a plain container off the official image", facts.Unsupported)
}
}
// The other half of the same distinction: what the operator really did
// override has to be visible, because a recreate that drops it starts
// cleanly as a different server.
func TestInspectContainerReportsWhatTheOperatorOverrode(t *testing.T) {
doc := inspectDoc(t, nil, []Mount{dataVolume("/var/lib/stalwart")})
doc = strings.Replace(doc, `"User":"stalwart"`, `"User":"1500:1500"`, 1)
doc = strings.Replace(doc, `"Cmd":["--config","/etc/stalwart/config.json"]`, `"Cmd":["--config","/srv/mine.toml"]`, 1)
if strings.Contains(doc, `"User":"stalwart"`) || strings.Contains(doc, "/etc/stalwart/config.json") {
t.Fatal("the fixture did not take the overrides; the inspect document shape changed")
}
fakeInspect(t, doc)
facts, err := InspectContainer(context.Background(), "stalwart")
if err != nil {
t.Fatalf("InspectContainer: %v", err)
}
if facts.User != "1500:1500" {
t.Errorf("User = %q, want the overridden 1500:1500", facts.User)
}
if strings.Join(facts.Cmd, " ") != "--config /srv/mine.toml" {
t.Errorf("Cmd = %v, want the overridden command", facts.Cmd)
}
// An entrypoint it did not override still reads as inherited.
if len(facts.Entrypoint) != 0 {
t.Errorf("Entrypoint = %v, want none", facts.Entrypoint)
}
}
// Without the image's defaults there is no way to tell an override from an
// inheritance, and guessing decides what a recreate carries. Same rule as
// a failed container inspect: an error, not an assumption.
func TestInspectContainerRefusesWhenTheImageCannotBeRead(t *testing.T) {
fakeInspectOn(t, inspectDoc(t, nil, []Mount{dataVolume("/var/lib/stalwart")}), "")
if _, err := InspectContainer(context.Background(), "stalwart"); err == nil {
t.Fatal("want an error when the image's defaults cannot be read")
}
}