diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index f6198f7..c1d98a8 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -618,17 +618,15 @@ func (r Resolution) compose(with Rendering, owner map[string]string) ([]map[stri // merging earlier would throw away the files it still needs. resources = append(append([]map[string]any{}, first...), resources...) - // Every container is given the mesh's names. Not a choice a module makes: a module that - // listed them would go stale the day a machine joins, and one that did not would be a - // module whose containers cannot reach anything by name. - // - // A container that was given names of its own keeps them and gets the mesh's beside them: - // the mesh does not know what else a workload needs to reach, and taking something away - // to add something is not what "also" means. - if len(with.Names) > 0 { - resources = withMeshNames(resources, with.Names) - } - + // No container is given the mesh's names (novox/hq ADR 0148). It used to be: every + // container got the whole roster as `--add-host` entries at creation, and a name that + // moved afterwards was wrong inside it for as long as it ran (issues 109, 135) — and once + // the roster was made part of a container's identity so that could be caught, one name + // moving anywhere replaced every container in the mesh (issue 151). A container resolves a + // mesh name through its machine's resolver at the moment it asks, which the runtime is + // told once per machine, as a file, by the resolver's own module. The names a module + // declares for itself are its own and stay exactly as written: they are part of what the + // module is, and the mesh does not know what they mean. // What this module may name from inside one of its own files. Gathered once per module // rather than per file, because it is a fact about the module. sealed, err := sealedFor(m, r.Needs, with) @@ -1482,43 +1480,6 @@ func (r Resolution) servedOnThisMachine(provision string, with Rendering) (map[s return nil, false, nil } -// withMeshNames gives every container in a set the mesh's names. -// -// Copied rather than edited in place: these maps come from a module's manifest, and mutating one -// would change what the catalogue holds for every other machine running that module. -// -// A host-network container gets the names too. It was once skipped, on the belief that it "shares -// the machine's hosts file already" — but it does not: `docker run --network host` still gives the -// container its own /etc/hosts (localhost and its own id only), so every `.internal` name the -// mesh wrote for the machine is invisible inside it, and a client that dials one gets EAI_AGAIN. The -// remedy is the same `--add-host` every other container gets — the runtime accepts it with -// `--network host` (verified), and without it a host-network consumer cannot reach a provider by the -// `.internal` address the mesh hands it as `${bound:...:at}`. -func withMeshNames(resources []map[string]any, names map[string]string) []map[string]any { - out := make([]map[string]any, 0, len(resources)) - for _, r := range resources { - if r["type"] != "container" { - out = append(out, r) - continue - } - - copied := map[string]any{} - for k, v := range r { - copied[k] = v - } - var given []any - if already, ok := copied["hosts"].([]any); ok { - given = append(given, already...) - } - for _, name := range sortedKeys(names) { - given = append(given, name+":"+names[name]) - } - copied["hosts"] = given - out = append(out, copied) - } - return out -} - // pinned refuses an image that is not really pinned, on its way to a machine. // // **Here and not at parse** (novox/hq 04-ISSUES/025). A manifest in a repository names artifacts diff --git a/internal/catalogue/names_test.go b/internal/catalogue/names_test.go index 73e24b5..846d9e9 100644 --- a/internal/catalogue/names_test.go +++ b/internal/catalogue/names_test.go @@ -1,6 +1,7 @@ package catalogue import ( + "reflect" "strings" "testing" ) @@ -30,36 +31,38 @@ func namesOf(r map[string]any) []string { return out } -// A container does not inherit the machine's names, so the mesh gives them to it. +// No mesh name is written into a container (novox/hq ADR 0148). It resolves them through its +// machine's resolver at the moment it asks, so a name that moves is answered differently by the +// next lookup, in every container, with nothing recreated. // -// It gets its own hosts file holding only its own hostname — every internal name the mesh wrote -// for the machine is invisible to what the machine runs. A database client on one node could not -// resolve another node, on a mesh where both names were correct and present on both machines. -func TestEveryContainerIsGivenTheMeshsNames(t *testing.T) { - got := containersOf(t, Resolution{Node: "laptop", Modules: []Manifest{{ - Module: "app", - Resources: []map[string]any{{"id": "web", "type": "container", "name": "web", - "image": "registry.example/web@sha256:" + strings.Repeat("a", 64)}}, - }}}, Rendering{Names: map[string]string{ - "anchor.internal": "10.42.0.1", "laptop.internal": "10.42.0.2", - }}) - if len(got) != 1 { - t.Fatalf("expected one container, got %d", len(got)) +// Checked the way the record says: the declaration a container gets does not move when the mesh's +// roster does. A roster with one machine and a roster with three produce the same container, byte +// for byte, so the digest a host computes from it cannot move either — which is what stopped one +// name moving from replacing every container in the mesh (issue 151). +func TestAContainerIsTheSameWhateverTheMeshsRosterSays(t *testing.T) { + module := Manifest{Module: "app", Resources: []map[string]any{{"id": "web", "type": "container", + "name": "web", "image": "registry.example/web@sha256:" + strings.Repeat("a", 64)}}} + one := containersOf(t, Resolution{Node: "laptop", Modules: []Manifest{module}}, + Rendering{Names: map[string]string{"laptop.internal": "10.42.0.2"}}) + three := containersOf(t, Resolution{Node: "laptop", Modules: []Manifest{module}}, + Rendering{Names: map[string]string{ + "anchor.internal": "10.42.0.1", "laptop.internal": "10.42.0.2", "git.example.tld": "10.42.0.1", + }}) + if len(one) != 1 || len(three) != 1 { + t.Fatalf("expected one container each, got %d and %d", len(one), len(three)) } - given := namesOf(got[0]) - if len(given) != 2 { - t.Fatalf("the container was given %d name(s): %v", len(given), given) + if given := namesOf(three[0]); len(given) != 0 { + t.Fatalf("the mesh's names were copied into the container: %v", given) } - if given[0] != "anchor.internal:10.42.0.1" { - t.Fatalf("the name is not in the form a runtime writes: %v", given) + if !reflect.DeepEqual(one[0], three[0]) { + t.Fatalf("the container moved with the roster:\n%v\n%v", one[0], three[0]) } } -// A container that named its own keeps them and gets the mesh's beside them. -// -// The mesh does not know what else a workload needs to reach, and taking something away in order -// to add something is not what "also" means. -func TestAContainersOwnNamesAreKept(t *testing.T) { +// The names a module declares for itself are its own: part of what the module is, kept exactly as +// written, and the mesh does not know what they mean. They are the one thing in a container's +// hosts that does move its identity, because they do not move when the mesh's roster does. +func TestAContainersOwnNamesAreKeptAsWritten(t *testing.T) { got := containersOf(t, Resolution{Node: "laptop", Modules: []Manifest{{ Module: "app", Resources: []map[string]any{{"id": "web", "type": "container", "name": "web", @@ -68,62 +71,15 @@ func TestAContainersOwnNamesAreKept(t *testing.T) { }}}, Rendering{Names: map[string]string{"anchor.internal": "10.42.0.1"}}) given := namesOf(got[0]) - if len(given) != 2 || given[0] != "something.else:203.0.113.9" { - t.Fatalf("the container's own names were lost: %v", given) + if len(given) != 1 || given[0] != "something.else:203.0.113.9" { + t.Fatalf("the container's own names were not kept as written: %v", given) } } -// A container on the machine's own network gets the names too — it does NOT share the machine's -// hosts file. `docker run --network host` still gives the container its own /etc/hosts (localhost -// and its own id only), so every `.internal` name the mesh wrote is invisible inside it, and a -// client that dials one gets EAI_AGAIN. It gets the same `--add-host` entries every other container -// gets (the runtime accepts them with `--network host`), so a host-network consumer can reach a -// provider by the `.internal` address the mesh hands it. -func TestAContainerOnTheMachinesNetworkIsGivenTheNamesToo(t *testing.T) { - got := containersOf(t, Resolution{Node: "anchor", Modules: []Manifest{{ - Module: "control", - Resources: []map[string]any{{"id": "c", "type": "container", "name": "c", - "image": "registry.example/c@sha256:" + strings.Repeat("a", 64), "network": "host"}}, - }}}, Rendering{Names: map[string]string{"anchor.internal": "10.42.0.1"}}) - - given := namesOf(got[0]) - if len(given) != 1 || given[0] != "anchor.internal:10.42.0.1" { - t.Fatalf("a host-networked container was not given the mesh's names: %v", got[0]) - } -} - -// A mesh with no private network gives nothing, rather than a name with no address behind it. -func TestAMeshWithNoNamesGivesNone(t *testing.T) { - got := containersOf(t, Resolution{Node: "alone", Modules: []Manifest{{ - Module: "app", - Resources: []map[string]any{{"id": "web", "type": "container", "name": "web", - "image": "registry.example/web@sha256:" + strings.Repeat("a", 64)}}, - }}}, Rendering{}) - if len(namesOf(got[0])) != 0 { - t.Fatalf("names were invented for a mesh that has none: %v", got[0]) - } -} - -// The catalogue's copy is not edited: these maps come from a manifest, and mutating one would -// change what every other machine running that module is given. -func TestGivingNamesDoesNotChangeTheCatalogue(t *testing.T) { - held := map[string]any{"id": "web", "type": "container", "name": "web", - "image": "registry.example/web@sha256:" + strings.Repeat("a", 64)} - module := Manifest{Module: "app", Resources: []map[string]any{held}} - - for _, node := range []string{"one", "two"} { - containersOf(t, Resolution{Node: node, Modules: []Manifest{module}}, - Rendering{Names: map[string]string{"anchor.internal": "10.42.0.1"}}) - } - if _, changed := held["hosts"]; changed { - t.Fatal("the manifest the catalogue holds was edited, so every machine now carries this") - } -} - -// Only containers. A file or a service given a `hosts` key is a declaration the host refuses -// outright — it takes no unknown field — so getting this wrong breaks the whole machine rather -// than one resource, and breaks it for something that was never about names. -func TestNothingButAContainerIsGivenNames(t *testing.T) { +// No resource is given a `hosts` key it did not declare. A file or a service carrying one is a +// declaration the host refuses outright — it takes no unknown field — so an invented key breaks +// the whole machine rather than one resource. +func TestNothingIsGivenNamesItDidNotDeclare(t *testing.T) { out, err := Resolution{Node: "laptop", Modules: []Manifest{{ Module: "app", Resources: []map[string]any{ @@ -137,11 +93,8 @@ func TestNothingButAContainerIsGivenNames(t *testing.T) { t.Fatal(err) } for _, r := range out { - if r["type"] == "container" { - continue - } if _, given := r["hosts"]; given { - t.Fatalf("a %v was given names, which the host will refuse: %v", r["type"], r) + t.Fatalf("a %v was given names it never declared: %v", r["type"], r) } } } diff --git a/internal/catalogue/port_into.go b/internal/catalogue/port_into.go index 8cc5866..f0066d0 100644 --- a/internal/catalogue/port_into.go +++ b/internal/catalogue/port_into.go @@ -98,8 +98,7 @@ func portInto(resource map[string]any, module string, listens []Listening, with // **A fresh map, and only when something changes.** This map came out of the module's // manifest and the resource around it is a shallow copy, so filling a value in place would - // change what the catalogue holds for every other machine running the module — the trap - // withMeshNames is written to avoid, one field along. + // change what the catalogue holds for every other machine running the module. var filled map[string]any for _, key := range named { written, ok := env[key].(string) diff --git a/internal/catalogue/resolver_manifests_test.go b/internal/catalogue/resolver_manifests_test.go index 0cce6cd..e6daf22 100644 --- a/internal/catalogue/resolver_manifests_test.go +++ b/internal/catalogue/resolver_manifests_test.go @@ -48,7 +48,7 @@ func TestTheResolverForwardsToFixedUpstreamsAndNeverReadsResolvConf(t *testing.T } for _, want := range []string{ "\nno-resolv\n", "\nserver=1.1.1.1\n", "\nserver=8.8.8.8\n", - "\nlisten-address=127.0.0.1\n", "\ninterface=mesh0\n", "\nbind-dynamic\n", + "\nlisten-address=127.0.0.1\n", "\nlisten-address=${machine:address}\n", "\nbind-dynamic\n", "\ndomain-needed\n", "\nbogus-priv\n", "\nconf-file=" + m.Facts["node-zones"].Path + "\n", } { @@ -56,6 +56,14 @@ func TestTheResolverForwardsToFixedUpstreamsAndNeverReadsResolvConf(t *testing.T t.Errorf("the resolver's configuration lacks %q:\n%s", strings.TrimSpace(want), config) } } + // By address and never by interface: dnsmasq admits a query by the interface it arrives on + // when told one, and a container's query to the private address arrives on the runtime's + // bridge — `interface=mesh0` dropped every such query, silently (novox/hq issue 110). + for _, line := range strings.Split(config, "\n") { + if strings.HasPrefix(line, "interface=") { + t.Errorf("the resolver answers by interface, so a container's query on a bridge is dropped: %s", line) + } + } // Not .53 or .54, which systemd-resolved holds; and not .55 any more, which was a convention // beside the one every machine already followed — the predecessor's resolv.conf says .1. for _, taken := range []string{"127.0.0.53", "127.0.0.54", "127.0.0.55"} { @@ -137,23 +145,39 @@ func TestTheResolverAndWhatAsksItComposeOnOneMachine(t *testing.T) { t.Errorf("the daemon does not restart on its configuration and the machines file both: %v", service["restart-on"]) } - // The runtime's own file, written into (novox/hq ADR 0102) with the one key this module states. + // The runtime's own file, written into (novox/hq ADR 0102) with the keys this module states: + // where containers resolve, and that a restart keeps them running — because the runtime reads + // `dns` only when it starts, and the one restart that needs is the operator's (issue 110). runtime := ids["dnsmasq.runtime-dns"] if runtime == nil || runtime["path"] != "/etc/docker/daemon.json" || runtime["into"] != "json" { t.Fatalf("the runtime's dns is not written into its file: %v", runtime) } - var keys map[string][]string + var keys map[string]any if err := json.Unmarshal([]byte(runtime["content"].(string)), &keys); err != nil { t.Fatalf("the runtime's keys are not JSON: %v", err) } - if len(keys) != 1 || len(keys["dns"]) != 1 || keys["dns"][0] != "10.42.0.1" { - t.Errorf("the runtime is pointed at %v; containers resolve at this machine's own private-network address, and nothing else is written", keys) + dns, _ := keys["dns"].([]any) + if len(keys) != 2 || len(dns) != 1 || dns[0] != "10.42.0.1" || keys["live-restore"] != true { + t.Errorf("the runtime is given %v; containers resolve at this machine's own private-network address, a restart keeps them, and nothing else is written", keys) } + // The runtime is reloaded when that file changes, and never restarted: a restart stops every + // container on the machine (ADR 0102), and a reload is what turns live-restore on. + var reloaded bool for _, r := range out { - if r["type"] == "service" && r["unit"] == "docker.service" && r["id"] != "" && - strings.HasPrefix(r["id"].(string), "dnsmasq.") { - t.Errorf("the resolver orders the runtime restarted or reloaded, which stops every container (ADR 0102) or does nothing for dns: %v", r) + if r["type"] != "service" || r["unit"] != "docker.service" { + continue } + if _, restarts := r["restart-on"]; restarts { + t.Errorf("the resolver orders the runtime restarted, which stops every container (ADR 0102): %v", r) + } + for _, on := range asStrings(r["reload-on"]) { + if on == "dnsmasq.runtime-dns" { + reloaded = true + } + } + } + if !reloaded { + t.Errorf("the runtime is not reloaded when its file changes, so live-restore never takes effect") } resolv := ids["resolv-conf.resolv"] diff --git a/internal/catalogue/routing_test.go b/internal/catalogue/routing_test.go index d813713..e8cc74f 100644 --- a/internal/catalogue/routing_test.go +++ b/internal/catalogue/routing_test.go @@ -198,37 +198,33 @@ func TestTheApexLabelComposesToTheBarePrivateAddress(t *testing.T) { } } +// novox/hq ADR 0066 propagate, by ADR 0148's means: a granted route name is published into the +// machine's roster mapped to the node that serves it, beside the `.internal` names, and the +// machine's resolver answers it to every container. Nothing is written into the container itself — +// a routed name that moved would otherwise be wrong inside every container until each was recreated. func TestARoutedNameResolvesToTheServingNode(t *testing.T) { - // novox/hq ADR 0066 propagate: a granted route name is published into internal resolution, - // mapped to the node that serves it, alongside the `.internal` names — so every - // container, and an in-mesh ACME validator, resolves a routed name to the proxy that serves it. - // The names map is what withMeshNames writes into every container as `--add-host`; a route name - // mapped to the serving node's address rides the same mechanism. + names := map[string]string{ + "anchor.internal": "10.42.0.1", + "git.example.tld": "10.42.0.1", + } got := containersOf(t, Resolution{Node: "anchor", Modules: []Manifest{{ Module: "app", Resources: []map[string]any{{"id": "web", "type": "container", "name": "web", "image": "registry.example/web@sha256:" + strings.Repeat("a", 64)}}, - }}}, Rendering{Names: map[string]string{ - "anchor.internal": "10.42.0.1", - "git.example.tld": "10.42.0.1", - }}) + }}}, Rendering{Names: names}) if len(got) != 1 { t.Fatalf("expected one container, got %d", len(got)) } - given := namesOf(got[0]) - var sawNode, sawRoute bool - for _, h := range given { - if h == "anchor.internal:10.42.0.1" { - sawNode = true - } - if h == "git.example.tld:10.42.0.1" { + if given := namesOf(got[0]); len(given) != 0 { + t.Fatalf("the routed name was copied into the container, where it would go stale: %v", given) + } + var sawRoute bool + for _, e := range entriesFrom(names, nil, "internal") { + if e.Name == "git.example.tld" && e.Address == "10.42.0.1" { sawRoute = true } } - if !sawNode { - t.Fatalf("the container lost the mesh's node names: %v", given) - } if !sawRoute { - t.Fatalf("the routed name was not published to the serving node: %v", given) + t.Fatalf("the routed name is not in the roster the machine's resolver answers from") } }