diff --git a/modules/mesh-provisioner-client/README.md b/modules/mesh-provisioner-client/README.md index c41c5166..1e68dc03 100644 --- a/modules/mesh-provisioner-client/README.md +++ b/modules/mesh-provisioner-client/README.md @@ -43,8 +43,9 @@ and the placement values its templates name — the client: `${environment:posix}` is `environment:posix`, `${contribution:node-hotkeys:trigger}` is `contribution:node-hotkeys:trigger`. A collector naming a collection its plan does not fails. A collection with no item is filled empty, and filled again whenever an item of it is recorded or goes; -- fills a module's files only once every value is there; otherwise it writes nothing and reports `waiting` with what - it waits for; +- fills a file only once every value of it is there, file by file (issue 1553): a file with a value that waits stays + as it is, the others are filled, and it reports `waiting` with every value it waits for and the files it filled; + `applied` only when every file is filled (a collection's items are recorded only when nothing of the module waits); - has the node-engine write each changed file through its local write socket (`/run/nox-mesh-host/fill.sock`, mesh-host `internal/filling`), which writes it as the declaration says (an `owned` file whole, a `seed` only when absent, a `managed` file's own keys or block) and restarts or reloads its program as the declaration says; diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go index a2a5cd19..52ea5bd4 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go @@ -639,27 +639,32 @@ func (c *Client) reconcile(ctx context.Context, kvs map[string]jetstream.KeyValu c.report(ctx, kvs, s, r) return } - if len(r.WaitsFor) > 0 { - // Every value or none: the files there are stay as they are. - r.Status = StatusWaiting - c.report(ctx, kvs, s, r) - return - } - // The items first: recorded, they fill the collectors, this module's own among them. - for _, it := range items { - if err := c.record(ctx, kvs, s, module, p, it.PlannedItem, render(it.text, values)); err != nil { - failed = append(failed, fmt.Sprintf("item %s: %v", it.key(), err)) - continue + waiting := len(r.WaitsFor) > 0 + // The items keep the rule of every value or none (issue 1553): a collection is recorded only once nothing of the + // module waits, so a collector never reads an item made while another value was missing. + if !waiting { + // The items first: recorded, they fill the collectors, this module's own among them. + for _, it := range items { + if err := c.record(ctx, kvs, s, module, p, it.PlannedItem, render(it.text, values)); err != nil { + failed = append(failed, fmt.Sprintf("item %s: %v", it.key(), err)) + continue + } + r.Items = append(r.Items, it.key()) } - r.Items = append(r.Items, it.key()) } - keep := map[string]bool{} - for _, it := range p.Items { - keep[it.key()] = true + if !waiting { + keep := map[string]bool{} + for _, it := range p.Items { + keep[it.key()] = true + } + c.forgetItems(ctx, kvs, s, module, keep) } - c.forgetItems(ctx, kvs, s, module, keep) moved := false for _, f := range targets { + // File by file (issue 1553, ADR 0349 clause 4): a target with a value that waits keeps the file there is. + if !allKnown(f.text, values) { + continue + } content := []byte(render(byFile[f.Path], values)) fa := FileApplied{Path: f.Path, Takes: f.Takes} a, err := c.Fill.Fill(ctx, f.Path, content) @@ -677,9 +682,12 @@ func (c *Client) reconcile(ctx context.Context, kvs map[string]jetstream.KeyValu } r.Files = append(r.Files, fa) } - if len(failed) > 0 { + switch { + case len(failed) > 0: r.Status, r.Reason = StatusFailed, strings.Join(failed, "; ") - } else { + case waiting: + r.Status = StatusWaiting + default: r.Status = StatusApplied } if moved { @@ -689,6 +697,16 @@ func (c *Client) reconcile(ctx context.Context, kvs map[string]jetstream.KeyValu c.report(ctx, kvs, s, r) } +// allKnown says whether every value a text needs is known (issue 1553). +func allKnown(text string, values map[Need]string) bool { + for _, n := range needsOf(text) { + if _, ok := values[n]; !ok { + return false + } + } + return true +} + // valueOf is a need's value, or what it waits for, or why it cannot be filled. func (c *Client) valueOf(ctx context.Context, s *state, module string, p Plan, n Need) (value, wait, fail string) { answer, known := s.registry[n.Kind] diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/secrets_test.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/secrets_test.go index 073df9a1..1538edb5 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/secrets_test.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/secrets_test.go @@ -253,9 +253,8 @@ func TestNatsASecretTheVaultDoesNotHoldYetIsWaitedFor(t *testing.T) { if _, asked := e.counts(appConf); asked != 0 { t.Error("a file waiting for its secret was written") } - if _, asked := e.counts(appDB); asked != 0 { - t.Error("a module waiting for one value had another file written: every value or none") - } + // 1553: a file whose own values are all there is filled, though another file of the module waits. + eventually(t, "the secret's own file is filled while the template waits", func() bool { return e.read(t, appDB) == "pw" }) vault.keep("own/g14/app/app-token", "arrived", true) eventually(t, "it is filled once the vault holds it", func() bool { return strings.Contains(e.read(t, appConf), "token = arrived\n") && e.read(t, appDB) == "pw" @@ -516,3 +515,50 @@ func TestAConsumersLoginIsNamedWithItsLocalName(t *testing.T) { t.Errorf("a login under a local name: %s", got) } } + +// 1553/AC1: a file holding one known secret is filled although a template of the module waits for a bound value; +// the template's file is not filled, and the report says waiting, names the value, and lists the file it filled. +func TestNatsAKnownSecretsFileIsFilledWhileATemplateWaits1553(t *testing.T) { + b := startBus(t) + heard := b.listen(t) + vault := startVault(t, b) + vault.keep("own/g14/app/app-token", "tok", false) + vault.keep("own/g14/app/db-password", "pw", false) + e := startEngine(t, map[string]string{appConf: "owned", appDB: "owned"}) + // No config font-size: the template waits for it. + b.assign(t, "app", app(t)) + aClient(t, b, e, t.TempDir()) + + eventually(t, "the secret's file is filled", func() bool { return e.read(t, appDB) == "pw" }) + eventually(t, "prov.applied says waiting for the value, with the file it filled", func() bool { + r, _ := heard.last("app") + return r.Status == StatusWaiting && len(r.WaitsFor) == 1 && strings.HasPrefix(r.WaitsFor[0], "config:font-size") && + len(r.Files) == 1 && r.Files[0].Path == appDB + }) + if _, asked := e.counts(appConf); asked != 0 { + t.Error("the template waiting for a value was filled") + } +} + +// 1553/AC3: a template one of whose values waits keeps its file as it is: Fill is never called for it, so no +// half-filled text is written, though its other value (the secret) is known. +func TestNatsATemplateWithAWaitingValueKeepsItsFileAsItIs1553(t *testing.T) { + b := startBus(t) + heard := b.listen(t) + vault := startVault(t, b) + vault.keep("own/g14/app/app-token", "tok", false) + vault.keep("own/g14/app/db-password", "pw", false) + e := startEngine(t, map[string]string{appConf: "owned", appDB: "owned"}) + b.assign(t, "app", app(t)) + aClient(t, b, e, t.TempDir()) + + eventually(t, "it waits", func() bool { r, _ := heard.last("app"); return r.Status == StatusWaiting }) + eventually(t, "the other file is filled, so the pass is over", func() bool { return e.read(t, appDB) == "pw" }) + time.Sleep(200 * time.Millisecond) + if _, asked := e.counts(appConf); asked != 0 { + t.Errorf("Fill was called %d times for a template with a waiting value", asked) + } + if got := e.read(t, appConf); got != "" { + t.Errorf("the template's file holds %q, want it untouched", got) + } +}