Merge pull request #5551 from hashicorp/b-revert-fingerprinter-manual…

…-config Revert accidental merge of pr #5482
hashicorp · Apr 11, 2019 · cfb1aa2 · cfb1aa2
2 parents 3171529 + c45652a
commit cfb1aa2
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 268 deletions.
diff --git 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 first.MemoryMB != second.MemoryMB {
+		return false
+	}
+	if first.DiskMB != second.DiskMB {
+		return false
 	}
-	if c.NetworkSpeed != 0 {
-		for _, n := range ns {
-			n.MBits = c.NetworkSpeed
+	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
@@ -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
@@ -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
@@ -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
 			}