Look at what a container mounts directly, accept a pre-upgrade label, and write the genesis secret without a newline
Review of the first cut found four things. A directory mounted into a container is no longer looked inside, not even for the files this host wrote there. The controller records every provider's received and contributions file as a plain file under a mounted directory, so folding those in would have recreated the route proxy — which re-reads its routes live, by design — on every route change, and killed every provisioner sidecar, which polls what it receives, mid-reconcile on every grant. Whether a service reads a file under its directory once or watches it is the service's; restart-on is how a module says "once", and it stays the opt-in. Env-files and files mounted directly remain by content. Genesis wrote the superuser secret as `value\n`; `secret accept` strips the line ending by design, so the postgres module declared `value` — and with a mounted file's content in the spec, phase three would have recreated the store it meant to adopt in place, with the temporary control plane connected to it. Genesis now writes the value alone. readCredentialFile tolerated both endings already. Pinned with the bytes the genesis code path writes, then the module's declaration of the same container: it must reconcile. A container carrying a label from before the host folded in what it reads is accepted rather than recreated, when that label matches the spec as it used to be computed: what it reads is recorded then, a change is caught from that record from the next apply on, and the label is renewed at the next genuine recreate. Recreating them all would have been a restart storm across the mesh in declaration order, the store first. The trade-off is stated in the code: a container already stale at upgrade time is not caught, and could not have been either way. The record of what a container read is looked up by its name when its declared id has none — the bundle's `store` becomes `postgres.server` for the same container — so a change on the day it is adopted still names the file. The by-target lookup takes the most recently applied record, since the bundle's record for the same target is never removed by the mesh's. novox/hq 04-ISSUES/103
This commit is contained in:
+58
-27
@@ -1167,14 +1167,13 @@ type inputs struct {
|
||||
// — an env-file a predecessor left, the superuser secret genesis writes before any declaration
|
||||
// names it — is read, so the digest is the same one the host records when it later writes the
|
||||
// same bytes there, and adopting a running store in place stays a reconcile rather than a
|
||||
// recreate (bootstrap phase three). Empty when there is nothing readable there: the runtime
|
||||
// refuses an absent env-file itself, with a better message than this could give.
|
||||
// recreate (bootstrap phase three; genesis writes the value with no line ending for exactly this
|
||||
// reason). Empty when there is nothing readable there: the runtime refuses an absent env-file
|
||||
// itself, with a better message than this could give.
|
||||
func (in inputs) fileDigest(path string) string {
|
||||
if in.known != nil {
|
||||
for _, f := range in.known.FilesUnder(path) {
|
||||
if f.Target == path {
|
||||
return f.Wrote
|
||||
}
|
||||
if f, recorded := in.known.At(string(declaration.TypeFile), path); recorded {
|
||||
return f.Wrote
|
||||
}
|
||||
}
|
||||
info, err := os.Stat(path)
|
||||
@@ -1193,12 +1192,16 @@ func (in inputs) fileDigest(path string) string {
|
||||
//
|
||||
// - every env-file: the runtime reads it once, at create, and `docker restart` hands the
|
||||
// container the same environment it had.
|
||||
// - a file bind-mounted into it, by its content — and, for a directory bind-mounted into it,
|
||||
// every file THIS HOST wrote at or beneath the source: the secrets, bindings and configs it
|
||||
// put under the module's state directories. What else is in a mounted directory is the
|
||||
// service's own data, which changes while it runs and is nothing to recreate it for.
|
||||
// - a file bind-mounted into it, DIRECTLY, by its content: a secret, a credential file.
|
||||
//
|
||||
// A step is not here: a run-once or scheduled container reads its files when it runs, and
|
||||
// **A directory bind-mounted into it is not looked inside**, not even for the files this host
|
||||
// wrote there. What a service reads out of a mounted directory, and when, is the service's
|
||||
// business: the route proxy re-reads its routes file live and would be recreated on every route
|
||||
// change; a provisioner sidecar polls what it receives every few seconds and would be killed
|
||||
// mid-reconcile on every grant. A module whose container does read such a file once, at start,
|
||||
// says so with restart-on — that is what the field is for, and it stays the opt-in.
|
||||
//
|
||||
// A step is not here either: a run-once or scheduled container reads its files when it runs, and
|
||||
// runs fresh each time. Only a container that stays running holds what it read.
|
||||
func (in inputs) reads(r *declaration.Container) map[string]string {
|
||||
if r.RunOnce || r.Schedule != "" {
|
||||
@@ -1213,15 +1216,9 @@ func (in inputs) reads(r *declaration.Container) map[string]string {
|
||||
if !strings.HasPrefix(src, "/") {
|
||||
continue // a named volume: the runtime's, holding data
|
||||
}
|
||||
if in.known != nil {
|
||||
for _, f := range in.known.FilesUnder(src) {
|
||||
out[f.Target] = f.Wrote
|
||||
}
|
||||
}
|
||||
if _, recorded := out[src]; !recorded {
|
||||
if digest := in.fileDigest(src); digest != "" {
|
||||
out[src] = digest
|
||||
}
|
||||
// fileDigest is empty for a directory, and for anything else that is not a regular file.
|
||||
if digest := in.fileDigest(src); digest != "" {
|
||||
out[src] = digest
|
||||
}
|
||||
}
|
||||
if len(out) == 0 {
|
||||
@@ -1399,18 +1396,42 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner,
|
||||
// already has.
|
||||
reasons := restartedBy(r.RestartOn, changed)
|
||||
|
||||
// What this container was created reading, as last recorded. By its declared id first, and by
|
||||
// its NAME when that id has no record: the bundle's `store` becomes the postgres module's
|
||||
// `postgres.server`, the same container under a new id, and a file change on the day it is
|
||||
// adopted is a real change with a real record — under the old id.
|
||||
wasReading := previous.Reads
|
||||
if previous.ID == "" && in.known != nil {
|
||||
if byName, ok := in.known.At(string(declaration.TypeContainer), r.Name); ok {
|
||||
wasReading = byName.Reads
|
||||
}
|
||||
}
|
||||
|
||||
// Which of the files it reads no longer hold what it was created reading. The spec label says
|
||||
// only that SOMETHING moved; the record of what was read says what — and that is the line a
|
||||
// person needs when a service went stale without a word (novox/hq 04-ISSUES/103).
|
||||
var changedFiles []string
|
||||
for _, path := range sortedKeys(previous.Reads) {
|
||||
if now, still := reads[path]; still && now != previous.Reads[path] {
|
||||
for _, path := range sortedKeys(wasReading) {
|
||||
if now, still := reads[path]; still && now != wasReading[path] {
|
||||
changedFiles = append(changedFiles, path)
|
||||
}
|
||||
}
|
||||
|
||||
// **A container labelled before the host folded in what it reads is accepted, not recreated.**
|
||||
//
|
||||
// Its label is the spec without the file lines. Recreating every such container on the first
|
||||
// apply after the host upgraded would be a restart storm across the mesh in declaration order —
|
||||
// the store first, under everything that uses it. So a label that matches the spec as it used
|
||||
// to be computed is taken as current: what it reads is recorded now, and from the next apply
|
||||
// on a changed file is caught by that record; the label itself is rewritten at the next
|
||||
// genuine recreate. The trade-off, stated: a container that was ALREADY stale when the host
|
||||
// upgraded — created against a file that has since changed — is not caught by this, and could
|
||||
// not be by the alternative either, which recreates it without knowing whether it needed to.
|
||||
legacy := len(reads) > 0 && before.Spec == containerSpecReading(r, in.declares, nil) &&
|
||||
(wasReading == nil || sameReads(wasReading, reads))
|
||||
|
||||
switch {
|
||||
case existed && before.Spec == want && before.Running && len(reasons) == 0:
|
||||
case existed && (before.Spec == want || legacy) && before.Running && len(reasons) == 0:
|
||||
out.Action = "unchanged"
|
||||
return out, nil
|
||||
case existed:
|
||||
@@ -1478,10 +1499,6 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner,
|
||||
}
|
||||
case len(reasons) > 0:
|
||||
out.Detail = "recreated to pick up " + strings.Join(reasons, ", ")
|
||||
case previous.Reads == nil && len(reads) > 0:
|
||||
// A container made before this host kept what it read: whether the files it holds
|
||||
// are the ones on disk cannot be known, so it is recreated once and from now on can.
|
||||
out.Detail = "recreated: what it reads was not on record, so what it holds could not be checked"
|
||||
default:
|
||||
out.Detail = "replaced; a container's configuration is fixed when it is created"
|
||||
}
|
||||
@@ -1650,6 +1667,20 @@ func restartedBy(restartOn []string, changed map[string]bool) []string {
|
||||
return which
|
||||
}
|
||||
|
||||
// sameReads is whether two records of what a container reads name the same files holding the
|
||||
// same content — a file added, dropped or changed makes them differ.
|
||||
func sameReads(a, b map[string]string) bool {
|
||||
if len(a) != len(b) {
|
||||
return false
|
||||
}
|
||||
for path, digest := range a {
|
||||
if b[path] != digest {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func sortedKeys(m map[string]string) []string {
|
||||
keys := make([]string, 0, len(m))
|
||||
for k := range m {
|
||||
|
||||
+110
-15
@@ -132,24 +132,27 @@ func TestAContainerIsRecreatedWhenAMountedSecretChanged(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestWhatAServiceWritesInAMountedDirectoryIsNotPartOfWhatItIs(t *testing.T) {
|
||||
// A bind-mounted directory is the service's data: it changes while the service runs, and
|
||||
// recreating for it would restart a database for every row it wrote. What the HOST wrote
|
||||
// under that directory — a config it rendered there — is another matter: the service read
|
||||
// that once, at start.
|
||||
func TestAMountedDirectoryIsNotLookedInside(t *testing.T) {
|
||||
// A bind-mounted directory is not part of what a container is — not the data the service
|
||||
// grows in it, and not the files the host itself writes there either. Whether a service reads
|
||||
// a file under its directory once at start or watches it live is the service's business: the
|
||||
// route proxy re-reads its routes live, a provisioner sidecar polls what it receives every few
|
||||
// seconds, and recreating either for a file the host rewrote would kill them for nothing. A
|
||||
// module whose container does read such a file once says so with restart-on, which stays the
|
||||
// opt-in.
|
||||
dir := t.TempDir()
|
||||
state := filepath.Join(dir, "state")
|
||||
config := filepath.Join(state, "config.toml")
|
||||
declare := func(level string) *declaration.Declaration {
|
||||
declare := func(level, restartOn string) *declaration.Declaration {
|
||||
return parseTrusted(t, `{"declaration":1,"resources":[
|
||||
{"id":"app.state","type":"directory","path":"`+state+`"},
|
||||
{"id":"app.config","type":"file","path":"`+config+`","content":"level = \"`+level+`\"\n"},
|
||||
{"id":"app.server","type":"container","name":"app","image":"`+pinned+`",
|
||||
"volumes":["`+state+`:/var/lib/app"]}
|
||||
"volumes":["`+state+`:/var/lib/app"]`+restartOn+`}
|
||||
]}`)
|
||||
}
|
||||
m := &machine{containers: map[string]*fakeContainer{}}
|
||||
_, known := applyCarried(t, declare("info"), store.State{}, m, nil)
|
||||
_, known := applyCarried(t, declare("info", ""), store.State{}, m, nil)
|
||||
|
||||
// The service grows its data in the directory it was given.
|
||||
if err := os.WriteFile(filepath.Join(state, "app.db"), []byte("rows"), 0o600); err != nil {
|
||||
@@ -162,20 +165,112 @@ func TestWhatAServiceWritesInAMountedDirectoryIsNotPartOfWhatItIs(t *testing.T)
|
||||
t.Fatal(err)
|
||||
}
|
||||
m.asked = nil
|
||||
report, known := applyCarried(t, declare("info"), known, m, nil)
|
||||
report, known := applyCarried(t, declare("info", ""), known, m, nil)
|
||||
if m.did("docker rm") || m.did("docker run") || report.Changed() {
|
||||
t.Errorf("a container was recreated for data its service wrote in a mounted directory: %v %+v",
|
||||
m.asked, report.Outcomes)
|
||||
}
|
||||
|
||||
// The host's own config under the same directory changes: that, the service read at start.
|
||||
// The host rewrites its own file under the same directory: still not a reason. The container
|
||||
// did not name it.
|
||||
m.asked = nil
|
||||
report, _ = applyCarried(t, declare("debug"), known, m, nil)
|
||||
if !m.removed("app") || !m.did("docker run") {
|
||||
t.Fatalf("a config the host wrote under a mounted directory changed and the container was not recreated: %v", m.asked)
|
||||
report, known = applyCarried(t, declare("debug", ""), known, m, nil)
|
||||
if m.did("docker rm") || m.did("docker run") {
|
||||
t.Errorf("a container was recreated for a file under a mounted directory it did not name: %v", m.asked)
|
||||
}
|
||||
if o := outcomeOf(report, "app.server"); o.Detail != "recreated: "+config+" changed" {
|
||||
t.Errorf("the recreate did not name the config: %+v", o)
|
||||
if o := outcomeOf(report, "app.config"); o.Action != "updated" {
|
||||
t.Fatalf("the config was not rewritten: %+v", o)
|
||||
}
|
||||
|
||||
// Naming it is what makes it a reason, as before this change.
|
||||
m.asked = nil
|
||||
report, _ = applyCarried(t, declare("trace", `,"restart-on":["app.config"]`), known, m, nil)
|
||||
if !m.removed("app") || !m.did("docker run") {
|
||||
t.Fatalf("a container naming a rewritten file under its mount was not recreated: %v", m.asked)
|
||||
}
|
||||
if o := outcomeOf(report, "app.server"); !strings.Contains(o.Detail, "app.config") {
|
||||
t.Errorf("the recreate did not name why: %+v", o)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAContainerLabelledBeforeTheHostReadItsFilesIsAcceptedNotRecreated(t *testing.T) {
|
||||
// The first apply after the host upgrades finds every container carrying a label computed
|
||||
// without the file lines. Recreating them all would be a restart storm across the mesh in
|
||||
// declaration order, the store first. A label that matches the spec as it used to be computed
|
||||
// is accepted: what the container reads is recorded now, and from then on a change is caught.
|
||||
dir := t.TempDir()
|
||||
env := filepath.Join(dir, "forge.env")
|
||||
declare := func(port string) *declaration.Declaration {
|
||||
return parseTrusted(t, `{"declaration":1,"resources":[
|
||||
{"id":"forge.env","type":"file","path":"`+env+`","content":"DATABASE_PORT=`+port+`\n","mode":"0600"},
|
||||
{"id":"forge.server","type":"container","name":"forge","image":"`+pinned+`","env-file":["`+env+`"]}
|
||||
]}`)
|
||||
}
|
||||
// The machine as the previous host left it: the file written and recorded, the container up
|
||||
// under the label that host computed — the spec with nothing about the file's content.
|
||||
if err := os.WriteFile(env, []byte("DATABASE_PORT=5432\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
d := declare("5432")
|
||||
legacy := containerSpecReading(d.Resources[1].(*declaration.Container), nil, nil)
|
||||
known := store.State{}
|
||||
known.Record(store.Applied{ID: "forge.env", Type: "file", Origin: store.OriginCarried, Target: env,
|
||||
Wrote: digestOf("DATABASE_PORT=5432\n")})
|
||||
known.Record(store.Applied{ID: "forge.server", Type: "container", Origin: store.OriginCarried, Target: "forge"})
|
||||
m := &machine{containers: map[string]*fakeContainer{"forge": {id: "made-by-host", running: true, spec: legacy}}}
|
||||
|
||||
report, known := applyCarried(t, d, known, m, nil)
|
||||
if m.did("docker rm") || m.did("docker run") || report.Changed() {
|
||||
t.Fatalf("a container labelled by the previous host was recreated on upgrade: %v %+v", m.asked, report.Outcomes)
|
||||
}
|
||||
if got, _ := known.Find("forge.server"); got.Reads[env] != digestOf("DATABASE_PORT=5432\n") {
|
||||
t.Fatalf("what the accepted container reads was not recorded: %+v", got)
|
||||
}
|
||||
// Accepted stays accepted: the next pass with nothing moved is quiet too.
|
||||
m.asked = nil
|
||||
report, known = applyCarried(t, d, known, m, nil)
|
||||
if m.did("docker rm") || m.did("docker run") || report.Changed() {
|
||||
t.Fatalf("an accepted container was recreated on the pass after: %v", m.asked)
|
||||
}
|
||||
|
||||
// And a change to the file is caught from the record, and the label is renewed.
|
||||
m.asked = nil
|
||||
report, _ = applyCarried(t, declare("5433"), known, m, nil)
|
||||
if !m.removed("forge") || !m.did("docker run") {
|
||||
t.Fatalf("an accepted container was not recreated when its env-file changed: %v", m.asked)
|
||||
}
|
||||
if o := outcomeOf(report, "forge.server"); o.Detail != "recreated: "+env+" changed" {
|
||||
t.Errorf("the recreate did not name the file: %+v", o)
|
||||
}
|
||||
if m.containers["forge"].spec == legacy {
|
||||
t.Error("the recreated container still carries the legacy label")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAContainerAdoptedUnderANewIdStillSaysWhichFileChanged(t *testing.T) {
|
||||
// The bundle's `store` becomes the postgres module's `postgres.server`: the same container by
|
||||
// name, under a new id with no record of its own. A file change on that day is a real change,
|
||||
// and the record of what it read is under the old id — by name, it is found.
|
||||
dir := t.TempDir()
|
||||
env := filepath.Join(dir, "store.env")
|
||||
m := &machine{containers: map[string]*fakeContainer{}}
|
||||
raised := parseTrusted(t, `{"declaration":1,"resources":[
|
||||
{"id":"env","type":"file","path":"`+env+`","content":"PORT=5432\n","mode":"0600"},
|
||||
{"id":"store","type":"container","name":"mesh-store","image":"`+pinned+`","env-file":["`+env+`"]}
|
||||
]}`)
|
||||
_, known := applyCarried(t, raised, store.State{}, m, nil)
|
||||
|
||||
adopted := parseTrusted(t, `{"declaration":1,"resources":[
|
||||
{"id":"postgres.env","type":"file","path":"`+env+`","content":"PORT=5433\n","mode":"0600"},
|
||||
{"id":"postgres.server","type":"container","name":"mesh-store","image":"`+pinned+`","env-file":["`+env+`"]}
|
||||
]}`)
|
||||
m.asked = nil
|
||||
report, _, err := Apply(context.Background(), archHost(t), adopted, known, store.OriginDeclared, m.run, nil, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if o := outcomeOf(report, "postgres.server"); o.Action != "updated" || o.Detail != "recreated: "+env+" changed" {
|
||||
t.Errorf("a container adopted under a new id did not say which file changed: %+v", o)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user