Skip to content

Commit

Permalink
client: remove null dynamic metadata keys (#18664)
Browse files Browse the repository at this point in the history
Setting a null value to a node metadata is expected to remove it from
subsequent reads. This is true both for static node metadata (defined in
the agent configuration file) as well as for dynamic node metadata
(defined via the Nomad API).

Null values for static metadata must be persisted to indicate that the
value has been removed, but strictly dynamic metadata null values can be
removed from state and client memory.
  • Loading branch information
lgfa29 authored Oct 5, 2023
1 parent ed204e0 commit d425c90
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 0 deletions.
3 changes: 3 additions & 0 deletions .changelog/18664.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
client: ensure null dynamic node metadata values are removed from memory
```
10 changes: 10 additions & 0 deletions client/meta_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
85 changes: 85 additions & 0 deletions client/meta_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

0 comments on commit d425c90

Please sign in to comment.