diff --git a/internal/bootstrap/rewrite.go b/internal/bootstrap/rewrite.go index 2cdcde5..dc040e4 100644 --- a/internal/bootstrap/rewrite.go +++ b/internal/bootstrap/rewrite.go @@ -19,6 +19,28 @@ import ( // broker and no mesh. const ControlPlaneID = "control-plane" +// TempPrefix is what the substrate's control plane is renamed with. +// +// **This is the whole of how a carried resource becomes a declared one.** The substrate raises a +// control plane and a module later declares one, and for a moment both exist — which looked like a +// handover problem needing a way for the host to stop owning something without destroying it. It is +// not one. The temporary control plane is called `temp-mesh-control` and the permanent one is +// called `mesh-control`: two containers, two owners, nothing shared and nothing to hand over. At +// the end the temporary one is dropped from the bundle and the host removes it, which is exactly +// what should happen to something named "temp" (novox/hq ADR 0067). +// +// The name is also the audit. After the pivot, a machine running `mesh-control` and not +// `temp-mesh-control` has completed it; one running both stopped in the middle; one running only +// the temp has not started. That is readable from `docker ps` by somebody who knows nothing else. +const TempPrefix = "temp-" + +// ControlPlaneModule is the module the permanent control plane is installed as, and the name its +// container takes — the name the substrate's own control plane gives up here so that it can. +// +// Declared beside the rename rather than beside the step that uses it, because this is where the +// two names are decided together and where the reason for both of them is written down. +const ControlPlaneModule = "mesh-control" + // brokerAddressVar is what a token tells an enrolling node to dial. // // Not rewritten here — see the note in Rewrite — but reported, because it is the field most likely @@ -44,6 +66,12 @@ type Rewritten struct { // installer produced earlier. Changed bool + // WasCalled is what the template called the control plane's container; TempName is what the + // produced bundle calls it. Renamed is false when the template already used the temporary name. + WasCalled string + TempName string + Renamed bool + // Kept is every other container image, unchanged, as " ". Reported rather than // assumed: "postgres was left alone" is a claim, and this is the evidence for it. Kept []string @@ -56,8 +84,10 @@ type Rewritten struct { // Rewrite produces the bundle this machine will apply from the template it was given. // -// **One substitution, and it is textual.** The control plane's image becomes the id of the image -// this machine now holds; nothing else changes. Textual rather than parse-and-re-serialise because +// **Two substitutions, and both are textual.** The control plane's image becomes the id of the image +// this machine now holds, and its container is renamed `temp-mesh-control`; nothing else changes. +// The rename is what makes the pivot expressible at all — see TempPrefix. Textual rather than +// parse-and-re-serialise because // the produced file has to be *read* — a person getting a machine working must be able to open it, // see the substrate they recognise, and see exactly one thing different. Re-serialising a parsed // declaration would drop every comment in the template, and those comments are where the reasons @@ -118,6 +148,20 @@ func Rewrite(template []byte, imageID string) (Rewritten, error) { out.Changed = true } + // And the container is renamed, for the reason TempPrefix records. Done here rather than + // anywhere later because the bundle is the only place the name is decided: the apply creates + // the container from it, the verify asks that container questions, and the retirement takes + // this same resource back out. One name, one place, read by everything. + out.WasCalled, out.TempName = control.Name, TempPrefix+control.Name + if strings.HasPrefix(control.Name, TempPrefix) { + out.TempName = control.Name + } + renamed, err := renameContainer(out.Bundle, control.Name, out.TempName) + if err != nil { + return Rewritten{}, err + } + out.Bundle, out.Renamed = renamed, out.TempName != control.Name + // Read back, on the bytes that will actually be applied. Everything above is an intention // until the produced file is parsed and asked what it says. after, err := declaration.ParseFileTrusted(out.Bundle) @@ -135,27 +179,68 @@ func Rewrite(template []byte, imageID string) (Rewritten, error) { return Rewritten{}, fmt.Errorf( "the produced bundle still names the control plane %q, not %q", produced.Image, imageID) } + if produced.Name != out.TempName { + return Rewritten{}, fmt.Errorf( + "the produced bundle still calls the control plane's container %q, not %q. The "+ + "permanent one is a module and takes the plain name, so a substrate that kept it "+ + "would put two owners on one container", produced.Name, out.TempName) + } // And nothing else moved. A substitution on text can in principle catch more than it was // aimed at, and "postgres was left exactly as it was" is the claim this checks rather than - // asserts. - was := containerImages(before) + // asserts. Both the image AND the name, because there are now two substitutions. + wasImage, wasName := containerImages(before), containerNames(before) for id, image := range containerImages(after) { if id == ControlPlaneID { continue } - if was[id] != image { + if wasImage[id] != image { return Rewritten{}, fmt.Errorf( "rewriting the control plane's image also changed %q, from %q to %q. Only the "+ "mesh's own image may move; everything else is somebody else's image at "+ - "somebody else's registry", id, was[id], image) + "somebody else's registry", id, wasImage[id], image) } out.Kept = append(out.Kept, fmt.Sprintf("%s %s", id, image)) } + for id, name := range containerNames(after) { + if id == ControlPlaneID || wasName[id] == name { + continue + } + return Rewritten{}, fmt.Errorf( + "renaming the control plane's container also renamed %q, from %q to %q. Only the "+ + "control plane moves out of the way; every other container keeps the name the "+ + "substrate gave it", id, wasName[id], name) + } sortStrings(out.Kept) return out, nil } +// renameContainer changes one container's name in the bundle's text. +// +// **The quoted name, not the bare word.** `mesh-control` also appears inside the image reference +// the template carries (`…/mesh-control@sha256:…`) and could appear inside a command line; a bare +// substitution would catch those too. What is wanted is a JSON string that IS the name, so the +// quotes are part of what is matched — `"mesh-control"` matches the container's `name` and an +// action's `in`, which are exactly the places the name means the container, and nothing else. +// +// It refuses when the text does not contain what the parse says is there, for the same reason the +// image substitution does: the two would then be reading different things, and a rename that +// replaced nothing and reported success would leave the module and the substrate fighting over one +// container three steps later. +func renameContainer(bundle []byte, from, to string) ([]byte, error) { + if from == to { + return bundle, nil + } + quoted := []byte(`"` + from + `"`) + if bytes.Count(bundle, quoted) == 0 { + return nil, fmt.Errorf( + "the control plane's container is called %q according to the parsed template, and %s "+ + "is not in the file. Nothing was renamed, and the substrate would raise a "+ + "container the module also wants", from, quoted) + } + return bytes.ReplaceAll(bundle, quoted, []byte(`"`+to+`"`)), nil +} + // controlPlaneIn finds the container this installer replaces the image of. func controlPlaneIn(d *declaration.Declaration) (*declaration.Container, error) { for _, r := range d.Resources { @@ -186,6 +271,16 @@ func containerImages(d *declaration.Declaration) map[string]string { return images } +func containerNames(d *declaration.Declaration) map[string]string { + names := map[string]string{} + for _, r := range d.Resources { + if container, ok := r.(*declaration.Container); ok { + names[container.ID] = container.Name + } + } + return names +} + func identities(d *declaration.Declaration) []string { var ids []string for _, r := range d.Resources { diff --git a/internal/bootstrap/rewrite_test.go b/internal/bootstrap/rewrite_test.go index 11798ae..7902f1c 100644 --- a/internal/bootstrap/rewrite_test.go +++ b/internal/bootstrap/rewrite_test.go @@ -4,6 +4,8 @@ import ( "os" "strings" "testing" + + "github.com/novox/mesh-host/internal/declaration" ) // Each test names the decision it defends (novox/hq ADR 0017). @@ -221,3 +223,99 @@ func TestTheAddressNodesWillDialIsReportedAndNotRewritten(t *testing.T) { "knows this machine's address", out.BrokerAddress) } } + +// --------------------------------------------------------------------------------------------- +// The rename, which is what makes genesis a pivot rather than a handover (novox/hq ADR 0067). +// --------------------------------------------------------------------------------------------- + +// **This is the test that dissolves the blocker.** The substrate raises a control plane and a +// module later declares one; if both are called `mesh-control` then for one moment two owners hold +// one container, and the host — which tracks what it owns — has no way to stop owning something +// without destroying it. Nothing here invents such a mechanism. The substrate's container is +// called `temp-mesh-control` instead, and there are simply two containers. +func TestTheSubstratesControlPlaneMovesOutOfTheModulesWay(t *testing.T) { + out, err := Rewrite(theRealBundle(t), held) + if err != nil { + t.Fatal(err) + } + if !out.Renamed { + t.Error("the rewrite reported nothing renamed, and the template named it mesh-control") + } + if out.TempName != "temp-mesh-control" { + t.Errorf("the substrate's control plane is called %q", out.TempName) + } + control, err := controlPlaneIn(out.Declaration) + if err != nil { + t.Fatal(err) + } + if control.Name != out.TempName { + t.Errorf("the produced bundle calls it %q, want %q", control.Name, out.TempName) + } + // And the plain name is free, which is the whole point: it belongs to the module now. + for _, name := range containerNames(out.Declaration) { + if name == ControlPlaneModule { + t.Errorf("the produced bundle still declares a container called %q, which the module "+ + "will also declare", ControlPlaneModule) + } + } +} + +// The image reference contains the string `mesh-control` too, and it is not a container name. A +// substitution that caught it would produce `…/temp-mesh-control@sha256:…`, which no registry +// serves — and it would be found inside a pull rather than here. +func TestTheImageReferenceIsNotMistakenForTheContainerName(t *testing.T) { + out, err := Rewrite(theRealBundle(t), held) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(out.Bundle), TempPrefix+"mesh-control@") || + strings.Contains(string(out.Bundle), "/"+TempPrefix+"mesh-control") { + t.Error("the rename reached inside an image reference") + } +} + +// Everything else keeps the name the substrate gave it. The store and the broker are containers +// too, and a rename that moved them would leave a machine whose substrate the host cannot find. +func TestRenamingTheControlPlaneLeavesEveryOtherContainerAlone(t *testing.T) { + before, err := declaration.ParseFileTrusted(theRealBundle(t)) + if err != nil { + t.Fatal(err) + } + out, err := Rewrite(theRealBundle(t), held) + if err != nil { + t.Fatal(err) + } + was, now := containerNames(before), containerNames(out.Declaration) + for id, name := range was { + if id == ControlPlaneID { + continue + } + if now[id] != name { + t.Errorf("%s was renamed from %q to %q", id, name, now[id]) + } + } +} + +// A re-run against a bundle this installer produced renames nothing and says so. The installer is +// run over and over while somebody gets a machine working, and a step that could not tell "already +// done" from "just done" makes the second run indistinguishable from the first. +func TestRewritingABundleThisAlreadyProducedRenamesNothing(t *testing.T) { + first, err := Rewrite(theRealBundle(t), held) + if err != nil { + t.Fatal(err) + } + second, err := Rewrite(first.Bundle, held) + if err != nil { + t.Fatal(err) + } + if second.Renamed { + t.Error("a bundle already naming temp-mesh-control was renamed again") + } + if second.TempName != first.TempName { + t.Errorf("the second pass calls it %q and the first called it %q", + second.TempName, first.TempName) + } + if string(second.Bundle) != string(first.Bundle) { + t.Error("rewriting a produced bundle changed it") + } +} diff --git a/internal/bootstrap/verify_test.go b/internal/bootstrap/verify_test.go index 45eb857..3711d7d 100644 --- a/internal/bootstrap/verify_test.go +++ b/internal/bootstrap/verify_test.go @@ -85,9 +85,11 @@ func TestASubstrateThatIsUpAndAnsweringIsAccepted(t *testing.T) { if err != nil { t.Fatal(err) } - // Three long-running containers: the store, the broker and the control plane. The run-once and - // scheduled shapes are excluded on purpose — a step that has exited is not a fault. - want := []string{"mesh-store", "mesh-broker", "mesh-control"} + // Three long-running containers: the store, the broker and the TEMPORARY control plane. The + // run-once and scheduled shapes are excluded on purpose — a step that has exited is not a + // fault. The name is `temp-mesh-control` because the permanent one is a module and takes the + // plain name (novox/hq ADR 0067), which is what makes the two of them coexist at all. + want := []string{"mesh-store", "mesh-broker", "temp-mesh-control"} if len(verified.Running) != len(want) { t.Fatalf("confirmed %v running, want %v", verified.Running, want) } @@ -162,7 +164,7 @@ func TestTheControlPlaneIsAskedByRunningItsOwnBinary(t *testing.T) { time.Second, 0, func(string) {}); err != nil { t.Fatal(err) } - if !runtime.ran("docker exec mesh-control " + controlPlaneBinary + " status") { + if !runtime.ran("docker exec " + TempPrefix + "mesh-control " + controlPlaneBinary + " status") { t.Errorf("the control plane was never asked anything: %v", runtime.commands) } }