diff --git a/command/agent/agent.go b/command/agent/agent.go index 0c8217d3545..a642fbc212e 100644 --- a/command/agent/agent.go +++ b/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 diff --git a/command/agent/command.go b/command/agent/command.go index eef8e97d2e0..50082276b5a 100644 --- a/command/agent/command.go +++ b/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 { diff --git a/nomad/structs/network.go b/nomad/structs/network.go index 530f8e6ba17..264ffda501e 100644 --- a/nomad/structs/network.go +++ b/nomad/structs/network.go @@ -181,7 +181,7 @@ func (idx *NetworkIndex) Overcommitted() bool { // fingerprinting. // // SetNode must be idempotent as preemption causes SetNode to be called -// multiple times on the same NetworkIndex, only clearing UsedPorts being +// multiple times on the same NetworkIndex, only clearing UsedPorts between // calls. // // An error is returned if the Node cannot produce a consistent NetworkIndex @@ -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 diff --git a/nomad/structs/network_test.go b/nomad/structs/network_test.go index ccb2900c10b..e9904e5b69b 100644 --- a/nomad/structs/network_test.go +++ b/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,7 +414,7 @@ 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) @@ -431,7 +422,7 @@ func TestNetworkIndex_AssignNetwork(t *testing.T) { // 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") diff --git a/website/content/docs/configuration/client.mdx b/website/content/docs/configuration/client.mdx index a4a6ea8e5a3..d0b5c5f3c94 100644 --- a/website/content/docs/configuration/client.mdx +++ b/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