Skip to content

Commit

Permalink
pass 2: move to agent and only agent, errors only
Browse files Browse the repository at this point in the history
  • Loading branch information
@schmichael
schmichael committed Jan 12, 2022
1 parent 0f84549 commit 482d975
Show file tree
Hide file tree
Showing 7 changed files with 276 additions and 182 deletions.
172 changes: 0 additions & 172 deletions client/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"
"fmt"
"io"
"net"
"os"
"reflect"
"strconv"
Expand Down Expand Up @@ -916,174 +915,3 @@ func (c *Config) NomadPluginConfig() *base.AgentConfig {
},
}
}

// Validate Client Agent Configuration. Returns a multierror.Error. Fields are
// refenced by their snake_case/HCL naming convention to match documentation.
//
// Must be called on an initialized Config such as generated by DefaultConfig.
//
// Many fields that are shared with server agents are not validated here such
// as:
//
// StateDir
// AllocDir
// ConsulConfig
// VaultConfig
// StatsCollectionInterval
// TLSConfig
// LogLevel
//
// NetworkInterface is not validated.
func (c *Config) Validate() *helper.ValidationResults {
results := helper.NewValidationResults()

if c.Region == "" {
results.AppendErrorf("Missing region")
} else {
//TODO Emit warning if region name is not a valid hostname
}

if n := c.NetworkSpeed; n < 0 {
results.AppendErrorf("network_speed must be >= 0 but found: %v", n)
}

if n := c.CpuCompute; n < 0 {
results.AppendErrorf("cpu_total_compute must be >= 0 but found: %v", n)
}

if n := c.MemoryMB; n < 0 {
results.AppendErrorf("memory_total_mb must be >= 0 but found: %v", n)
}

if n := c.MaxKillTimeout; n < 0 {
results.AppendErrorf("max_kill_timeout must be >= 0 but found: %v", n)
}

for i, s := range c.Servers {
_, _, err := net.SplitHostPort(s)
if err == nil {
continue
}
results.AppendErrorf("servers[%d] invalid: %v", i, err)
}

c.validateNode(results, c.Node)

if c.MaxDynamicPort > structs.MaxValidPort {
results.AppendErrorf("max_dynamic_port must be <= %d but found %d", structs.MaxValidPort, c.MaxDynamicPort)
}

if c.MaxDynamicPort == 0 {
results.AppendErrorf("max_dynamic_port must be > 0")
}

if c.MinDynamicPort > structs.MaxValidPort {
results.AppendErrorf("min_dynamic_port must be <= %d but found %d", structs.MaxValidPort, c.MinDynamicPort)
}

if c.MinDynamicPort == 0 {
results.AppendErrorf("min_dynamic_port must be > 0")
}

if c.MinDynamicPort > c.MaxDynamicPort {
results.AppendErrorf("min_dynamic_port (%d) must be < max_dynamic_port (%d)", c.MinDynamicPort, c.MaxDynamicPort)
}

//TODO Validate ChrootEnv

if c.GCInterval <= 0 {
results.AppendErrorf("gc_interval must be > 0 but found %s", c.GCInterval)
}

if c.GCParallelDestroys <= 0 {
results.AppendErrorf("gc_parallel_destroys must be > 0 but found %d", c.GCParallelDestroys)
}

if c.GCDiskUsageThreshold <= 0 {
results.AppendErrorf("gc_disk_usage_threshold must be > 0 but found %f", c.GCDiskUsageThreshold)
}

if c.GCInodeUsageThreshold <= 0 {
results.AppendErrorf("gc_inode_usage_threshold must be > 0 but found %f", c.GCInodeUsageThreshold)
}

if c.GCMaxAllocs <= 0 {
results.AppendErrorf("gc_max_allocs must be > 0 but found %d", c.GCMaxAllocs)
}

panic("TODO start at ACLTokenTTL")

return results
}

// validateNode validates the subset of structs.Node fields configured for
// client agents.
func (c *Config) validateNode(results *helper.ValidationResults, node *structs.Node) {
if node == nil {
// Node should be statically initialized so a failure here is a coding error.
results.AppendErrorf("Node should be initialized. Please report a bug.")
return
}

if node.Datacenter == "" {
// Datacenter should be statically initialized so a failure here is a coding error.
results.AppendErrorf("datacenter must be set.")
} else {
//TODO Emit warning if Datacenter is not a valid hostname
}

if node.Name == "" {
results.AppendErrorf("name must be set.")
}

if _, _, err := net.SplitHostPort(node.HTTPAddr); err != nil {
// Should be unreachable due to agent's address normalization,
// but doesn't hurt to double check.
results.AppendErrorf("http address invalid: %v", err)
}

c.validateNodeReserved(results, node.ReservedResources)
}

// validateNodeReserved validates the node reserved resources for client
// agents.
//
// Node.Reserved is deprecated but until all references are removed it
// should be validated.
func (c *Config) validateNodeReserved(results *helper.ValidationResults, r *structs.NodeReservedResources) {
if r == nil {
// Coding error so use Go name for field instead of HCL.
results.AppendErrorf("Node.ReservedResources must be initialized. Please report a bug")
return
}

if r.Cpu.CpuShares < 0 {
results.AppendErrorf("reserved.cpu must be >= 0 but found: %d", r.Cpu.CpuShares)
}

if c.CpuCompute > 0 && int64(c.CpuCompute) <= r.Cpu.CpuShares {
//TODO Elevate to error post-1.2.x
results.AppendWarning("reserved.cpu >= cpu_total_compute: node will be ineligible for new work until fixed (this warning may become a fatal error in the future)")
}

// ReservedCpuCores is validated by the Agent

if r.Memory.MemoryMB < 0 {
results.AppendErrorf("reserved.memory must be >= 0 but found: %d", r.Memory.MemoryMB)
}

if c.MemoryMB > 0 && int64(c.MemoryMB) <= r.Memory.MemoryMB {
//TODO Elevate to error post-1.2.x
results.AppendWarning("reserved.memory >= memory_total_mb: node will be ineligible for new work until fixed (this warning may become a fatal error in the future)")
}

if r.Disk.DiskMB < 0 {
results.AppendErrorf("reserved.disk must be >= 0 but found: %d", r.Disk.DiskMB)
}

if ports := r.Networks.ReservedHostPorts; ports != "" {
if _, err := structs.ParsePortRanges(ports); err != nil {
results.AppendErrorf("reserved.reserved_ports %q invalid: %w", ports, err)
}
}
}
29 changes: 28 additions & 1 deletion command/agent/agent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,14 @@ func NewAgent(config *Config, logger log.InterceptLogger, logOutput io.Writer, i
// Global logger should match internal logger as much as possible
golog.SetFlags(golog.LstdFlags | golog.Lmicroseconds)

// Validate config after logging is initialized but before starting any
// more agent components. This allows reporting validation issues to
// the user's configured logger at the expense of not being able to
// usefully validate logging parameters.
if err := a.validateConfig(); err != nil {
return nil, fmt.Errorf("Invalid agent configuration. Must fix configuration errors before agent can start: %w", err)
}

if err := a.setupConsul(config.Consul); err != nil {
return nil, fmt.Errorf("Failed to initialize Consul client: %v", err)
}
Expand All @@ -152,6 +160,25 @@ func NewAgent(config *Config, logger log.InterceptLogger, logOutput io.Writer, i
return a, nil
}

// validateConfig and return an error if the config is invalid. Errors and
// warnings are logged but only errors return a non-nil value.
func (a *Agent) validateConfig() error {
results := a.config.Validate()
if !results.Problems() {
// Valid! Short-circuit
return nil
}

for _, warning := range results.Warnings {
a.logger.Warn("Agent validation warning", "warning", warning)
}
for _, err := range results.Errors.Errors {
a.logger.Error("Agent validation error", "error", err)
}

return results.ErrSummary()
}

// convertServerConfig takes an agent config and log output and returns a Nomad
// Config. There may be missing fields that must be set by the agent. To do this
// call finalizeServerConfig
Expand Down Expand Up @@ -641,7 +668,7 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {
if agentConfig.Client.Reserved.Cores != "" {
cores, err := cpuset.Parse(agentConfig.Client.Reserved.Cores)
if err != nil {
return nil, fmt.Errorf("failed to parse client > reserved > cores value %q: %v", agentConfig.Client.Reserved.Cores, err)
return nil, fmt.Errorf("failed to parse client.reserved.cores value %q: %v", agentConfig.Client.Reserved.Cores, err)
}
res.Cpu.ReservedCpuCores = cores.ToSlice()
}
Expand Down
Loading

0 comments on commit 482d975

Please sign in to comment.