Skip to content

Commit

Permalink
cni: fix networkinterface config override /w multiple network resources
Browse files Browse the repository at this point in the history
  • Loading branch information
nickethier committed Mar 27, 2020
1 parent 55bb5f6 commit b899020
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 48 deletions.
37 changes: 20 additions & 17 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1412,7 +1412,6 @@ func (c *Client) updateNodeFromFingerprint(response *fingerprint.FingerprintResp
// 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) {
Expand All @@ -1425,7 +1424,6 @@ func (c *Client) updateNodeFromFingerprint(response *fingerprint.FingerprintResp
// 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) {
Expand All @@ -1441,33 +1439,38 @@ 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 device is configured, filter up to contain details for only
func updateNetworks(up structs.Networks, c *config.Config) structs.Networks {
if up == nil {
return nil
}

if c.NetworkInterface != "" {
// For host networks, if a network device is configured filter up to contain details for only
// that device
upd := []*structs.NetworkResource{}
for _, n := range up {
if c.NetworkInterface == n.Device {
switch n.Mode {
case "host":
if c.NetworkInterface == n.Device {
upd = append(upd, n)
}
default:
upd = append(upd, n)

}
}
// If updates, use them. Otherwise, ns contains the configured interfaces
if len(upd) > 0 {
ns = upd
}
up = upd
}

// ns is set, apply the config NetworkSpeed to all
if c.NetworkSpeed != 0 {
for _, n := range ns {
n.MBits = c.NetworkSpeed
for _, n := range up {
if n.Mode == "host" {
n.MBits = c.NetworkSpeed
}
}
}
return ns
return up
}

// retryIntv calculates a retry interval value given the base
Expand Down
48 changes: 21 additions & 27 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1184,17 +1184,16 @@ func TestClient_UpdateNodeFromFingerprintKeepsConfig(t *testing.T) {
client.updateNodeFromFingerprint(&fingerprint.FingerprintResponse{
NodeResources: &structs.NodeResources{
Cpu: structs.NodeCpuResources{CpuShares: 123},
Networks: []*structs.NetworkResource{{Device: "any-interface"}},
Networks: []*structs.NetworkResource{{Mode: "host", Device: "any-interface"}},
},
Resources: &structs.Resources{
CPU: 80,
Networks: []*structs.NetworkResource{{Device: "any-interface"}},
CPU: 80,
},
})
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)
idx := len(client.config.Node.NodeResources.Networks) - 1
require.Equal(t, int64(123), client.config.Node.NodeResources.Cpu.CpuShares)
require.Equal(t, "any-interface", client.config.Node.NodeResources.Networks[idx].Device)
require.Equal(t, 80, client.config.Node.Resources.CPU)

// lookup an interface. client.Node starts with a hardcoded value, eth0,
// and is only updated async through fingerprinter.
Expand All @@ -1210,48 +1209,43 @@ func TestClient_UpdateNodeFromFingerprintKeepsConfig(t *testing.T) {
client, cleanup = TestClient(t, func(c *config.Config) {
c.NetworkInterface = dev
c.Node.Name = name
c.Options["fingerprint.blacklist"] = "network"
// 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},
{Mode: "host", Device: "any-interface", MBits: 20},
},
},
Resources: &structs.Resources{
CPU: 80,
Networks: []*structs.NetworkResource{{Device: "any-interface"}},
},
})
assert.Equal(t, int64(123), client.config.Node.NodeResources.Cpu.CpuShares)
require.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)
require.Equal(t, 2, len(client.config.Node.NodeResources.Networks))
require.Equal(t, dev, client.config.Node.NodeResources.Networks[0].Device)
require.Equal(t, "bridge", client.config.Node.NodeResources.Networks[1].Mode)

// 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}},
Cpu: structs.NodeCpuResources{CpuShares: 123},
Networks: []*structs.NetworkResource{
{Mode: "host", Device: "any-interface", MBits: 20},
},
},
Resources: &structs.Resources{
CPU: 80,
Networks: []*structs.NetworkResource{{Device: "any-interface"}},
CPU: 80,
},
})
assert.Equal(t, "any-interface", client.config.Node.NodeResources.Networks[0].Device)
assert.Equal(t, 100, client.config.Node.NodeResources.Networks[0].MBits)
assert.Equal(t, 3, len(client.config.Node.NodeResources.Networks))
assert.Equal(t, "any-interface", client.config.Node.NodeResources.Networks[2].Device)
assert.Equal(t, 100, client.config.Node.NodeResources.Networks[2].MBits)
assert.Equal(t, 0, client.config.Node.NodeResources.Networks[1].MBits)
}

// Support multiple IP addresses (ipv4 vs. 6, e.g.) on the configured network interface
Expand Down
6 changes: 2 additions & 4 deletions client/fingerprint/env_aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,12 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F
response.AddAttribute(key, v)
}

// newNetwork is populated and added to the Nodes resources
var newNetwork *structs.NetworkResource

// copy over network specific information
if val, ok := response.Attributes["unique.platform.aws.local-ipv4"]; ok && val != "" {
response.AddAttribute("unique.network.ip-address", val)

newNetwork = &structs.NetworkResource{
newNetwork := &structs.NetworkResource{
Mode: "host",
Device: "eth0",
IP: val,
CIDR: val + "/32",
Expand Down

0 comments on commit b899020

Please sign in to comment.