client fingerprinter doesn't overwrite manual configuration

Revert "Revert accidental merge of pr #5482"
This reverts commit c45652ab8c113487b9d4fbfb107782cbcf8a85b0.
This commit is contained in:
Lang Martin 2019-04-11 13:17:26 -04:00
parent 72862db778
commit eba4e29440
5 changed files with 268 additions and 52 deletions

View File

@ -1227,14 +1227,30 @@ func (c *Client) updateNodeFromFingerprint(response *fingerprint.FingerprintResp
}
// COMPAT(0.10): Remove in 0.10
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.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.NodeResources != nil && !c.config.Node.NodeResources.Equals(response.NodeResources) {
nodeHasChanged = true
c.config.Node.NodeResources.Merge(response.NodeResources)
// 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 nodeHasChanged {
@ -1244,32 +1260,27 @@ func (c *Client) updateNodeFromFingerprint(response *fingerprint.FingerprintResp
return c.configCopy.Node
}
// 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 len(first.Networks) != len(second.Networks) {
return false
}
for i, e := range first.Networks {
if len(second.Networks) < i {
return false
// 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}
}
}
f := second.Networks[i]
if !e.Equals(f) {
return false
// if not matched, ns has the old data
}
if c.NetworkSpeed != 0 {
for _, n := range ns {
n.MBits = c.NetworkSpeed
}
}
return true
return ns
}
// retryIntv calculates a retry interval value given the base

View File

@ -1243,6 +1243,81 @@ 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}

View File

@ -67,7 +67,9 @@ 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 during fingerprinting: %v", err)
return fmt.Errorf("Error while detecting network interface %s during fingerprinting: %v",
cfg.NetworkInterface,
err)
case intf == nil:
// No interface could be found
return nil

View File

@ -263,7 +263,7 @@ func setImplicitConstraints(j *structs.Job) {
found := false
for _, c := range tg.Constraints {
if c.Equal(vaultConstraint) {
if c.Equals(vaultConstraint) {
found = true
break
}
@ -288,7 +288,7 @@ func setImplicitConstraints(j *structs.Job) {
found := false
for _, c := range tg.Constraints {
if c.Equal(sigConstraint) {
if c.Equals(sigConstraint) {
found = true
break
}

View File

@ -1715,7 +1715,7 @@ type Resources struct {
DiskMB int
IOPS int // COMPAT(0.10): Only being used to issue warnings
Networks Networks
Devices []*RequestedDevice
Devices ResourceDevices
}
const (
@ -1771,6 +1771,7 @@ 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
@ -1789,6 +1790,52 @@ 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.
@ -1807,6 +1854,7 @@ 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()
@ -1855,6 +1903,7 @@ 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)
}
@ -1862,6 +1911,7 @@ 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"
@ -1877,6 +1927,7 @@ 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
@ -1897,6 +1948,7 @@ 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)
}
@ -2074,11 +2126,24 @@ type RequestedDevice struct {
// Constraints are a set of constraints to apply when selecting the device
// to use.
Constraints []*Constraint
Constraints Constraints
// Affinities are a set of affinites to apply when selecting the device
// to use.
Affinities []*Affinity
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)
}
func (r *RequestedDevice) Copy() *RequestedDevice {
@ -2249,15 +2314,9 @@ func (n *NodeResources) Equals(o *NodeResources) bool {
if !n.Disk.Equals(&o.Disk) {
return false
}
if len(n.Networks) != len(o.Networks) {
if !n.Networks.Equals(&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) {
@ -2267,7 +2326,28 @@ func (n *NodeResources) Equals(o *NodeResources) bool {
return true
}
// DevicesEquals returns true if the two device arrays are equal
// 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
func DevicesEquals(d1, d2 []*NodeDeviceResource) bool {
if len(d1) != len(d2) {
return false
@ -6415,10 +6495,11 @@ type Constraint struct {
}
// Equal checks if two constraints are equal
func (c *Constraint) Equal(o *Constraint) bool {
return c.LTarget == o.LTarget &&
c.RTarget == o.RTarget &&
c.Operand == o.Operand
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) Copy() *Constraint {
@ -6494,6 +6575,29 @@ 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
@ -6504,11 +6608,12 @@ type Affinity struct {
}
// Equal checks if two affinities are equal
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) 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) Copy() *Affinity {
@ -6589,6 +6694,29 @@ 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