Code mantainance (#647)

* func: remove consul

* func: remove loop

* func: remove duplicated call to status

* func: remove consul from configuration and tests

* Update agent/agent_test.go

Co-authored-by: Luiz Aoqui <[email protected]>

* fix: move the error check from the monitor up, to avoid infinite loops

* style: add comment

* func: simplify the calls to plugins

* fix: update test

* style: remove unused code

* style: wrap get statu on metrics

* Update policy/handler.go

Co-authored-by: Luiz Aoqui <[email protected]>

* Update policy/handler.go

Co-authored-by: Luiz Aoqui <[email protected]>

* Update policy/manager.go

Co-authored-by: Luiz Aoqui <[email protected]>

* Update policyeval/base_worker.go

Co-authored-by: Luiz Aoqui <[email protected]>

* Update policyeval/base_worker.go

Co-authored-by: Luiz Aoqui <[email protected]>

* style: document channels

* style: remove unsued target status from e
valuation

---------

Co-authored-by: Luiz Aoqui <[email protected]>

Author: Juanadelacuesta

.gitignore

@@ -18,4 +18,7 @@ plugins/test/noop-*/noop-*
 # demo configs
 example.nomad.hcl
 spec.hcl
-config.hcl
+config.hcl
+
+# IDE config files
+.vscode/

agent/agent.go

@@ -11,7 +11,6 @@ import (
 	"syscall"
 
 	metrics "github.com/armon/go-metrics"
-	consulapi "github.com/hashicorp/consul/api"
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/nomad-autoscaler/agent/config"
 	"github.com/hashicorp/nomad-autoscaler/plugins/manager"
@@ -29,7 +28,6 @@ type Agent struct {
 	config        *config.Agent
 	configPaths   []string
 	nomadClient   *api.Client
-	consulClient  *consulapi.Client
 	pluginManager *manager.PluginManager
 	policySources map[policy.SourceName]policy.Source
 	policyManager *policy.Manager
@@ -63,11 +61,6 @@ func (a *Agent) Run() error {
 		return err
 	}
 
-	// Generate the Consul client
-	if err := a.generateConsulClient(); err != nil {
-		return err
-	}
-
 	// launch plugins
 	if err := a.setupPlugins(); err != nil {
 		return fmt.Errorf("failed to setup plugins: %v", err)
@@ -199,28 +192,6 @@ func (a *Agent) generateNomadClient() error {
 	return nil
 }
 
-// generateConsulClient creates a Consul client for use within the agent.
-func (a *Agent) generateConsulClient() error {
-	if a.config.Consul == nil {
-		a.consulClient = nil
-		return nil
-	}
-
-	cfg, err := a.config.Consul.MergeWithDefault()
-	if err != nil {
-		return fmt.Errorf("error generating Consul client config: %v", err)
-	}
-
-	// Generate the Consul client.
-	client, err := consulapi.NewClient(cfg)
-	if err != nil {
-		return fmt.Errorf("failed to instantiate Consul client: %v", err)
-	}
-	a.consulClient = client
-
-	return nil
-}
-
 // reload triggers the reload of sub-routines based on the operator sending a
 // SIGHUP signal to the agent.
 func (a *Agent) reload() {
@@ -243,11 +214,6 @@ func (a *Agent) reload() {
 		os.Exit(1)
 	}
 
-	if err := a.generateConsulClient(); err != nil {
-		a.logger.Error("failed to reload Autoscaler configuration", "error", err)
-		os.Exit(1)
-	}
-
 	a.logger.Debug("reloading policy sources")
 	// Set new Nomad client in the Nomad policy source.
 	ps, ok := a.policySources[policy.SourceNameNomad]

agent/agent_test.go

@@ -7,10 +7,8 @@ import (
 	"errors"
 	"testing"
 
-	"github.com/hashicorp/nomad-autoscaler/agent/config"
 	"github.com/hashicorp/nomad/api"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func TestAgent_generateNomadClient(t *testing.T) {
@@ -33,7 +31,7 @@ func TestAgent_generateNomadClient(t *testing.T) {
 				},
 			},
 			expectedOutputEr: errors.New(`failed to instantiate Nomad client: invalid address '	': parse "\t": net/url: invalid control character in URL`),
-			name: "invalid input Nomad address", //nolint
+			name:             "invalid input Nomad address",
 		},
 	}
 
@@ -45,25 +43,3 @@ func TestAgent_generateNomadClient(t *testing.T) {
 		}
 	}
 }
-
-func TestAgent_generateConsulClient(t *testing.T) {
-	require := require.New(t)
-	agent := Agent{
-		config: &config.Agent{
-			Consul: nil,
-		},
-	}
-
-	// nil config => no consul client
-	require.NoError(agent.generateConsulClient())
-	require.Nil(agent.consulClient, "client should be nil because config was nil")
-
-	// empty config => default consul client
-	agent.config.Consul = &config.Consul{}
-	require.NoError(agent.generateConsulClient())
-	require.NotNil(agent.consulClient)
-
-	// explicit config overrides defaults, can throw errors
-	agent.config.Consul.Addr = "bad://1234"
-	require.EqualError(agent.generateConsulClient(), "failed to instantiate Consul client: Unknown protocol scheme: bad")
-}

agent/config/config.go

@@ -6,14 +6,11 @@ package config
 import (
 	"errors"
 	"fmt"
-	"net/http"
 	"os"
 	"path/filepath"
 	"sort"
-	"strings"
 	"time"
 
-	consulapi "github.com/hashicorp/consul/api"
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/hcl/v2/hclsimple"
 	"github.com/hashicorp/nomad-autoscaler/plugins"
@@ -66,9 +63,6 @@ type Agent struct {
 	// Telemetry is the configuration used to setup metrics collection.
 	Telemetry *Telemetry `hcl:"telemetry,block"`
 
-	// Consul is used to configure a consul API client
-	Consul *Consul `hcl:"consul,block"`
-
 	APMs       []*Plugin `hcl:"apm,block"`
 	Targets    []*Plugin `hcl:"target,block"`
 	Strategies []*Plugin `hcl:"strategy,block"`
@@ -160,166 +154,6 @@ type Nomad struct {
 	SkipVerify bool `hcl:"skip_verify,optional"`
 }
 
-// Consul contains the configuration information necessary to
-// communicate with a Consul server.
-type Consul struct {
-	// Addr is the HTTP endpoint address of the local Consul agent
-	//
-	// Uses Consul's default and env var.
-	Addr string `hcl:"address"`
-
-	// TimeoutHCL is used by Consul HTTP Client
-	TimeoutHCL string `hcl:"timeout" json:"-"`
-
-	// Token is used to provide a per-request ACL token. This options overrides
-	// the agent's default token
-	Token string `hcl:"token"`
-
-	// Auth is the information to use for http access to Consul agent
-	Auth string `hcl:"auth"`
-
-	// EnableSSL sets the transport scheme to talk to the Consul agent as https
-	//
-	// Uses Consul's default and env var.
-	EnableSSL *bool `hcl:"ssl"`
-
-	// VerifySSL enables or disables SSL verification when the transport scheme
-	// for the consul api client is https
-	//
-	// Uses Consul's default and env var.
-	VerifySSL *bool `hcl:"verify_ssl"`
-
-	// CAFile is the path to the ca certificate used for Consul communication.
-	//
-	// Uses Consul's default and env var.
-	CAFile string `hcl:"ca_file"`
-
-	// CertFile is the path to the certificate for Consul communication
-	CertFile string `hcl:"cert_file"`
-
-	// KeyFile is the path to the private key for Consul communication
-	KeyFile string `hcl:"key_file"`
-
-	// Namespace sets the Consul namespace used for all calls against the
-	// Consul API. If this is unset, then we don't specify a consul namespace.
-	Namespace string `hcl:"namespace"`
-
-	// Datacenter sets the Consul datacenter used for all calls against the
-	// Consul API. If this is unset, then we don't specify a consul datacenter.
-	Datacenter string `hcl:"datacenter"`
-}
-
-func (c *Consul) MergeWithDefault() (*consulapi.Config, error) {
-	if c == nil {
-		return nil, nil
-	}
-
-	cfg := consulapi.DefaultConfig()
-
-	if c.Addr != "" {
-		cfg.Address = c.Addr
-	}
-	if c.Token != "" {
-		cfg.Token = c.Token
-	}
-	if c.TimeoutHCL != "" {
-		d, err := time.ParseDuration(c.TimeoutHCL)
-		if err != nil {
-			return nil, fmt.Errorf("consul->timeout can't parse time duration %s", c.TimeoutHCL)
-		}
-		// Create a custom Client to set the timeout
-		if cfg.HttpClient == nil {
-			cfg.HttpClient = &http.Client{}
-		}
-		cfg.HttpClient.Timeout = d
-		cfg.HttpClient.Transport = cfg.Transport
-	}
-	if c.Auth != "" {
-		var username, password string
-		if strings.Contains(c.Auth, ":") {
-			split := strings.SplitN(c.Auth, ":", 2)
-			username = split[0]
-			password = split[1]
-		} else {
-			username = c.Auth
-		}
-
-		cfg.HttpAuth = &consulapi.HttpBasicAuth{
-			Username: username,
-			Password: password,
-		}
-	}
-	if c.EnableSSL != nil && *c.EnableSSL {
-		cfg.Scheme = "https"
-		cfg.TLSConfig = consulapi.TLSConfig{
-			Address:  cfg.Address,
-			CAFile:   c.CAFile,
-			CertFile: c.CertFile,
-			KeyFile:  c.KeyFile,
-		}
-		if c.VerifySSL != nil {
-			cfg.TLSConfig.InsecureSkipVerify = !*c.VerifySSL
-		}
-		tlsConfig, err := consulapi.SetupTLSConfig(&cfg.TLSConfig)
-		if err != nil {
-			return nil, err
-		}
-		cfg.Transport.TLSClientConfig = tlsConfig
-	}
-	if c.Namespace != "" {
-		cfg.Namespace = c.Namespace
-	}
-	if c.Datacenter != "" {
-		cfg.Datacenter = c.Datacenter
-	}
-
-	return cfg, nil
-}
-
-func (c *Consul) merge(b *Consul) *Consul {
-	if c == nil {
-		return b
-	}
-
-	result := *c
-
-	if b.Addr != "" {
-		result.Addr = b.Addr
-	}
-	if b.TimeoutHCL != "" {
-		result.TimeoutHCL = b.TimeoutHCL
-	}
-	if b.Datacenter != "" {
-		result.Datacenter = b.Datacenter
-	}
-	if b.Namespace != "" {
-		result.Namespace = b.Namespace
-	}
-	if b.Token != "" {
-		result.Token = b.Token
-	}
-	if b.Auth != "" {
-		result.Auth = b.Auth
-	}
-	if b.CAFile != "" {
-		result.CAFile = b.CAFile
-	}
-	if b.CertFile != "" {
-		result.CertFile = b.CertFile
-	}
-	if b.KeyFile != "" {
-		result.KeyFile = b.KeyFile
-	}
-	if b.EnableSSL != nil {
-		result.EnableSSL = b.EnableSSL
-	}
-	if b.VerifySSL != nil {
-		result.VerifySSL = b.VerifySSL
-	}
-
-	return &result
-}
-
 // Telemetry holds the user specified configuration for metrics collection.
 type Telemetry struct {
 
@@ -557,8 +391,7 @@ func Default() (*Agent, error) {
 			BindAddress: defaultHTTPBindAddress,
 			BindPort:    defaultHTTPBindPort,
 		},
-		Consul: nil,
-		Nomad:  &Nomad{},
+		Nomad: &Nomad{},
 		Telemetry: &Telemetry{
 			CollectionInterval: defaultTelemetryCollectionInterval,
 		},
@@ -623,10 +456,6 @@ func (a *Agent) Merge(b *Agent) *Agent {
 		result.Nomad = result.Nomad.merge(b.Nomad)
 	}
 
-	if b.Consul != nil {
-		result.Consul = result.Consul.merge(b.Consul)
-	}
-
 	if b.Telemetry != nil {
 		result.Telemetry = result.Telemetry.merge(b.Telemetry)
 	}

agent/config/config_test.go

@@ -94,7 +94,6 @@ func TestAgent_Merge(t *testing.T) {
 		HTTP: &HTTP{
 			BindPort: 4646,
 		},
-		Consul: &Consul{},
 		Nomad: &Nomad{
 			Address:       "https://nomad-new.systems:4646",
 			Region:        "moon-base-1",
@@ -194,7 +193,6 @@ func TestAgent_Merge(t *testing.T) {
 			BindAddress: "scaler.nomad",
 			BindPort:    4646,
 		},
-		Consul: &Consul{},
 		Nomad: &Nomad{
 			Address:       "https://nomad-new.systems:4646",
 			Region:        "moon-base-1",
@@ -314,7 +312,6 @@ func TestAgent_Merge(t *testing.T) {
 	assert.Equal(t, expectedResult.LogJson, actualResult.LogJson)
 	assert.Equal(t, expectedResult.LogLevel, actualResult.LogLevel)
 	assert.Equal(t, expectedResult.Nomad, actualResult.Nomad)
-	assert.Equal(t, expectedResult.Consul, actualResult.Consul)
 	assert.Equal(t, expectedResult.PluginDir, actualResult.PluginDir)
 	assert.Equal(t, expectedResult.Policy, actualResult.Policy)
 	assert.Equal(t, expectedResult.PolicyEval, actualResult.PolicyEval)