diff --git a/nomad/structs/funcs.go b/nomad/structs/funcs.go index 513b10cdc..4d2bae575 100644 --- a/nomad/structs/funcs.go +++ b/nomad/structs/funcs.go @@ -41,24 +41,11 @@ func FilterTerminalAllocs(allocs []*Allocation) []*Allocation { return allocs[:n] } -// PortsOvercommited checks if any ports are over-committed. -// This does not handle CIDR subsets, and computes for the entire -// CIDR block currently. -func PortsOvercommited(r *Resources) bool { - for _, net := range r.Networks { - ports := make(map[int]struct{}) - for _, port := range net.ReservedPorts { - if _, ok := ports[port]; ok { - return true - } - ports[port] = struct{}{} - } - } - return false -} - -// AllocsFit checks if a given set of allocations will fit on a node -func AllocsFit(node *Node, allocs []*Allocation) (bool, *Resources, error) { +// AllocsFit checks if a given set of allocations will fit on a node. +// The netIdx can optionally be provided if its already been computed. +// If the netIdx is provided, it is assumed that the client has already +// ensured there are no collisions. +func AllocsFit(node *Node, allocs []*Allocation, netIdx *NetworkIndex) (bool, *Resources, error) { // Compute the utilization from zero used := new(Resources) @@ -82,8 +69,16 @@ func AllocsFit(node *Node, allocs []*Allocation) (bool, *Resources, error) { return false, used, nil } - // Ensure ports are not over commited - if PortsOvercommited(used) { + // Create the network index if missing + if netIdx == nil { + netIdx = NewNetworkIndex() + if netIdx.SetNode(node) || netIdx.AddAllocs(allocs) { + return false, used, nil + } + } + + // Check if the network is overcommitted + if netIdx.Overcommitted() { return false, used, nil } diff --git a/nomad/structs/funcs_test.go b/nomad/structs/funcs_test.go index 4411f7728..06899b151 100644 --- a/nomad/structs/funcs_test.go +++ b/nomad/structs/funcs_test.go @@ -39,25 +39,48 @@ func TestFilterTerminalALlocs(t *testing.T) { } } -func TestPortsOvercommitted(t *testing.T) { - r := &Resources{ - Networks: []*NetworkResource{ - &NetworkResource{ - ReservedPorts: []int{22, 80}, - }, - &NetworkResource{ - ReservedPorts: []int{22, 80}, +func TestAllocsFit_PortsOvercommitted(t *testing.T) { + n := &Node{ + Resources: &Resources{ + Networks: []*NetworkResource{ + &NetworkResource{ + CIDR: "10.0.0.0/8", + MBits: 100, + }, }, }, } - if PortsOvercommited(r) { - t.Fatalf("bad") + + a1 := &Allocation{ + TaskResources: map[string]*Resources{ + "web": &Resources{ + Networks: []*NetworkResource{ + &NetworkResource{ + IP: "10.0.0.1", + MBits: 50, + ReservedPorts: []int{8000}, + }, + }, + }, + }, } - // Overcommit 22 - r.Networks[1].ReservedPorts[1] = 22 - if !PortsOvercommited(r) { - t.Fatalf("bad") + // Should fit one allocation + fit, _, err := AllocsFit(n, []*Allocation{a1}, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + if !fit { + t.Fatalf("Bad") + } + + // Should not fit second allocation + fit, _, err = AllocsFit(n, []*Allocation{a1, a1}, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + if fit { + t.Fatalf("Bad") } } @@ -82,7 +105,7 @@ func TestAllocsFit(t *testing.T) { IOPS: 50, Networks: []*NetworkResource{ &NetworkResource{ - CIDR: "10.0.0.0/8", + IP: "10.0.0.1", MBits: 50, ReservedPorts: []int{80}, }, @@ -98,7 +121,7 @@ func TestAllocsFit(t *testing.T) { IOPS: 50, Networks: []*NetworkResource{ &NetworkResource{ - CIDR: "10.0.0.0/8", + IP: "10.0.0.1", MBits: 50, ReservedPorts: []int{8000}, }, @@ -107,7 +130,7 @@ func TestAllocsFit(t *testing.T) { } // Should fit one allocation - fit, used, err := AllocsFit(n, []*Allocation{a1}) + fit, used, err := AllocsFit(n, []*Allocation{a1}, nil) if err != nil { t.Fatalf("err: %v", err) } @@ -124,7 +147,7 @@ func TestAllocsFit(t *testing.T) { } // Should not fit second allocation - fit, used, err = AllocsFit(n, []*Allocation{a1, a1}) + fit, used, err = AllocsFit(n, []*Allocation{a1, a1}, nil) if err != nil { t.Fatalf("err: %v", err) } diff --git a/nomad/structs/network.go b/nomad/structs/network.go index f1314cf7d..39292e831 100644 --- a/nomad/structs/network.go +++ b/nomad/structs/network.go @@ -35,8 +35,20 @@ func NewNetworkIndex() *NetworkIndex { } } -// SetNode is used to setup the available network resources -func (idx *NetworkIndex) SetNode(node *Node) { +// Overcommitted checks if the network is overcommitted +func (idx *NetworkIndex) Overcommitted() bool { + for device, used := range idx.UsedBandwidth { + avail := idx.AvailBandwidth[device] + if used > avail { + return true + } + } + return false +} + +// SetNode is used to setup the available network resources. Returns +// true if there is a collision +func (idx *NetworkIndex) SetNode(node *Node) (collide bool) { // Add the available CIDR blocks for _, n := range node.Resources.Networks { if n.CIDR != "" { @@ -48,26 +60,34 @@ func (idx *NetworkIndex) SetNode(node *Node) { // Add the reserved resources if r := node.Reserved; r != nil { for _, n := range r.Networks { - idx.AddReserved(n) + if idx.AddReserved(n) { + collide = true + } } } + return } -// AddAllocs is used to add the used network resources -func (idx *NetworkIndex) AddAllocs(allocs []*Allocation) { +// AddAllocs is used to add the used network resources. Returns +// true if there is a collision +func (idx *NetworkIndex) AddAllocs(allocs []*Allocation) (collide bool) { for _, alloc := range allocs { for _, task := range alloc.TaskResources { if len(task.Networks) == 0 { continue } n := task.Networks[0] - idx.AddReserved(n) + if idx.AddReserved(n) { + collide = true + } } } + return } -// AddReserved is used to add a reserved network usage -func (idx *NetworkIndex) AddReserved(n *NetworkResource) { +// AddReserved is used to add a reserved network usage, returns true +// if there is a port collision +func (idx *NetworkIndex) AddReserved(n *NetworkResource) (collide bool) { // Add the port usage used := idx.UsedPorts[n.IP] if used == nil { @@ -75,11 +95,16 @@ func (idx *NetworkIndex) AddReserved(n *NetworkResource) { idx.UsedPorts[n.IP] = used } for _, port := range n.ReservedPorts { - used[port] = struct{}{} + if _, ok := used[port]; ok { + collide = true + } else { + used[port] = struct{}{} + } } // Add the bandwidth idx.UsedBandwidth[n.Device] += n.MBits + return } // yieldIP is used to iteratively invoke the callback with @@ -107,7 +132,8 @@ func (idx *NetworkIndex) yieldIP(cb func(net *NetworkResource, ip net.IP) bool) } } -// AssignNetwork is used to assign network resources given an ask +// AssignNetwork is used to assign network resources given an ask. +// If the ask cannot be satisfied, returns nil func (idx *NetworkIndex) AssignNetwork(ask *NetworkResource) (out *NetworkResource) { idx.yieldIP(func(n *NetworkResource, ip net.IP) (stop bool) { // Convert the IP to a string diff --git a/nomad/structs/network_test.go b/nomad/structs/network_test.go index eb7094d75..802964472 100644 --- a/nomad/structs/network_test.go +++ b/nomad/structs/network_test.go @@ -6,6 +6,48 @@ import ( "testing" ) +func TestNetworkIndex_Overcommitted(t *testing.T) { + idx := NewNetworkIndex() + + // Consume some network + reserved := &NetworkResource{ + Device: "eth0", + IP: "192.168.0.100", + MBits: 505, + ReservedPorts: []int{8000, 9000}, + } + collide := idx.AddReserved(reserved) + if collide { + t.Fatalf("bad") + } + if !idx.Overcommitted() { + t.Fatalf("have no resources") + } + + // Add resources + n := &Node{ + Resources: &Resources{ + Networks: []*NetworkResource{ + &NetworkResource{ + Device: "eth0", + CIDR: "192.168.0.100/32", + MBits: 1000, + }, + }, + }, + } + idx.SetNode(n) + if idx.Overcommitted() { + t.Fatalf("have resources") + } + + // Double up our ussage + idx.AddReserved(reserved) + if !idx.Overcommitted() { + t.Fatalf("should be overcommitted") + } +} + func TestNetworkIndex_SetNode(t *testing.T) { idx := NewNetworkIndex() n := &Node{ @@ -29,7 +71,10 @@ func TestNetworkIndex_SetNode(t *testing.T) { }, }, } - idx.SetNode(n) + collide := idx.SetNode(n) + if collide { + t.Fatalf("bad") + } if len(idx.AvailNetworks) != 1 { t.Fatalf("Bad") @@ -77,7 +122,10 @@ func TestNetworkIndex_AddAllocs(t *testing.T) { }, }, } - idx.AddAllocs(allocs) + collide := idx.AddAllocs(allocs) + if collide { + t.Fatalf("bad") + } if idx.UsedBandwidth["eth0"] != 70 { t.Fatalf("Bad") @@ -102,7 +150,10 @@ func TestNetworkIndex_AddReserved(t *testing.T) { MBits: 20, ReservedPorts: []int{8000, 9000}, } - idx.AddReserved(reserved) + collide := idx.AddReserved(reserved) + if collide { + t.Fatalf("bad") + } if idx.UsedBandwidth["eth0"] != 20 { t.Fatalf("Bad") @@ -113,6 +164,12 @@ func TestNetworkIndex_AddReserved(t *testing.T) { if _, ok := idx.UsedPorts["192.168.0.100"][9000]; !ok { t.Fatalf("Bad") } + + // Try to reserve the same network + collide = idx.AddReserved(reserved) + if !collide { + t.Fatalf("bad") + } } func TestNetworkIndex_yieldIP(t *testing.T) {