Keep the node's own as it runs from an engine's first start, and refuse the node's own apply removing a module no want removed (issue 774, review round 1)
The reviewer showed how76e214astill lets the 07:54 outage happen at the re-landed engine's first start on novox. applied/ is empty there, so loadApplied loads nothing. The kept declaration retires, because every placed module has a have. Nothing then carries the node's own composition. The guard misses nats, which has no want there, so its nats.conf is guessed as nats's and removed. - At the start, when the store has no node's own composition, the node's own runs as the kept declaration has it, and the store keeps it now. The kept declaration retires only once the store holds `node`. - What this engine applied runs until loadHad reads the composition a differing have names. The bucket no longer keeps the one novox's have names, so the stored one stays. - In the node's own apply, a resource of another module goes only when that module's want says removed (Scope.RemovedByWant). A module the node's own composition carried has no want, so Placed never named it. Cost: the node's own apply cannot drop one of its own dotted resources without such a want. It is refused whole and said, never removed silently. Tests, with no new test functions: - A first-start case in the agent's Everything test (empty store, then a start with the declaration retired). It fails on76e214a. - Two rows in the guard's table: no want refuses; a want saying removed removes. - A case for the store in the authority test.
This commit is contained in:
@@ -30,6 +30,16 @@ func (s appliedStore) path(module string) (string, error) {
|
||||
return filepath.Join(s.dir, module+".json"), nil
|
||||
}
|
||||
|
||||
// holds says whether the store keeps the module's composition.
|
||||
func (s appliedStore) holds(module string) bool {
|
||||
path, err := s.path(module)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
_, err = os.Stat(path)
|
||||
return err == nil
|
||||
}
|
||||
|
||||
func (s appliedStore) Load() (map[string][]byte, error) {
|
||||
entries, err := os.ReadDir(s.dir)
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
|
||||
@@ -127,6 +127,38 @@ func TestTheAppliedAuthorityIsKeptForTheRecord(t *testing.T) {
|
||||
if err := certifyThisNode(context.Background(), state, "n1", trust, ask, func() { written++ }); err == nil || written != 1 {
|
||||
t.Errorf("a certificate of another authority was kept: %v", err)
|
||||
}
|
||||
|
||||
// Issue 774: what the engine applied is kept beside the state too, one file per module, 0600 in a 0700 directory,
|
||||
// written by a rename so a crash leaves the one before; a name that is no module's is refused, never a path.
|
||||
t.Run("the compositions the engine applied are kept beside the state", func(t *testing.T) {
|
||||
applied := appliedStoreFor(filepath.Join(t.TempDir(), "state", "state.json"))
|
||||
if m, err := applied.Load(); err != nil || len(m) != 0 || applied.holds("node") {
|
||||
t.Fatalf("an empty store loads %v, %v", m, err)
|
||||
}
|
||||
for _, tc := range []struct {
|
||||
module string
|
||||
takes bool
|
||||
}{{"node", true}, {"nats", true}, {"mesh-delivery-view", true}, {"../state", false}, {"a/b", false}, {"..", false}, {"", false}} {
|
||||
if err := applied.Keep(tc.module, []byte(`{"v":1}`)); (err == nil) != tc.takes {
|
||||
t.Errorf("keeping %q: %v, want taken %v", tc.module, err, tc.takes)
|
||||
}
|
||||
}
|
||||
if err := applied.Keep("node", []byte(`{"v":2}`)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
entries, _ := os.ReadDir(applied.dir)
|
||||
dirInfo, _ := os.Stat(applied.dir)
|
||||
fileInfo, _ := os.Stat(filepath.Join(applied.dir, "node.json"))
|
||||
if len(entries) != 3 || dirInfo.Mode().Perm() != 0o700 || fileInfo.Mode().Perm() != 0o600 {
|
||||
t.Fatalf("the store holds %d files (want 3, no temporary one left), directory %v, file %v", len(entries), dirInfo.Mode(), fileInfo.Mode())
|
||||
}
|
||||
if err := applied.Drop("nats"); err != nil || applied.Drop("nats") != nil || applied.holds("nats") {
|
||||
t.Fatalf("a dropped module is still held, or dropping twice fails: %v", err)
|
||||
}
|
||||
if m, err := applied.Load(); err != nil || string(m["node"]) != `{"v":2}` || len(m) != 2 {
|
||||
t.Fatalf("the store loads %v, %v", m, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// testAuthority is a self-signed authority certificate for key, as the vault makes its own.
|
||||
|
||||
@@ -454,8 +454,15 @@ func nodeAgent(ctx context.Context, opts options, signer ed25519.PublicKey, trus
|
||||
var ag *agent.Agent
|
||||
// The kept release-5 declaration is no longer read once every module the seat's placement puts here has a have of its
|
||||
// own (ADR 0550 §2: its signature is the controller's, trusted no more after the lease write).
|
||||
if retired, why := placedAllHave(linked, session, node); retired && !keptRetired.Swap(true) {
|
||||
say("the kept declaration is no longer read: " + why)
|
||||
// Nor before this engine keeps the node's own as it runs (applied/node.json): until then the kept declaration is the one
|
||||
// record of what the node's own composition carries, its folded modules among them (issue 774, review of the
|
||||
// re-land, round 1: novox's first start would compose its own with nats running nothing).
|
||||
if retired, why := placedAllHave(linked, session, node); retired && !keptRetired.Load() {
|
||||
if !appliedStoreFor(opts.state).holds(agent.NodeModule) {
|
||||
say("the kept declaration is still read: " + why + ", but this engine keeps no node's own composition yet")
|
||||
} else if !keptRetired.Swap(true) {
|
||||
say("the kept declaration is no longer read: " + why)
|
||||
}
|
||||
}
|
||||
kept := keptOnce(opts, signer)
|
||||
// The mesh's own certificate the declaration kept from before carries, kept for the record before this machine
|
||||
@@ -529,7 +536,7 @@ func nodeAgent(ctx context.Context, opts options, signer ed25519.PublicKey, trus
|
||||
if herr != nil {
|
||||
res = failedWith(herr.Error())
|
||||
} else {
|
||||
res = applyComposition(actx, opts, held, others, ag.Placed(scopeName(c)), sched, say)
|
||||
res = applyComposition(actx, opts, held, others, ag.Placed(scopeName(c)), ag.RemovedByWant(), sched, say)
|
||||
}
|
||||
done()
|
||||
// The mesh's own certificate it carries is kept for this machine's record (ADR 0555 §3).
|
||||
@@ -690,7 +697,10 @@ func (c *agentNow) get() *agent.Agent {
|
||||
//
|
||||
// placed is the modules a want still places here besides this one: none of their resources is removed by this apply,
|
||||
// which is refused instead (issue 774).
|
||||
func applyComposition(ctx context.Context, opts options, c delivery.Composed, others []delivery.Composed, placed map[string]bool,
|
||||
//
|
||||
// removed is the modules whose want says removed: in the node's own apply, a resource of any other module goes only when
|
||||
// its module is among them (issue 774).
|
||||
func applyComposition(ctx context.Context, opts options, c delivery.Composed, others []delivery.Composed, placed, removed map[string]bool,
|
||||
sched *apply.Scheduler, say link.Announce) agent.Result {
|
||||
name := scopeName(c)
|
||||
if c.Module == "" {
|
||||
@@ -707,6 +717,12 @@ func applyComposition(ctx context.Context, opts options, c delivery.Composed, ot
|
||||
return failedWith(err.Error())
|
||||
}
|
||||
scope := apply.Scope{Module: name, Others: resourcesOf(others), Placed: placed}
|
||||
if name == agent.NodeModule {
|
||||
scope.RemovedByWant = removed
|
||||
if scope.RemovedByWant == nil {
|
||||
scope.RemovedByWant = map[string]bool{}
|
||||
}
|
||||
}
|
||||
got := applyModuleHeld(ctx, opts, d, &scope, sched, say)
|
||||
if got.Refused != "" {
|
||||
return failedWith(got.Refused)
|
||||
|
||||
Reference in New Issue
Block a user