Skip to content

Commit

Permalink
Merge pull request #11167 from a-zagaevskiy/master
Browse files Browse the repository at this point in the history
Support configurable dynamic port range
  • Loading branch information
schmichael authored Oct 13, 2021
2 parents 1d30caa + fc89835 commit 6a0dede
Show file tree
Hide file tree
Showing 15 changed files with 280 additions and 32 deletions.
3 changes: 3 additions & 0 deletions .changelog/11167.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
client: Allow configuring minimum and maximum host ports used for dynamic ports
```
3 changes: 3 additions & 0 deletions api/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,9 @@ type NodeResources struct {
Disk NodeDiskResources
Networks []*NetworkResource
Devices []*NodeDeviceResource

MinDynamicPort int
MaxDynamicPort int
}

type NodeCpuResources struct {
Expand Down
3 changes: 3 additions & 0 deletions api/nodes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ func TestNodes_Info(t *testing.T) {
result.ID, result.Datacenter)
}

require.Equal(t, 20000, result.NodeResources.MinDynamicPort)
require.Equal(t, 32000, result.NodeResources.MaxDynamicPort)

// Check that the StatusUpdatedAt field is being populated correctly
if result.StatusUpdatedAt < startTime {
t.Fatalf("start time: %v, status updated: %v", startTime, result.StatusUpdatedAt)
Expand Down
22 changes: 22 additions & 0 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,18 @@ func (c *Client) init() error {

c.logger.Info("using alloc directory", "alloc_dir", c.config.AllocDir)

reserved := "<none>"
if c.config.Node != nil && c.config.Node.ReservedResources != nil {
// Node should always be non-nil due to initialization in the
// agent package, but don't risk a panic just for a long line.
reserved = c.config.Node.ReservedResources.Networks.ReservedHostPorts
}
c.logger.Info("using dynamic ports",
"min", c.config.MinDynamicPort,
"max", c.config.MaxDynamicPort,
"reserved", reserved,
)

// Ensure cgroups are created on linux platform
if runtime.GOOS == "linux" && c.cpusetManager != nil {
err := c.cpusetManager.Init()
Expand Down Expand Up @@ -1385,6 +1397,8 @@ func (c *Client) setupNode() error {
}
if node.NodeResources == nil {
node.NodeResources = &structs.NodeResources{}
node.NodeResources.MinDynamicPort = c.config.MinDynamicPort
node.NodeResources.MaxDynamicPort = c.config.MaxDynamicPort
}
if node.ReservedResources == nil {
node.ReservedResources = &structs.NodeReservedResources{}
Expand Down Expand Up @@ -1496,6 +1510,14 @@ func (c *Client) updateNodeFromFingerprint(response *fingerprint.FingerprintResp
c.config.Node.NodeResources.Merge(response.NodeResources)
nodeHasChanged = true
}

response.NodeResources.MinDynamicPort = c.config.MinDynamicPort
response.NodeResources.MaxDynamicPort = c.config.MaxDynamicPort
if c.config.Node.NodeResources.MinDynamicPort != response.NodeResources.MinDynamicPort ||
c.config.Node.NodeResources.MaxDynamicPort != response.NodeResources.MaxDynamicPort {
nodeHasChanged = true
}

}

if nodeHasChanged {
Expand Down
12 changes: 8 additions & 4 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -737,11 +737,15 @@ func TestClient_Init(t *testing.T) {
defer os.RemoveAll(dir)
allocDir := filepath.Join(dir, "alloc")

config := config.DefaultConfig()
config.AllocDir = allocDir
config.StateDBFactory = cstate.GetStateDBFactory(true)

// Node is always initialized in agent.go:convertClientConfig()
config.Node = mock.Node()

client := &Client{
config: &config.Config{
AllocDir: allocDir,
StateDBFactory: cstate.GetStateDBFactory(true),
},
config: config,
logger: testlog.HCLogger(t),
}

Expand Down
8 changes: 8 additions & 0 deletions client/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,12 @@ type Config struct {
// communicating with plugin subsystems over loopback
ClientMinPort uint

// MaxDynamicPort is the largest dynamic port generated
MaxDynamicPort int

// MinDynamicPort is the smallest dynamic port generated
MinDynamicPort int

// A mapping of directories on the host OS to attempt to embed inside each
// task's chroot.
ChrootEnv map[string]string
Expand Down Expand Up @@ -331,6 +337,8 @@ func DefaultConfig() *Config {
CNIInterfacePrefix: "eth",
HostNetworks: map[string]*structs.ClientHostNetworkConfig{},
CgroupParent: cgutil.DefaultCgroupParent,
MaxDynamicPort: structs.DefaultMinDynamicPort,
MinDynamicPort: structs.DefaultMaxDynamicPort,
}
}

Expand Down
2 changes: 2 additions & 0 deletions command/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,8 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {
}
conf.ClientMaxPort = uint(agentConfig.Client.ClientMaxPort)
conf.ClientMinPort = uint(agentConfig.Client.ClientMinPort)
conf.MaxDynamicPort = agentConfig.Client.MaxDynamicPort
conf.MinDynamicPort = agentConfig.Client.MinDynamicPort
conf.DisableRemoteExec = agentConfig.Client.DisableRemoteExec
if agentConfig.Client.TemplateConfig.FunctionBlacklist != nil {
conf.TemplateConfig.FunctionDenylist = agentConfig.Client.TemplateConfig.FunctionBlacklist
Expand Down
14 changes: 14 additions & 0 deletions command/agent/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
gatedwriter "github.com/hashicorp/nomad/helper/gated-writer"
"github.com/hashicorp/nomad/helper/logging"
"github.com/hashicorp/nomad/helper/winsvc"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/nomad/structs/config"
"github.com/hashicorp/nomad/version"
"github.com/mitchellh/cli"
Expand Down Expand Up @@ -372,6 +373,19 @@ func (c *Command) isValidConfig(config, cmdConfig *Config) bool {
return false
}

if config.Client.MinDynamicPort < 0 || config.Client.MinDynamicPort > structs.MaxValidPort {
c.Ui.Error(fmt.Sprintf("Invalid dynamic port range: min_dynamic_port=%d", config.Client.MinDynamicPort))
return false
}
if config.Client.MaxDynamicPort < 0 || config.Client.MaxDynamicPort > structs.MaxValidPort {
c.Ui.Error(fmt.Sprintf("Invalid dynamic port range: max_dynamic_port=%d", config.Client.MaxDynamicPort))
return false
}
if config.Client.MinDynamicPort > config.Client.MaxDynamicPort {
c.Ui.Error(fmt.Sprintf("Invalid dynamic port range: min_dynamic_port=%d and max_dynamic_port=%d", config.Client.MinDynamicPort, config.Client.MaxDynamicPort))
return false
}

if !config.DevMode {
// Ensure that we have the directories we need to run.
if config.Server.Enabled && config.DataDir == "" {
Expand Down
143 changes: 143 additions & 0 deletions command/agent/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ package agent

import (
"io/ioutil"
"math"
"os"
"path/filepath"
"strings"
"testing"

"github.com/mitchellh/cli"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/hashicorp/nomad/version"
)
Expand Down Expand Up @@ -241,3 +244,143 @@ func TestCommand_NullCharInRegion(t *testing.T) {
}
}
}

// TestIsValidConfig asserts that invalid configurations return false.
func TestIsValidConfig(t *testing.T) {

cases := []struct {
name string
conf Config // merged into DefaultConfig()

// err should appear in error output; success expected if err
// is empty
err string
}{
{
name: "Default",
conf: Config{
DataDir: "/tmp",
Client: &ClientConfig{Enabled: true},
},
},
{
name: "NoMode",
conf: Config{
Client: &ClientConfig{Enabled: false},
Server: &ServerConfig{Enabled: false},
},
err: "Must specify either",
},
{
name: "InvalidRegion",
conf: Config{
Client: &ClientConfig{
Enabled: true,
},
Region: "Hello\000World",
},
err: "Region contains",
},
{
name: "InvalidDatacenter",
conf: Config{
Client: &ClientConfig{
Enabled: true,
},
Datacenter: "Hello\000World",
},
err: "Datacenter contains",
},
{
name: "RelativeDir",
conf: Config{
Client: &ClientConfig{
Enabled: true,
},
DataDir: "foo/bar",
},
err: "must be given as an absolute",
},
{
name: "NegativeMinDynamicPort",
conf: Config{
Client: &ClientConfig{
Enabled: true,
MinDynamicPort: -1,
},
},
err: "min_dynamic_port",
},
{
name: "NegativeMaxDynamicPort",
conf: Config{
Client: &ClientConfig{
Enabled: true,
MaxDynamicPort: -1,
},
},
err: "max_dynamic_port",
},
{
name: "BigMinDynamicPort",
conf: Config{
Client: &ClientConfig{
Enabled: true,
MinDynamicPort: math.MaxInt32,
},
},
err: "min_dynamic_port",
},
{
name: "BigMaxDynamicPort",
conf: Config{
Client: &ClientConfig{
Enabled: true,
MaxDynamicPort: math.MaxInt32,
},
},
err: "max_dynamic_port",
},
{
name: "MinMaxDynamicPortSwitched",
conf: Config{
Client: &ClientConfig{
Enabled: true,
MinDynamicPort: 5000,
MaxDynamicPort: 4000,
},
},
err: "and max",
},
{
name: "DynamicPortOk",
conf: Config{
DataDir: "/tmp",
Client: &ClientConfig{
Enabled: true,
MinDynamicPort: 4000,
MaxDynamicPort: 5000,
},
},
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
mui := cli.NewMockUi()
cmd := &Command{Ui: mui}
config := DefaultConfig().Merge(&tc.conf)
result := cmd.isValidConfig(config, DefaultConfig())
if tc.err == "" {
// No error expected
assert.True(t, result, mui.ErrorWriter.String())
return
}

// Error expected
assert.False(t, result)
require.Contains(t, mui.ErrorWriter.String(), tc.err)
t.Logf("%s returned: %s", tc.name, mui.ErrorWriter.String())
})
}
}
16 changes: 16 additions & 0 deletions command/agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,14 @@ type ClientConfig struct {
// communicating with plugin subsystems
ClientMinPort int `hcl:"client_min_port"`

// MaxDynamicPort is the upper range of the dynamic ports that the client
// uses for allocations
MaxDynamicPort int `hcl:"max_dynamic_port"`

// MinDynamicPort is the lower range of the dynamic ports that the client
// uses for allocations
MinDynamicPort int `hcl:"min_dynamic_port"`

// Reserved is used to reserve resources from being used by Nomad. This can
// be used to target a certain utilization or to prevent Nomad from using a
// particular set of ports.
Expand Down Expand Up @@ -923,6 +931,8 @@ func DefaultConfig() *Config {
MaxKillTimeout: "30s",
ClientMinPort: 14000,
ClientMaxPort: 14512,
MinDynamicPort: 20000,
MaxDynamicPort: 32000,
Reserved: &Resources{},
GCInterval: 1 * time.Minute,
GCParallelDestroys: 2,
Expand Down Expand Up @@ -1610,6 +1620,12 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig {
if b.ClientMinPort != 0 {
result.ClientMinPort = b.ClientMinPort
}
if b.MaxDynamicPort != 0 {
result.MaxDynamicPort = b.MaxDynamicPort
}
if b.MinDynamicPort != 0 {
result.MinDynamicPort = b.MinDynamicPort
}
if result.Reserved == nil && b.Reserved != nil {
reserved := *b.Reserved
result.Reserved = &reserved
Expand Down
4 changes: 4 additions & 0 deletions command/agent/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ func TestConfig_Merge(t *testing.T) {
},
NetworkSpeed: 100,
CpuCompute: 100,
MinDynamicPort: 10001,
MaxDynamicPort: 10002,
MemoryMB: 100,
MaxKillTimeout: "20s",
ClientMaxPort: 19996,
Expand Down Expand Up @@ -292,6 +294,8 @@ func TestConfig_Merge(t *testing.T) {
ClientMinPort: 22000,
NetworkSpeed: 105,
CpuCompute: 105,
MinDynamicPort: 10002,
MaxDynamicPort: 10003,
MemoryMB: 105,
MaxKillTimeout: "50s",
DisableRemoteExec: false,
Expand Down
Loading

0 comments on commit 6a0dede

Please sign in to comment.