diff --git a/.changelog/18664.txt b/.changelog/18664.txt new file mode 100644 index 000000000..8359cb6fe --- /dev/null +++ b/.changelog/18664.txt @@ -0,0 +1,3 @@ +```release-note:bug +client: ensure null dynamic node metadata values are removed from memory +``` diff --git a/client/meta_endpoint.go b/client/meta_endpoint.go index 7336747e5..099bcd20f 100644 --- a/client/meta_endpoint.go +++ b/client/meta_endpoint.go @@ -45,6 +45,16 @@ func (n *NodeMeta) Apply(args *structs.NodeMetaApplyRequest, reply *structs.Node dyn = maps.Clone(n.c.metaDynamic) maps.Copy(dyn, args.Meta) + // Delete null values from the dynamic metadata if they are also not + // static. Static null values must be kept so their removal is + // persisted in client state. + for k, v := range args.Meta { + _, static := n.c.metaStatic[k] + if v == nil && !static { + delete(dyn, k) + } + } + if stateErr = n.c.stateDB.PutNodeMeta(dyn); stateErr != nil { return } diff --git a/client/meta_endpoint_test.go b/client/meta_endpoint_test.go index 6b2778be7..ac93aa48b 100644 --- a/client/meta_endpoint_test.go +++ b/client/meta_endpoint_test.go @@ -96,3 +96,88 @@ func TestNodeMeta_Validation(t *testing.T) { err = c1.ClientRPC("NodeMeta.Apply", applyReq, &resp) must.ErrorContains(t, err, "*") } + +func TestNodeMeta_unset(t *testing.T) { + ci.Parallel(t) + + s, cleanupS := nomad.TestServer(t, nil) + defer cleanupS() + testutil.WaitForLeader(t, s.RPC) + + c1, cleanup := TestClient(t, func(c *config.Config) { + c.Servers = []string{s.GetConfig().RPCAddr.String()} + c.Node.Meta["static_meta"] = "true" + }) + defer cleanup() + + // Set dynamic node metadata. + applyReq := &structs.NodeMetaApplyRequest{ + NodeID: c1.NodeID(), + Meta: map[string]*string{ + "dynamic_meta": pointer.Of("true"), + }, + } + var resp structs.NodeMetaResponse + err := c1.ClientRPC("NodeMeta.Apply", applyReq, &resp) + must.NoError(t, err) + + // Check static_meta: + // 1. must be present in Static. + // 2. must be present in Meta + must.Eq(t, resp.Static["static_meta"], "true") + must.Eq(t, resp.Meta["static_meta"], "true") + + // Check dynamic_meta: + // 1. must be present in Dynamic. + // 2. must be present in Meta + must.Eq(t, *resp.Dynamic["dynamic_meta"], "true") + must.Eq(t, resp.Meta["dynamic_meta"], "true") + + // Unset static node metada. + applyReq = &structs.NodeMetaApplyRequest{ + NodeID: c1.NodeID(), + Meta: map[string]*string{ + "static_meta": nil, + }, + } + err = c1.ClientRPC("NodeMeta.Apply", applyReq, &resp) + must.NoError(t, err) + + // Check static_meta: + // 1. must be present in Static. + // 2. must not be present in Meta + // 3. must be nil in Dynamic to persist its removal even on restarts. + must.Eq(t, resp.Static["static_meta"], "true") + must.MapNotContainsKey(t, resp.Meta, "static_meta") + must.Nil(t, resp.Dynamic["static_meta"]) + + // Check dynamic_meta: + // 1. must be present in Dynamic. + // 2. must be present in Meta + must.Eq(t, *resp.Dynamic["dynamic_meta"], "true") + must.Eq(t, resp.Meta["dynamic_meta"], "true") + + // Unset dynamic node metada. + applyReq = &structs.NodeMetaApplyRequest{ + NodeID: c1.NodeID(), + Meta: map[string]*string{ + "dynamic_meta": nil, + }, + } + err = c1.ClientRPC("NodeMeta.Apply", applyReq, &resp) + must.NoError(t, err) + + // Check static_meta: + // 1. must be present in Static. + // 2. must not be present in Meta + // 3. must be nil in Dynamic to persist its removal even on restarts. + must.Eq(t, resp.Static["static_meta"], "true") + must.MapNotContainsKey(t, resp.Meta, "static_meta") + must.Nil(t, resp.Dynamic["static_meta"]) + + // Check dynamic_meta: + // 1. must not be present in Dynamic. + // 2. must not be present in Meta + must.MapNotContainsKey(t, resp.Dynamic, "dynamic_meta") + must.MapNotContainsKey(t, resp.Meta, "dynamic_meta") +}