From 9edd16eaeba09a3940ee938ca0e3c0fc12a93797 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 18 Oct 2026 02:37:28 +0200 Subject: [PATCH] mesh-provisioner-client takes a module off the node when its assignment is ended (issue 1098) --- .../cmd/mesh-provisioner-client/client.go | 3 + .../cmd/mesh-provisioner-client/ended_test.go | 58 +++++++++++++++++++ .../cmd/mesh-provisioner-client/plan.go | 19 ++++++ 3 files changed, 80 insertions(+) create mode 100644 modules/mesh-provisioner-client/cmd/mesh-provisioner-client/ended_test.go 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 42b87570..7d31b841 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go @@ -445,6 +445,9 @@ func (c *Client) take(s *state, u update) []string { // A kind's answer moves what every module waits on. return sortedKeys(s.plans) case AssignmentsBucket: + // A value that is not a standing assignment (ended, or in a state: ADR 0408 clause 4) is read as a removed + // key, so the module leaves the machine as it does when the key goes (issue 1098). + gone = gone || !standingValue(e.Value()) // Whom it binds, whichever machine's it is: a provider here listing its consumers is filled again. var providers []string if takeBinds(s, key, e.Value(), gone) { diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/ended_test.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/ended_test.go new file mode 100644 index 00000000..c9e2d4b3 --- /dev/null +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/ended_test.go @@ -0,0 +1,58 @@ +package main + +import ( + "encoding/json" + "fmt" + "testing" + "time" +) + +// Issue 1098: a value of the assignments bucket with `ended` set takes the module off the node exactly as a removed +// key does, and a value with the fields the controller adds to an assignment (build, commit, rollout, unknown ones) +// is read under the same key. +func TestNatsAnEndedAssignmentTakesAModuleOffTheNodeLikeARemovedKey(t *testing.T) { + b := startBus(t) + heard := b.listen(t) + e := startEngine(t, map[string]string{dunstrc: "owned"}) + b.config(t, "dunst", "font-size", "13") + p := dunst(t) + b.assign(t, "dunst", p) + aClient(t, b, e, t.TempDir(), time.Hour) + eventually(t, "the needs are recorded", func() bool { return len(b.keys(t, NeedsBucket, node+".dunst.")) == 2 }) + + b.put(t, AssignmentsBucket, node+".dunst", fmt.Sprintf(`{"node":%q,"module":"dunst","assigned":"2026-10-14T20:00:00Z",`+ + `"build":"b-1","commit":"abc","rollout":"r-1","ended":"2026-10-18T01:00:00Z",`+ + `"provisioning":%s}`, node, planJSON(t, p))) + eventually(t, "its needs go", func() bool { return len(b.keys(t, NeedsBucket, node+".dunst.")) == 0 }) + eventually(t, "its report goes", func() bool { return len(b.keys(t, AppliedBucket, node+".dunst")) == 0 }) + eventually(t, "prov.applied says it is removed", func() bool { + r, _ := heard.last("dunst") + return r.Status == StatusRemoved + }) +} + +func TestNatsAnAssignmentWithTheNewFieldsIsFilledUnderTheSameKey(t *testing.T) { + b := startBus(t) + e := startEngine(t, map[string]string{dunstrc: "owned"}) + b.config(t, "dunst", "font-size", "13") + p := dunst(t) + b.assign(t, "dunst", p) + aClient(t, b, e, t.TempDir(), time.Hour) + eventually(t, "the needs are recorded", func() bool { return len(b.keys(t, NeedsBucket, node+".dunst.")) == 2 }) + b.put(t, AssignmentsBucket, node+".dunst", fmt.Sprintf(`{"node":%q,"module":"dunst","assigned":"2026-10-14T20:00:00Z",`+ + `"build":"b-1","artifact":{"name":"dunst","version":"1"},"commit":"abc","rollout":"r-1",`+ + `"since":"2026-10-18T00:00:00Z","ended":"","state":"","future":1,"provisioning":%s}`, node, planJSON(t, p))) + time.Sleep(300 * time.Millisecond) + if got := b.keys(t, NeedsBucket, node+".dunst."); len(got) != 2 { + t.Errorf("a value with the new fields took the module's needs away: %v", got) + } +} + +func planJSON(t *testing.T, p Plan) []byte { + t.Helper() + raw, err := json.Marshal(p) + if err != nil { + t.Fatal(err) + } + return raw +} diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/plan.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/plan.go index 8caf8968..82ee8977 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/plan.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/plan.go @@ -29,6 +29,25 @@ type Assignment struct { Provisioning json.RawMessage `json:"provisioning,omitempty"` } +// standingValue is whether an assignment's value is a standing assignment: neither `ended` (the module was taken off +// its node) nor `state` (a pending one) is set. A value that does not parse is not decided here: planOf refuses it. +func standingValue(value []byte) bool { + var v struct { + Ended json.RawMessage `json:"ended"` + State json.RawMessage `json:"state"` + } + if json.Unmarshal(value, &v) != nil { + return true + } + return !fieldSet(v.Ended) && !fieldSet(v.State) +} + +// fieldSet is whether a field of an assignment's value carries something: not absent, null or empty. +func fieldSet(f json.RawMessage) bool { + s := strings.TrimSpace(string(f)) + return s != "" && s != "null" && s != `""` +} + // Plan is one module's plan on this node. type Plan struct { Templates Templates `json:"templates"`