Wire the container path up, behind a flag that says what it is
Everything the container migration needs has landed a piece at a time and nothing called any of it. `run` now does: stage pulls and verifies an image instead of downloading a binary, the recovery cycle launches a throwaway container against the live container's own mounts, and cutover recreates it. Preflight's blanket refusal of docker goes with it -- what still refuses is specific to a container rather than to containers, which is compose and data that is not on a volume. It refuses without --container-path-unproven, and that flag is the honest part of this change. Every test drives a fake docker. That proves the right commands are assembled and proves nothing about whether a real image reads the config it is handed -- which is the exact limit ARCHITECTURE.md section 4.8 records about the rollback code that was deleted for being tested only against fakes. A doc note seemed too quiet for a tool that stops a mail server, so it is a flag nobody reaches without being told. The converted config reaches the container through the data volume. It is written under the host side of whichever mount covers --data-dir and named on the container side, because cutover recreates a container with the mounts it had and cannot invent a new one for a config file. --data-dir therefore names the path inside the container, which preflight already says when it matches no mount. PatchPaths stays unused, deliberately. Its documented purpose is pointing a rehearsal at a sandbox; a real container's dumped settings already carry container-side paths, because they come from the live server rather than from a file on this host. The preflight test that asserted docker was refused outright now asserts the replacement rather than being deleted -- "docker is allowed through here" is the thing that would be wrong to regress. Its fixture had to make --data-dir both a real host directory and one the fake container mounts, since disk-space stats it and container-data-volume wants it covered. README gains the container section and, at the top, the note that this is ihasmail's companion.
This commit is contained in:
@@ -200,19 +200,19 @@ func (c *Checker) Run(ctx context.Context, store *checkpoint.Store, rs *checkpoi
|
||||
// automate it - but cutover runs after the service has been
|
||||
// stopped. Refusing there means refusing with mail already down,
|
||||
// which is how a migration attempt turned into an outage.
|
||||
if kind == DeploymentDocker && !c.opts.DeploymentCheckAdvisory {
|
||||
return CheckResult{
|
||||
Status: StatusFail,
|
||||
Detail: "detected deployment kind: docker - this tool cannot cut over a container. " +
|
||||
"Migrating one means pulling the new image and recreating the container, which has to be done by hand; " +
|
||||
"`rehearse` still works and will tell you what the migration involves",
|
||||
}, string(kind)
|
||||
}
|
||||
status := StatusOK
|
||||
if kind == DeploymentUnknown {
|
||||
detail := fmt.Sprintf("detected deployment kind: %s", kind)
|
||||
switch kind {
|
||||
case DeploymentUnknown:
|
||||
status = StatusWarn
|
||||
case DeploymentDocker:
|
||||
// No longer a refusal on its own: a container can be migrated
|
||||
// now. What still refuses is specific and checked below -
|
||||
// compose, and data that is not on a volume - because those are
|
||||
// properties of this container rather than of containers.
|
||||
detail += " - the container checks below decide whether this one can be migrated"
|
||||
}
|
||||
return CheckResult{Status: status, Detail: fmt.Sprintf("detected deployment kind: %s", kind)}, string(kind)
|
||||
return CheckResult{Status: status, Detail: detail}, string(kind)
|
||||
})
|
||||
if err != nil {
|
||||
return report, err
|
||||
|
||||
@@ -475,7 +475,16 @@ func withFakeDocker(t *testing.T) {
|
||||
// install with a fake docker on PATH.
|
||||
func dockerPreflight(t *testing.T, advisory bool) Report {
|
||||
t.Helper()
|
||||
withFakeDocker(t)
|
||||
// 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,
|
||||
// mounted by the fake container.
|
||||
dataDir := t.TempDir()
|
||||
for _, p := range systemdUnitPaths {
|
||||
if _, err := os.Stat(p); err == nil {
|
||||
t.Skipf("host has %s, which detection prefers over docker", p)
|
||||
}
|
||||
}
|
||||
fakeInspect(t, inspectDoc(t, nil, []Mount{dataVolume(dataDir)}))
|
||||
|
||||
counterPath := filepath.Join(t.TempDir(), "invocations")
|
||||
binaryPath := writeFakeBinary(t, "0.15.5", counterPath)
|
||||
@@ -495,7 +504,7 @@ func dockerPreflight(t *testing.T, advisory bool) Report {
|
||||
// only thing that varies: whether stalwart-cli happens to be installed
|
||||
// on the machine running the tests is not what this is testing.
|
||||
report, err := New(Options{
|
||||
BinaryPath: binaryPath, ConfigPath: configPath, DataDir: t.TempDir(),
|
||||
BinaryPath: binaryPath, ConfigPath: configPath, DataDir: dataDir,
|
||||
TargetVersion: "latest", ToolCheckAdvisory: true, DeploymentCheckAdvisory: advisory,
|
||||
}).Run(context.Background(), store, rs)
|
||||
if err != nil {
|
||||
@@ -504,21 +513,25 @@ func dockerPreflight(t *testing.T, advisory bool) Report {
|
||||
return report
|
||||
}
|
||||
|
||||
// A container has to be refused here, in preflight, and not later. Cutover
|
||||
// already refuses it -- but cutover runs after the service has been stopped,
|
||||
// so refusing there refuses with mail down, which turned an attempted
|
||||
// migration into an outage.
|
||||
func TestPreflightBlocksADockerDeployment(t *testing.T) {
|
||||
// Being a container is no longer a refusal on its own - cutover can
|
||||
// recreate one now. What refuses is specific to *this* container: compose
|
||||
// management, and data that is not on a volume. Those live in
|
||||
// container_test.go, and both still block.
|
||||
//
|
||||
// This previously asserted the blanket refusal. It asserts the replacement
|
||||
// rather than being deleted, because "docker is allowed through here" is
|
||||
// the thing that would be wrong to regress.
|
||||
func TestPreflightAllowsAPlainContainerThroughTheKindCheck(t *testing.T) {
|
||||
report := dockerPreflight(t, false)
|
||||
if !report.Blocking() {
|
||||
t.Fatalf("expected a blocking report for a docker deployment, got:\n%s", report.String())
|
||||
if report.Blocking() {
|
||||
t.Fatalf("a plain container on a volume should not be blocked:\n%s", report.String())
|
||||
}
|
||||
var found bool
|
||||
for _, res := range report.Results {
|
||||
if res.Name == "deployment-kind" {
|
||||
found = true
|
||||
if res.Status != StatusFail {
|
||||
t.Errorf("deployment-kind status = %q, want %q", res.Status, StatusFail)
|
||||
if res.Status != StatusOK {
|
||||
t.Errorf("deployment-kind status = %q, want %q", res.Status, StatusOK)
|
||||
}
|
||||
if !strings.Contains(res.Detail, "docker") {
|
||||
t.Errorf("deployment-kind detail does not mention docker: %q", res.Detail)
|
||||
|
||||
@@ -300,9 +300,9 @@ func (c *Checker) runContainerChecks(ctx context.Context, runCheck checkFunc) er
|
||||
return CheckResult{Status: status, Detail: fmt.Sprintf(
|
||||
"data dir %s is not covered by any of the container's mounts (%s), so it lives in the writable layer and would "+
|
||||
"not survive the container being replaced. Check whether --data-dir names the path inside the container",
|
||||
c.opts.DataDir, describeMounts(facts.Mounts))}, ""
|
||||
c.opts.DataDir, DescribeMounts(facts.Mounts))}, ""
|
||||
}
|
||||
return CheckResult{Status: StatusOK, Detail: "container has writable mounts: " + describeMounts(writable)}, ""
|
||||
return CheckResult{Status: StatusOK, Detail: "container has writable mounts: " + DescribeMounts(writable)}, ""
|
||||
})
|
||||
return err
|
||||
}
|
||||
@@ -322,7 +322,8 @@ func mountSource(m Mount) string {
|
||||
return m.Source
|
||||
}
|
||||
|
||||
func describeMounts(mounts []Mount) string {
|
||||
// DescribeMounts renders mounts for an operator-facing message.
|
||||
func DescribeMounts(mounts []Mount) string {
|
||||
if len(mounts) == 0 {
|
||||
return "none"
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user