From fd1c71dc8d40dfa2b974f3d83f79fad9069efda5 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Thu, 23 Dec 2021 12:27:39 -0800 Subject: [PATCH 1/4] core: match struct field order in Copy() --- nomad/structs/structs.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index b856b9ddb6c..e82eba443f8 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -2025,20 +2025,20 @@ func (n *Node) Copy() *Node { nn := new(Node) *nn = *n nn.Attributes = helper.CopyMapStringString(nn.Attributes) - nn.Resources = nn.Resources.Copy() - nn.Reserved = nn.Reserved.Copy() nn.NodeResources = nn.NodeResources.Copy() nn.ReservedResources = nn.ReservedResources.Copy() + nn.Resources = nn.Resources.Copy() + nn.Reserved = nn.Reserved.Copy() nn.Links = helper.CopyMapStringString(nn.Links) nn.Meta = helper.CopyMapStringString(nn.Meta) - nn.Events = copyNodeEvents(n.Events) nn.DrainStrategy = nn.DrainStrategy.Copy() - nn.LastDrain = nn.LastDrain.Copy() + nn.Events = copyNodeEvents(n.Events) + nn.Drivers = copyNodeDrivers(n.Drivers) nn.CSIControllerPlugins = copyNodeCSI(nn.CSIControllerPlugins) nn.CSINodePlugins = copyNodeCSI(nn.CSINodePlugins) - nn.Drivers = copyNodeDrivers(n.Drivers) nn.HostVolumes = copyNodeHostVolumes(n.HostVolumes) nn.HostNetworks = copyNodeHostNetworks(n.HostNetworks) + nn.LastDrain = nn.LastDrain.Copy() return nn } From ea39088f0aaa807934aae9521a1f58ab4d9f6295 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Thu, 23 Dec 2021 12:28:19 -0800 Subject: [PATCH 2/4] core: fix DNS and CPU Core copying --- nomad/structs/structs.go | 12 ++++++++++-- nomad/structs/structs_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index e82eba443f8..77ea61b16d7 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -2657,6 +2657,7 @@ func (n *NetworkResource) Copy() *NetworkResource { } newR := new(NetworkResource) *newR = *n + newR.DNS = n.DNS.Copy() if n.ReservedPorts != nil { newR.ReservedPorts = make([]Port, len(n.ReservedPorts)) copy(newR.ReservedPorts, n.ReservedPorts) @@ -2874,8 +2875,7 @@ func (n *NodeResources) Copy() *NodeResources { newN := new(NodeResources) *newN = *n - - // Copy the networks + newN.Cpu = n.Cpu.Copy() newN.Networks = n.Networks.Copy() // Copy the devices @@ -3062,6 +3062,14 @@ type NodeCpuResources struct { ReservableCpuCores []uint16 } +func (n NodeCpuResources) Copy() NodeCpuResources { + newN := n + newN.ReservableCpuCores = make([]uint16, len(n.ReservableCpuCores)) + copy(newN.ReservableCpuCores, n.ReservableCpuCores) + + return newN +} + func (n *NodeCpuResources) Merge(o *NodeCpuResources) { if o == nil { return diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index b595b419b0a..bc57916333a 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -1672,6 +1672,15 @@ func TestNetworkResource_Copy(t *testing.T) { t.Run(tc.name, func(t *testing.T) { output := tc.inputNetworkResource.Copy() assert.Equal(t, tc.inputNetworkResource, output, tc.name) + + if output == nil { + return + } + + // Assert changes to the copy aren't propagated to the + // original + output.DNS.Servers[1] = "foo" + assert.NotEqual(t, tc.inputNetworkResource, output, tc.name) }) } } @@ -5971,6 +5980,31 @@ func TestMultiregion_CopyCanonicalize(t *testing.T) { require.False(old.Diff(nonEmptyOld)) } +func TestNodeResources_Copy(t *testing.T) { + orig := &NodeResources{ + Cpu: NodeCpuResources{ + CpuShares: int64(32000), + TotalCpuCores: 32, + ReservableCpuCores: []uint16{1, 2, 3, 9}, + }, + Memory: NodeMemoryResources{ + MemoryMB: int64(64000), + }, + Networks: Networks{ + { + Device: "foo", + }, + }, + } + + kopy := orig.Copy() + assert.Equal(t, orig, kopy) + + // Make sure slices aren't shared + kopy.Cpu.ReservableCpuCores[1] = 9000 + assert.NotEqual(t, orig, kopy) +} + func TestNodeResources_Merge(t *testing.T) { res := &NodeResources{ Cpu: NodeCpuResources{ From 0fdf624edad74435101307bc12795e599193df1b Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Thu, 23 Dec 2021 12:34:05 -0800 Subject: [PATCH 3/4] add changelog for Node.Copy fix --- .changelog/11744.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/11744.txt diff --git a/.changelog/11744.txt b/.changelog/11744.txt new file mode 100644 index 00000000000..f8a3fb52e0a --- /dev/null +++ b/.changelog/11744.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: Fix missing fields in Node.Copy() +``` From e1c8d58cea479fe2b74c148766e4f59aa1fc65a2 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Thu, 23 Dec 2021 16:40:35 -0800 Subject: [PATCH 4/4] do not initialize copy's slice if nil in original --- nomad/node_endpoint_test.go | 4 +--- nomad/structs/structs.go | 6 ++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nomad/node_endpoint_test.go b/nomad/node_endpoint_test.go index 65ac8c03a0c..1070892b081 100644 --- a/nomad/node_endpoint_test.go +++ b/nomad/node_endpoint_test.go @@ -1531,9 +1531,7 @@ func TestClientEndpoint_GetNode(t *testing.T) { node.StatusUpdatedAt = resp2.Node.StatusUpdatedAt node.SecretID = "" node.Events = resp2.Node.Events - if !reflect.DeepEqual(node, resp2.Node) { - t.Fatalf("bad: %#v \n %#v", node, resp2.Node) - } + require.Equal(t, node, resp2.Node) // assert that the node register event was set correctly if len(resp2.Node.Events) != 1 { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 77ea61b16d7..8aaf8951452 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -3064,8 +3064,10 @@ type NodeCpuResources struct { func (n NodeCpuResources) Copy() NodeCpuResources { newN := n - newN.ReservableCpuCores = make([]uint16, len(n.ReservableCpuCores)) - copy(newN.ReservableCpuCores, n.ReservableCpuCores) + if n.ReservableCpuCores != nil { + newN.ReservableCpuCores = make([]uint16, len(n.ReservableCpuCores)) + copy(newN.ReservableCpuCores, n.ReservableCpuCores) + } return newN }