diff --git a/client/client.go b/client/client.go index cdecfb645..d2c12b05d 100644 --- a/client/client.go +++ b/client/client.go @@ -1227,30 +1227,14 @@ func (c *Client) updateNodeFromFingerprint(response *fingerprint.FingerprintResp } // COMPAT(0.10): Remove in 0.10 - // update the response networks with the config - // if we still have node changes, merge them - if response.Resources != nil { - response.Resources.Networks = updateNetworks( - c.config.Node.Resources.Networks, - response.Resources.Networks, - c.config) - if !c.config.Node.Resources.Equals(response.Resources) { - c.config.Node.Resources.Merge(response.Resources) - nodeHasChanged = true - } + if response.Resources != nil && !resourcesAreEqual(c.config.Node.Resources, response.Resources) { + nodeHasChanged = true + c.config.Node.Resources.Merge(response.Resources) } - // update the response networks with the config - // if we still have node changes, merge them - if response.NodeResources != nil { - response.NodeResources.Networks = updateNetworks( - c.config.Node.NodeResources.Networks, - response.NodeResources.Networks, - c.config) - if !c.config.Node.NodeResources.Equals(response.NodeResources) { - c.config.Node.NodeResources.Merge(response.NodeResources) - nodeHasChanged = true - } + if response.NodeResources != nil && !c.config.Node.NodeResources.Equals(response.NodeResources) { + nodeHasChanged = true + c.config.Node.NodeResources.Merge(response.NodeResources) } if nodeHasChanged { @@ -1260,27 +1244,32 @@ func (c *Client) updateNodeFromFingerprint(response *fingerprint.FingerprintResp return c.configCopy.Node } -// updateNetworks preserves manually configured network options, but -// applies fingerprint updates -func updateNetworks(ns structs.Networks, up structs.Networks, c *config.Config) structs.Networks { - if c.NetworkInterface == "" { - ns = up - } else { - // if a network is configured, use only that network - // use the fingerprinted data - for _, n := range up { - if c.NetworkInterface == n.Device { - ns = []*structs.NetworkResource{n} - } - } - // if not matched, ns has the old data +// resourcesAreEqual is a temporary function to compare whether resources are +// equal. We can use this until we change fingerprinters to set pointers on a +// return type. +func resourcesAreEqual(first, second *structs.Resources) bool { + if first.CPU != second.CPU { + return false } - if c.NetworkSpeed != 0 { - for _, n := range ns { - n.MBits = c.NetworkSpeed + if first.MemoryMB != second.MemoryMB { + return false + } + if first.DiskMB != second.DiskMB { + return false + } + if len(first.Networks) != len(second.Networks) { + return false + } + for i, e := range first.Networks { + if len(second.Networks) < i { + return false + } + f := second.Networks[i] + if !e.Equals(f) { + return false } } - return ns + return true } // retryIntv calculates a retry interval value given the base diff --git a/client/client_test.go b/client/client_test.go index ea034ddf5..a2f0b8e6a 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1243,81 +1243,6 @@ func TestClient_UpdateNodeFromDevicesAccumulates(t *testing.T) { } -// TestClient_UpdateNodeFromFingerprintKeepsConfig asserts manually configured -// network interfaces take precedence over fingerprinted ones. -func TestClient_UpdateNodeFromFingerprintKeepsConfig(t *testing.T) { - t.Parallel() - - // Client without network configured updates to match fingerprint - client, cleanup := TestClient(t, nil) - defer cleanup() - // capture the platform fingerprinted device name for the next test - dev := client.config.Node.NodeResources.Networks[0].Device - client.updateNodeFromFingerprint(&fingerprint.FingerprintResponse{ - NodeResources: &structs.NodeResources{ - Cpu: structs.NodeCpuResources{CpuShares: 123}, - Networks: []*structs.NetworkResource{{Device: "any-interface"}}, - }, - Resources: &structs.Resources{ - CPU: 80, - Networks: []*structs.NetworkResource{{Device: "any-interface"}}, - }, - }) - assert.Equal(t, int64(123), client.config.Node.NodeResources.Cpu.CpuShares) - assert.Equal(t, "any-interface", client.config.Node.NodeResources.Networks[0].Device) - assert.Equal(t, 80, client.config.Node.Resources.CPU) - assert.Equal(t, "any-interface", client.config.Node.Resources.Networks[0].Device) - - // Client with network interface configured keeps the config - // setting on update - name := "TestClient_UpdateNodeFromFingerprintKeepsConfig2" - client, cleanup = TestClient(t, func(c *config.Config) { - c.NetworkInterface = dev - c.Node.Name = name - // Node is already a mock.Node, with a device - c.Node.NodeResources.Networks[0].Device = dev - c.Node.Resources.Networks = c.Node.NodeResources.Networks - }) - defer cleanup() - client.updateNodeFromFingerprint(&fingerprint.FingerprintResponse{ - NodeResources: &structs.NodeResources{ - Cpu: structs.NodeCpuResources{CpuShares: 123}, - Networks: []*structs.NetworkResource{ - {Device: "any-interface", MBits: 20}, - {Device: dev, MBits: 20}, - }, - }, - Resources: &structs.Resources{ - CPU: 80, - Networks: []*structs.NetworkResource{{Device: "any-interface"}}, - }, - }) - assert.Equal(t, int64(123), client.config.Node.NodeResources.Cpu.CpuShares) - // only the configured device is kept - assert.Equal(t, 1, len(client.config.Node.NodeResources.Networks)) - assert.Equal(t, dev, client.config.Node.NodeResources.Networks[0].Device) - // network speed updates to the configured network are kept - assert.Equal(t, 20, client.config.Node.NodeResources.Networks[0].MBits) - assert.Equal(t, 80, client.config.Node.Resources.CPU) - assert.Equal(t, dev, client.config.Node.Resources.Networks[0].Device) - - // Network speed is applied to all NetworkResources - client.config.NetworkInterface = "" - client.config.NetworkSpeed = 100 - client.updateNodeFromFingerprint(&fingerprint.FingerprintResponse{ - NodeResources: &structs.NodeResources{ - Cpu: structs.NodeCpuResources{CpuShares: 123}, - Networks: []*structs.NetworkResource{{Device: "any-interface", MBits: 20}}, - }, - Resources: &structs.Resources{ - CPU: 80, - Networks: []*structs.NetworkResource{{Device: "any-interface"}}, - }, - }) - assert.Equal(t, "any-interface", client.config.Node.NodeResources.Networks[0].Device) - assert.Equal(t, 100, client.config.Node.NodeResources.Networks[0].MBits) -} - func TestClient_computeAllocatedDeviceStats(t *testing.T) { logger := testlog.HCLogger(t) c := &Client{logger: logger} diff --git a/client/fingerprint/network.go b/client/fingerprint/network.go index 9cc4fcc65..2d706a1e9 100644 --- a/client/fingerprint/network.go +++ b/client/fingerprint/network.go @@ -67,9 +67,7 @@ func (f *NetworkFingerprint) Fingerprint(req *FingerprintRequest, resp *Fingerpr intf, err := f.findInterface(cfg.NetworkInterface) switch { case err != nil: - return fmt.Errorf("Error while detecting network interface %s during fingerprinting: %v", - cfg.NetworkInterface, - err) + return fmt.Errorf("Error while detecting network interface during fingerprinting: %v", err) case intf == nil: // No interface could be found return nil diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 746c5e3cf..1a5cdaf43 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -263,7 +263,7 @@ func setImplicitConstraints(j *structs.Job) { found := false for _, c := range tg.Constraints { - if c.Equals(vaultConstraint) { + if c.Equal(vaultConstraint) { found = true break } @@ -288,7 +288,7 @@ func setImplicitConstraints(j *structs.Job) { found := false for _, c := range tg.Constraints { - if c.Equals(sigConstraint) { + if c.Equal(sigConstraint) { found = true break } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 4a975764c..43dd3c438 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1715,7 +1715,7 @@ type Resources struct { DiskMB int IOPS int // COMPAT(0.10): Only being used to issue warnings Networks Networks - Devices ResourceDevices + Devices []*RequestedDevice } const ( @@ -1771,7 +1771,6 @@ func (r *Resources) Validate() error { } // Merge merges this resource with another resource. -// COMPAT(0.10): Remove in 0.10 func (r *Resources) Merge(other *Resources) { if other.CPU != 0 { r.CPU = other.CPU @@ -1790,52 +1789,6 @@ func (r *Resources) Merge(other *Resources) { } } -// COMPAT(0.10): Remove in 0.10 -func (r *Resources) Equals(o *Resources) bool { - if r == o { - return true - } - if r == nil || o == nil { - return false - } - return r.CPU == o.CPU && - r.MemoryMB == o.MemoryMB && - r.DiskMB == o.DiskMB && - r.IOPS == o.IOPS && - r.Networks.Equals(&o.Networks) && - r.Devices.Equals(&o.Devices) -} - -// COMPAT(0.10): Remove in 0.10 -// ResourceDevices are part of Resources -type ResourceDevices []*RequestedDevice - -// COMPAT(0.10): Remove in 0.10 -// Equals ResourceDevices as set keyed by Name -func (d *ResourceDevices) Equals(o *ResourceDevices) bool { - if d == o { - return true - } - if d == nil || o == nil { - return false - } - if len(*d) != len(*o) { - return false - } - m := make(map[string]*RequestedDevice, len(*d)) - for _, e := range *d { - m[e.Name] = e - } - for _, oe := range *o { - de, ok := m[oe.Name] - if !ok || !de.Equals(oe) { - return false - } - } - return true -} - -// COMPAT(0.10): Remove in 0.10 func (r *Resources) Canonicalize() { // Ensure that an empty and nil slices are treated the same to avoid scheduling // problems since we use reflect DeepEquals. @@ -1854,7 +1807,6 @@ func (r *Resources) Canonicalize() { // MeetsMinResources returns an error if the resources specified are less than // the minimum allowed. // This is based on the minimums defined in the Resources type -// COMPAT(0.10): Remove in 0.10 func (r *Resources) MeetsMinResources() error { var mErr multierror.Error minResources := MinResources() @@ -1903,7 +1855,6 @@ func (r *Resources) Copy() *Resources { } // NetIndex finds the matching net index using device name -// COMPAT(0.10): Remove in 0.10 func (r *Resources) NetIndex(n *NetworkResource) int { return r.Networks.NetIndex(n) } @@ -1911,7 +1862,6 @@ func (r *Resources) NetIndex(n *NetworkResource) int { // Superset checks if one set of resources is a superset // of another. This ignores network resources, and the NetworkIndex // should be used for that. -// COMPAT(0.10): Remove in 0.10 func (r *Resources) Superset(other *Resources) (bool, string) { if r.CPU < other.CPU { return false, "cpu" @@ -1927,7 +1877,6 @@ func (r *Resources) Superset(other *Resources) (bool, string) { // Add adds the resources of the delta to this, potentially // returning an error if not possible. -// COMPAT(0.10): Remove in 0.10 func (r *Resources) Add(delta *Resources) error { if delta == nil { return nil @@ -1948,7 +1897,6 @@ func (r *Resources) Add(delta *Resources) error { return nil } -// COMPAT(0.10): Remove in 0.10 func (r *Resources) GoString() string { return fmt.Sprintf("*%#v", *r) } @@ -2126,24 +2074,11 @@ type RequestedDevice struct { // Constraints are a set of constraints to apply when selecting the device // to use. - Constraints Constraints + Constraints []*Constraint // Affinities are a set of affinites to apply when selecting the device // to use. - Affinities Affinities -} - -func (r *RequestedDevice) Equals(o *RequestedDevice) bool { - if r == o { - return true - } - if r == nil || o == nil { - return false - } - return r.Name == o.Name && - r.Count == o.Count && - r.Constraints.Equals(&o.Constraints) && - r.Affinities.Equals(&o.Affinities) + Affinities []*Affinity } func (r *RequestedDevice) Copy() *RequestedDevice { @@ -2314,9 +2249,15 @@ func (n *NodeResources) Equals(o *NodeResources) bool { if !n.Disk.Equals(&o.Disk) { return false } - if !n.Networks.Equals(&o.Networks) { + + if len(n.Networks) != len(o.Networks) { return false } + for i, n := range n.Networks { + if !n.Equals(o.Networks[i]) { + return false + } + } // Check the devices if !DevicesEquals(n.Devices, o.Devices) { @@ -2326,28 +2267,7 @@ func (n *NodeResources) Equals(o *NodeResources) bool { return true } -// Equals equates Networks as a set -func (n *Networks) Equals(o *Networks) bool { - if n == o { - return true - } - if n == nil || o == nil { - return false - } - if len(*n) != len(*o) { - return false - } - for _, ne := range *n { - for _, oe := range *o { - if !ne.Equals(oe) { - return false - } - } - } - return true -} - -// DevicesEquals returns true if the two device arrays are set equal +// DevicesEquals returns true if the two device arrays are equal func DevicesEquals(d1, d2 []*NodeDeviceResource) bool { if len(d1) != len(d2) { return false @@ -6495,11 +6415,10 @@ type Constraint struct { } // Equal checks if two constraints are equal -func (c *Constraint) Equals(o *Constraint) bool { - return c == o || - c.LTarget == o.LTarget && - c.RTarget == o.RTarget && - c.Operand == o.Operand +func (c *Constraint) Equal(o *Constraint) bool { + return c.LTarget == o.LTarget && + c.RTarget == o.RTarget && + c.Operand == o.Operand } func (c *Constraint) Copy() *Constraint { @@ -6575,29 +6494,6 @@ func (c *Constraint) Validate() error { return mErr.ErrorOrNil() } -type Constraints []*Constraint - -// Equals compares Constraints as a set -func (xs *Constraints) Equals(ys *Constraints) bool { - if xs == ys { - return true - } - if xs == nil || ys == nil { - return false - } - if len(*xs) != len(*ys) { - return false - } - for _, x := range *xs { - for _, y := range *ys { - if !x.Equals(y) { - return false - } - } - } - return true -} - // Affinity is used to score placement options based on a weight type Affinity struct { LTarget string // Left-hand target @@ -6608,12 +6504,11 @@ type Affinity struct { } // Equal checks if two affinities are equal -func (a *Affinity) Equals(o *Affinity) bool { - return a == o || - a.LTarget == o.LTarget && - a.RTarget == o.RTarget && - a.Operand == o.Operand && - a.Weight == o.Weight +func (a *Affinity) Equal(o *Affinity) bool { + return a.LTarget == o.LTarget && + a.RTarget == o.RTarget && + a.Operand == o.Operand && + a.Weight == o.Weight } func (a *Affinity) Copy() *Affinity { @@ -6694,29 +6589,6 @@ type Spread struct { str string } -type Affinities []*Affinity - -// Equals compares Affinities as a set -func (xs *Affinities) Equals(ys *Affinities) bool { - if xs == ys { - return true - } - if xs == nil || ys == nil { - return false - } - if len(*xs) != len(*ys) { - return false - } - for _, x := range *xs { - for _, y := range *ys { - if !x.Equals(y) { - return false - } - } - } - return true -} - func (s *Spread) Copy() *Spread { if s == nil { return nil