Get structs tests passing

command/agent/agent.go

@@ -617,6 +617,20 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {
 	}
 	conf.HostVolumes = hvMap
 
+	// Ensure only one of reserved.reserved_ports or
+	// host_network.reserved_ports is set.
+	/*
+		if globalRes := agentConfig.Client.Reserved.ReservedPorts; globalRes != "" {
+			for _, hostnet := range agentConfig.Client.HostNetworks {
+				if hostRes := hostnet.ReservedPorts; hostRes != "" {
+					// Global and network-specific reserved ports
+					// aren't allowed.
+					return nil, fmt.Errorf("Cannot specify reserved.reserved_ports (%q) and host_network[%q].reserved port.", globalRes, hostRes)
+				}
+			}
+		}
+	*/
+
 	// Setup the node
 	conf.Node = new(structs.Node)
 	conf.Node.Datacenter = agentConfig.Datacenter

command/agent/command.go

@@ -399,11 +399,23 @@ func (c *Command) IsValidConfig(config, cmdConfig *Config) bool {
 	}
 
 	for _, hn := range config.Client.HostNetworks {
+		if hn.ReservedPorts == "" {
+			continue
+		}
+
+		// Ensure port range is valid
 		if _, err := structs.ParsePortRanges(hn.ReservedPorts); err != nil {
 			c.Ui.Error(fmt.Sprintf("host_network[%q].reserved_ports %q invalid: %v",
 				hn.Name, hn.ReservedPorts, err))
 			return false
 		}
+
+		// Ensure ports aren't reserved multiple places. Ideally the global configuration would be the default.gT
+		/*
+			if config.Client.Reserved.ReservedPorts != "" {
+					return nil, fmt.Errorf("Cannot specify reserved.reserved_ports (%q) and host_network[%q].reserved port.", globalRes, hostRes)
+			}
+		*/
 	}
 
 	if err := config.Client.Artifact.Validate(); err != nil {

nomad/structs/network.go

@@ -207,22 +207,6 @@ func (idx *NetworkIndex) SetNode(node *Node) error {
 		taskNetworks = node.Resources.Networks
 	}
 
-	// nodeNetworks are used for group.network asks.
-	var nodeNetworks []*NodeNetworkResource
-	if node.NodeResources != nil && len(node.NodeResources.NodeNetworks) != 0 {
-		nodeNetworks = node.NodeResources.NodeNetworks
-	}
-
-	// Filter task networks down to those with a device. For example
-	// taskNetworks may contain a "bridge" interface which has no device
-	// set and cannot be used to fulfill asks.
-	for _, n := range taskNetworks {
-		if n.Device != "" {
-			idx.TaskNetworks = append(idx.TaskNetworks, n)
-			idx.AvailBandwidth[n.Device] = n.MBits
-		}
-	}
-
 	// Reserved ports get merged downward. For example given an agent
 	// config:
 	//
@@ -231,8 +215,8 @@ func (idx *NetworkIndex) SetNode(node *Node) error {
 	// client.host_network["eth1"] = {reserved_ports = "1-1000"}
 	//
 	// Addresses on taskNetworks reserve port 22
-	// Addresses on eth0 reserve 22,80,443 (note that 22 is included!)
-	// Addresses on eth1 reserve 1-1000    (22 already included in this range)
+	// Addresses on eth0 reserve 22,80,443 (note 22 is also reserved!)
+	// Addresses on eth1 reserve 1-1000
 	globalResPorts := []uint{}
 
 	// COMPAT(0.11): Remove in 0.11
@@ -252,6 +236,7 @@ func (idx *NetworkIndex) SetNode(node *Node) error {
 		}
 	} else if node.Reserved != nil {
 		for _, n := range node.Reserved.Networks {
+			used := idx.getUsedPortsFor(n.IP)
 			for _, ports := range [][]Port{n.ReservedPorts, n.DynamicPorts} {
 				for _, p := range ports {
 					if p.Value > MaxValidPort || p.Value < 0 {
@@ -260,14 +245,41 @@ func (idx *NetworkIndex) SetNode(node *Node) error {
 						// by validation upstream.
 						return fmt.Errorf("invalid port %d for reserved_ports", p.Value)
 					}
+
 					globalResPorts = append(globalResPorts, uint(p.Value))
+					used.Set(uint(p.Value))
 				}
 			}
+
+			// Reserve mbits
+			if n.Device != "" {
+				idx.UsedBandwidth[n.Device] += n.MBits
+			}
 		}
 	}
 
-	// TODO: upgrade path?
-	// is it possible to get duplicates here?
+	// Filter task networks down to those with a device. For example
+	// taskNetworks may contain a "bridge" interface which has no device
+	// set and cannot be used to fulfill asks.
+	for _, n := range taskNetworks {
+		if n.Device != "" {
+			idx.TaskNetworks = append(idx.TaskNetworks, n)
+			idx.AvailBandwidth[n.Device] = n.MBits
+		}
+
+		// Reserve ports
+		used := idx.getUsedPortsFor(n.IP)
+		for _, p := range globalResPorts {
+			used.Set(p)
+		}
+	}
+
+	// nodeNetworks are used for group.network asks.
+	var nodeNetworks []*NodeNetworkResource
+	if node.NodeResources != nil && len(node.NodeResources.NodeNetworks) != 0 {
+		nodeNetworks = node.NodeResources.NodeNetworks
+	}
+
 	for _, n := range nodeNetworks {
 		for _, a := range n.Addresses {
 			// Index host networks by their unique alias for asks
@@ -474,7 +486,7 @@ func incIP(ip net.IP) {
 // AssignPorts based on an ask from the scheduler processing a group.network
 // stanza. Supports multi-interfaces through node configured host_networks.
 //
-// AssignNetwork supports the deprecated task.network stanza.
+// AssignTaskNetwork supports the deprecated task.resources.network stanza.
 func (idx *NetworkIndex) AssignPorts(ask *NetworkResource) (AllocatedPorts, error) {
 	var offer AllocatedPorts
 

nomad/structs/network_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/nomad/ci"
+	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/require"
 )
 
@@ -189,20 +190,10 @@ func TestNetworkIndex_SetNode(t *testing.T) {
 			},
 		},
 	}
-	collide, reason := idx.SetNode(n)
-	if collide || reason != "" {
-		t.Fatalf("bad")
-	}
-
-	if len(idx.AvailNetworks) != 1 {
-		t.Fatalf("Bad")
-	}
-	if idx.AvailBandwidth["eth0"] != 1000 {
-		t.Fatalf("Bad")
-	}
-	if !idx.UsedPorts["192.168.0.100"].Check(22) {
-		t.Fatalf("Bad")
-	}
+	must.NoError(t, idx.SetNode(n))
+	must.Len(t, 1, idx.TaskNetworks)
+	must.Eq(t, idx.AvailBandwidth["eth0"], 1000)
+	must.True(t, idx.UsedPorts["192.168.0.100"].Check(22))
 }
 
 func TestNetworkIndex_AddAllocs(t *testing.T) {
@@ -327,7 +318,7 @@ func TestNetworkIndex_yieldIP(t *testing.T) {
 	}
 }
 
-func TestNetworkIndex_AssignNetwork(t *testing.T) {
+func TestNetworkIndex_AssignTaskNetwork(t *testing.T) {
 	ci.Parallel(t)
 	idx := NewNetworkIndex()
 	n := &Node{
@@ -379,7 +370,7 @@ func TestNetworkIndex_AssignNetwork(t *testing.T) {
 	ask := &NetworkResource{
 		ReservedPorts: []Port{{"main", 8000, 0, ""}},
 	}
-	offer, err := idx.AssignNetwork(ask)
+	offer, err := idx.AssignTaskNetwork(ask)
 	require.NoError(t, err)
 	require.NotNil(t, offer)
 	require.Equal(t, "192.168.0.101", offer.IP)
@@ -391,7 +382,7 @@ func TestNetworkIndex_AssignNetwork(t *testing.T) {
 	ask = &NetworkResource{
 		DynamicPorts: []Port{{"http", 0, 80, ""}, {"https", 0, 443, ""}, {"admin", 0, -1, ""}},
 	}
-	offer, err = idx.AssignNetwork(ask)
+	offer, err = idx.AssignTaskNetwork(ask)
 	require.NoError(t, err)
 	require.NotNil(t, offer)
 	require.Equal(t, "192.168.0.100", offer.IP)
@@ -410,7 +401,7 @@ func TestNetworkIndex_AssignNetwork(t *testing.T) {
 		ReservedPorts: []Port{{"main", 2345, 0, ""}},
 		DynamicPorts:  []Port{{"http", 0, 80, ""}, {"https", 0, 443, ""}, {"admin", 0, 8080, ""}},
 	}
-	offer, err = idx.AssignNetwork(ask)
+	offer, err = idx.AssignTaskNetwork(ask)
 	require.NoError(t, err)
 	require.NotNil(t, offer)
 	require.Equal(t, "192.168.0.100", offer.IP)
@@ -423,15 +414,15 @@ func TestNetworkIndex_AssignNetwork(t *testing.T) {
 	ask = &NetworkResource{
 		MBits: 1000,
 	}
-	offer, err = idx.AssignNetwork(ask)
+	offer, err = idx.AssignTaskNetwork(ask)
 	require.Error(t, err)
 	require.Equal(t, "bandwidth exceeded", err.Error())
 	require.Nil(t, offer)
 }
 
 // This test ensures that even with a small domain of available ports we are
 // able to make a dynamic port allocation.
-func TestNetworkIndex_AssignNetwork_Dynamic_Contention(t *testing.T) {
+func TestNetworkIndex_AssignTaskNetwork_Dynamic_Contention(t *testing.T) {
 	ci.Parallel(t)
 
 	// Create a node that only has one free port
@@ -459,7 +450,7 @@ func TestNetworkIndex_AssignNetwork_Dynamic_Contention(t *testing.T) {
 	ask := &NetworkResource{
 		DynamicPorts: []Port{{"http", 0, 80, ""}},
 	}
-	offer, err := idx.AssignNetwork(ask)
+	offer, err := idx.AssignTaskNetwork(ask)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -503,23 +494,11 @@ func TestNetworkIndex_SetNode_Old(t *testing.T) {
 			},
 		},
 	}
-	collide, reason := idx.SetNode(n)
-	if collide || reason != "" {
-		t.Fatalf("bad")
-	}
-
-	if len(idx.AvailNetworks) != 1 {
-		t.Fatalf("Bad")
-	}
-	if idx.AvailBandwidth["eth0"] != 1000 {
-		t.Fatalf("Bad")
-	}
-	if idx.UsedBandwidth["eth0"] != 1 {
-		t.Fatalf("Bad")
-	}
-	if !idx.UsedPorts["192.168.0.100"].Check(22) {
-		t.Fatalf("Bad")
-	}
+	must.NoError(t, idx.SetNode(n))
+	must.Len(t, 1, idx.TaskNetworks)
+	must.Eq(t, idx.AvailBandwidth["eth0"], 1000)
+	must.Eq(t, idx.UsedBandwidth["eth0"], 1)
+	must.True(t, idx.UsedPorts["192.168.0.100"].Check(22))
 }
 
 // COMPAT(0.11): Remove in 0.11
@@ -618,7 +597,7 @@ func TestNetworkIndex_yieldIP_Old(t *testing.T) {
 }
 
 // COMPAT(0.11): Remove in 0.11
-func TestNetworkIndex_AssignNetwork_Old(t *testing.T) {
+func TestNetworkIndex_AssignTaskNetwork_Old(t *testing.T) {
 	ci.Parallel(t)
 
 	idx := NewNetworkIndex()
@@ -681,7 +660,7 @@ func TestNetworkIndex_AssignNetwork_Old(t *testing.T) {
 	ask := &NetworkResource{
 		ReservedPorts: []Port{{"main", 8000, 0, ""}},
 	}
-	offer, err := idx.AssignNetwork(ask)
+	offer, err := idx.AssignTaskNetwork(ask)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -700,7 +679,7 @@ func TestNetworkIndex_AssignNetwork_Old(t *testing.T) {
 	ask = &NetworkResource{
 		DynamicPorts: []Port{{"http", 0, 80, ""}, {"https", 0, 443, ""}, {"admin", 0, 8080, ""}},
 	}
-	offer, err = idx.AssignNetwork(ask)
+	offer, err = idx.AssignTaskNetwork(ask)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -724,7 +703,7 @@ func TestNetworkIndex_AssignNetwork_Old(t *testing.T) {
 		ReservedPorts: []Port{{"main", 2345, 0, ""}},
 		DynamicPorts:  []Port{{"http", 0, 80, ""}, {"https", 0, 443, ""}, {"admin", 0, 8080, ""}},
 	}
-	offer, err = idx.AssignNetwork(ask)
+	offer, err = idx.AssignTaskNetwork(ask)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -744,7 +723,7 @@ func TestNetworkIndex_AssignNetwork_Old(t *testing.T) {
 	ask = &NetworkResource{
 		MBits: 1000,
 	}
-	offer, err = idx.AssignNetwork(ask)
+	offer, err = idx.AssignTaskNetwork(ask)
 	if err.Error() != "bandwidth exceeded" {
 		t.Fatalf("err: %v", err)
 	}
@@ -756,7 +735,7 @@ func TestNetworkIndex_AssignNetwork_Old(t *testing.T) {
 // COMPAT(0.11): Remove in 0.11
 // This test ensures that even with a small domain of available ports we are
 // able to make a dynamic port allocation.
-func TestNetworkIndex_AssignNetwork_Dynamic_Contention_Old(t *testing.T) {
+func TestNetworkIndex_AssignTaskNetwork_Dynamic_Contention_Old(t *testing.T) {
 	ci.Parallel(t)
 
 	// Create a node that only has one free port
@@ -791,7 +770,7 @@ func TestNetworkIndex_AssignNetwork_Dynamic_Contention_Old(t *testing.T) {
 	ask := &NetworkResource{
 		DynamicPorts: []Port{{"http", 0, 80, ""}},
 	}
-	offer, err := idx.AssignNetwork(ask)
+	offer, err := idx.AssignTaskNetwork(ask)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -811,7 +790,7 @@ func TestNetworkIndex_AssignNetwork_Dynamic_Contention_Old(t *testing.T) {
 
 func TestIntContains(t *testing.T) {
 	ci.Parallel(t)
-	
+
 	l := []int{1, 2, 10, 20}
 	if isPortReserved(l, 50) {
 		t.Fatalf("bad")

website/content/docs/configuration/client.mdx

@@ -206,9 +206,12 @@ chroot as doing so would cause infinite recursion.
 
 - `disk` `(int: 0)` - Specifies the amount of disk to reserve, in MB.
 
-- `reserved_ports` `(string: "")` - Specifies a comma-separated list of ports to
-  reserve on all fingerprinted network devices. Ranges can be specified by using
-  a hyphen separated the two inclusive ends.
+- `reserved_ports` `(string: "")` - Specifies a comma-separated list of ports
+  to reserve on all fingerprinted network devices. Ranges can be specified by
+  using a hyphen separated the two inclusive ends. See also
+  [`host_network`](#host_network-stanza) for reserving ports on specific host
+  networks.
+
 
 ### `artifact` Parameters
 
@@ -396,8 +399,10 @@ client {
 - `interface` `(string: "")` - Filters searching of addresses to a specific interface.
 
 - `reserved_ports` `(string: "")` - Specifies a comma-separated list of ports to
-  reserve on all fingerprinted network devices. Ranges can be specified by using
+  reserve on all addresses associated with this network. Ranges can be specified by using
   a hyphen separating the two inclusive ends.
+  [`reserved.reserved_ports`](#reserved_ports) are also reserved on each host
+  network.
 
 ## `client` Examples