Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consul: probe consul namespace feature before using namespace api #10715

Merged
merged 2 commits into from
Jun 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ BUG FIXES:
* api: Fixed event stream connection initialization when there are no events to send [[GH-10637](https://github.com/hashicorp/nomad/issues/10637)]
* cli: Fixed a bug where `quota status` and `namespace status` commands may panic if the CLI targets a pre-1.1.0 cluster [[GH-10620](https://github.com/hashicorp/nomad/pull/10620)]
* cli: Fixed a bug where `alloc exec` may fail with "unexpected EOF" without returning the exit code after a command [[GH-10657](https://github.com/hashicorp/nomad/issues/10657)]
* consul: Fixed a bug where consul namespace API would be queried even when consul namespaces were not enabled [[GH-10715](https://github.com/hashicorp/nomad/pull/10715)]
* csi: Fixed a bug where `mount_options` were not passed to CSI controller plugins for validation during volume creation and mounting. [[GH-10643](https://github.com/hashicorp/nomad/issues/10643)]
* csi: Fixed a bug where `capability` blocks were not passed to CSI controller plugins for validation for `nomad volume register` commands. [[GH-10703](https://github.com/hashicorp/nomad/issues/10703)]
* drivers/exec: Fixed a bug where `exec` and `java` tasks inherit the Nomad agent's `oom_score_adj` value [[GH-10698](https://github.com/hashicorp/nomad/issues/10698)]
Expand Down
2 changes: 1 addition & 1 deletion client/allocrunner/groupservice_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func TestGroupServiceHook_Update08Alloc(t *testing.T) {
consulConfig.Address = testconsul.HTTPAddr
consulClient, err := consulapi.NewClient(consulConfig)
require.NoError(t, err)
namespacesClient := agentconsul.NewNamespacesClient(consulClient.Namespaces())
namespacesClient := agentconsul.NewNamespacesClient(consulClient.Namespaces(), consulClient.Agent())

serviceClient := agentconsul.NewServiceClient(consulClient.Agent(), namespacesClient, testlog.HCLogger(t), true)

Expand Down
8 changes: 4 additions & 4 deletions client/allocrunner/taskrunner/connect_native_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func TestTaskRunner_ConnectNativeHook_Ok(t *testing.T) {
consulConfig.Address = testConsul.HTTPAddr
consulAPIClient, err := consulapi.NewClient(consulConfig)
require.NoError(t, err)
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces())
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent())

consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true)
go consulClient.Run()
Expand Down Expand Up @@ -377,7 +377,7 @@ func TestTaskRunner_ConnectNativeHook_with_SI_token(t *testing.T) {
consulConfig.Address = testConsul.HTTPAddr
consulAPIClient, err := consulapi.NewClient(consulConfig)
require.NoError(t, err)
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces())
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent())

consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true)
go consulClient.Run()
Expand Down Expand Up @@ -454,7 +454,7 @@ func TestTaskRunner_ConnectNativeHook_shareTLS(t *testing.T) {
consulConfig.Address = testConsul.HTTPAddr
consulAPIClient, err := consulapi.NewClient(consulConfig)
require.NoError(t, err)
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces())
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent())

consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true)
go consulClient.Run()
Expand Down Expand Up @@ -574,7 +574,7 @@ func TestTaskRunner_ConnectNativeHook_shareTLS_override(t *testing.T) {
consulConfig.Address = testConsul.HTTPAddr
consulAPIClient, err := consulapi.NewClient(consulConfig)
require.NoError(t, err)
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces())
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent())

consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true)
go consulClient.Run()
Expand Down
6 changes: 3 additions & 3 deletions client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ func TestEnvoyBootstrapHook_with_SI_token(t *testing.T) {
consulConfig.Address = testConsul.HTTPAddr
consulAPIClient, err := consulapi.NewClient(consulConfig)
require.NoError(t, err)
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces())
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent())

consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true)
go consulClient.Run()
Expand Down Expand Up @@ -426,7 +426,7 @@ func TestTaskRunner_EnvoyBootstrapHook_sidecar_ok(t *testing.T) {
consulConfig.Address = testConsul.HTTPAddr
consulAPIClient, err := consulapi.NewClient(consulConfig)
require.NoError(t, err)
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces())
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent())

consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true)
go consulClient.Run()
Expand Down Expand Up @@ -491,7 +491,7 @@ func TestTaskRunner_EnvoyBootstrapHook_gateway_ok(t *testing.T) {
consulConfig.Address = testConsul.HTTPAddr
consulAPIClient, err := consulapi.NewClient(consulConfig)
require.NoError(t, err)
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces())
namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent())

// Register Group Services
serviceClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true)
Expand Down
14 changes: 10 additions & 4 deletions client/allocrunner/taskrunner/task_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1155,9 +1155,12 @@ func TestTaskRunner_CheckWatcher_Restart(t *testing.T) {

// Replace mock Consul ServiceClient, with the real ServiceClient
// backed by a mock consul whose checks are always unhealthy.
consulAgent := agentconsul.NewMockAgent()
consulAgent := agentconsul.NewMockAgent(agentconsul.Features{
Enterprise: false,
Namespaces: false,
})
consulAgent.SetStatus("critical")
namespacesClient := agentconsul.NewNamespacesClient(agentconsul.NewMockNamespaces(nil))
namespacesClient := agentconsul.NewNamespacesClient(agentconsul.NewMockNamespaces(nil), consulAgent)
consulClient := agentconsul.NewServiceClient(consulAgent, namespacesClient, conf.Logger, true)
go consulClient.Run()
defer consulClient.Shutdown()
Expand Down Expand Up @@ -1835,8 +1838,11 @@ func TestTaskRunner_DriverNetwork(t *testing.T) {
defer cleanup()

// Use a mock agent to test for services
consulAgent := agentconsul.NewMockAgent()
namespacesClient := agentconsul.NewNamespacesClient(agentconsul.NewMockNamespaces(nil))
consulAgent := agentconsul.NewMockAgent(agentconsul.Features{
Enterprise: false,
Namespaces: false,
})
namespacesClient := agentconsul.NewNamespacesClient(agentconsul.NewMockNamespaces(nil), consulAgent)
consulClient := agentconsul.NewServiceClient(consulAgent, namespacesClient, conf.Logger, true)
defer consulClient.Shutdown()
go consulClient.Run()
Expand Down
74 changes: 18 additions & 56 deletions client/fingerprint/consul.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ package fingerprint
import (
"fmt"
"strconv"
"strings"
"time"

consul "github.com/hashicorp/consul/api"
consulapi "github.com/hashicorp/consul/api"
log "github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-version"
agentconsul "github.com/hashicorp/nomad/command/agent/consul"
)

const (
Expand All @@ -19,17 +18,14 @@ const (
// ConsulFingerprint is used to fingerprint for Consul
type ConsulFingerprint struct {
logger log.Logger
client *consul.Client
client *consulapi.Client
lastState string
extractors map[string]consulExtractor
}

// consulInfo aliases the type returned from the Consul agent self endpoint.
type consulInfo = map[string]map[string]interface{}

// consulExtractor is used to parse out one attribute from consulInfo. Returns
// the value of the attribute, and whether the attribute exists.
type consulExtractor func(consulInfo) (string, bool)
type consulExtractor func(agentconsul.Self) (string, bool)

// NewConsulFingerprint is used to create a Consul fingerprint
func NewConsulFingerprint(logger log.Logger) Fingerprint {
Expand Down Expand Up @@ -95,7 +91,7 @@ func (f *ConsulFingerprint) initialize(req *FingerprintRequest) error {
return fmt.Errorf("failed to initialize Consul client config: %v", err)
}

f.client, err = consul.NewClient(consulConfig)
f.client, err = consulapi.NewClient(consulConfig)
if err != nil {
return fmt.Errorf("failed to initialize Consul client: %s", err)
}
Expand All @@ -117,7 +113,7 @@ func (f *ConsulFingerprint) initialize(req *FingerprintRequest) error {
return nil
}

func (f *ConsulFingerprint) query(resp *FingerprintResponse) consulInfo {
func (f *ConsulFingerprint) query(resp *FingerprintResponse) agentconsul.Self {
// We'll try to detect consul by making a query to to the agent's self API.
// If we can't hit this URL consul is probably not running on this machine.
info, err := f.client.Agent().Self()
Expand All @@ -144,48 +140,36 @@ func (f *ConsulFingerprint) link(resp *FingerprintResponse) {
}
}

func (f *ConsulFingerprint) server(info consulInfo) (string, bool) {
func (f *ConsulFingerprint) server(info agentconsul.Self) (string, bool) {
s, ok := info["Config"]["Server"].(bool)
return strconv.FormatBool(s), ok
}

func (f *ConsulFingerprint) version(info consulInfo) (string, bool) {
func (f *ConsulFingerprint) version(info agentconsul.Self) (string, bool) {
v, ok := info["Config"]["Version"].(string)
return v, ok
}

func (f *ConsulFingerprint) sku(info consulInfo) (string, bool) {
v, ok := info["Config"]["Version"].(string)
if !ok {
return "", ok
}

ver, vErr := version.NewVersion(v)
if vErr != nil {
return "", false
}
if strings.Contains(ver.Metadata(), "ent") {
return "ent", true
}
return "oss", true
func (f *ConsulFingerprint) sku(info agentconsul.Self) (string, bool) {
return agentconsul.SKU(info)
}

func (f *ConsulFingerprint) revision(info consulInfo) (string, bool) {
func (f *ConsulFingerprint) revision(info agentconsul.Self) (string, bool) {
r, ok := info["Config"]["Revision"].(string)
return r, ok
}

func (f *ConsulFingerprint) name(info consulInfo) (string, bool) {
func (f *ConsulFingerprint) name(info agentconsul.Self) (string, bool) {
n, ok := info["Config"]["NodeName"].(string)
return n, ok
}

func (f *ConsulFingerprint) dc(info consulInfo) (string, bool) {
func (f *ConsulFingerprint) dc(info agentconsul.Self) (string, bool) {
d, ok := info["Config"]["Datacenter"].(string)
return d, ok
}

func (f *ConsulFingerprint) segment(info consulInfo) (string, bool) {
func (f *ConsulFingerprint) segment(info agentconsul.Self) (string, bool) {
tags, tagsOK := info["Member"]["Tags"].(map[string]interface{})
if !tagsOK {
return "", false
Expand All @@ -194,38 +178,16 @@ func (f *ConsulFingerprint) segment(info consulInfo) (string, bool) {
return s, ok
}

func (f *ConsulFingerprint) connect(info consulInfo) (string, bool) {
func (f *ConsulFingerprint) connect(info agentconsul.Self) (string, bool) {
c, ok := info["DebugConfig"]["ConnectEnabled"].(bool)
return strconv.FormatBool(c), ok
}

func (f *ConsulFingerprint) grpc(info consulInfo) (string, bool) {
func (f *ConsulFingerprint) grpc(info agentconsul.Self) (string, bool) {
p, ok := info["DebugConfig"]["GRPCPort"].(float64)
return fmt.Sprintf("%d", int(p)), ok
}

func (f *ConsulFingerprint) namespaces(info consulInfo) (string, bool) {
return f.feature("Namespaces", info)
}

// possible values as of v1.9.5+ent:
// Automated Backups, Automated Upgrades, Enhanced Read Scalability,
// Network Segments, Redundancy Zone, Advanced Network Federation,
// Namespaces, SSO, Audit Logging
func (f *ConsulFingerprint) feature(name string, info consulInfo) (string, bool) {
lic, licOK := info["Stats"]["license"].(map[string]interface{})
if !licOK {
return "", false
}

features, exists := lic["features"].(string)
if !exists {
return "", false
}

if !strings.Contains(features, name) {
return "", false
}

return "true", true
func (f *ConsulFingerprint) namespaces(info agentconsul.Self) (string, bool) {
return strconv.FormatBool(agentconsul.Namespaces(info)), true
}
Loading