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

client: accommodate Consul 1.14.0 gRPC and agent self changes. #15309

Merged
merged 5 commits into from
Nov 21, 2022
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
3 changes: 3 additions & 0 deletions .changelog/15309.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
fingerprint: Ensure Nomad can correctly fingerprint Consul gRPC where the Consul agent is running v1.14.0 or greater
```
2 changes: 1 addition & 1 deletion client/allocrunner/alloc_runner_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func (ar *allocRunner) initRunnerHooks(config *clientconfig.Config) error {
logger: hookLogger,
shutdownDelayCtx: ar.shutdownDelayCtx,
}),
newConsulGRPCSocketHook(hookLogger, alloc, ar.allocDir, config.ConsulConfig),
newConsulGRPCSocketHook(hookLogger, alloc, ar.allocDir, config.ConsulConfig, config.Node.Attributes),
newConsulHTTPSocketHook(hookLogger, alloc, ar.allocDir, config.ConsulConfig),
newCSIHook(alloc, hookLogger, ar.csiManager, ar.rpcClient, ar, hrs, ar.clientConfig.Node.SecretID),
newChecksHook(hookLogger, alloc, ar.checkStore, ar),
Expand Down
43 changes: 33 additions & 10 deletions client/allocrunner/consul_grpc_sock_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ const (
// socketProxyStopWaitTime is the amount of time to wait for a socket proxy
// to stop before assuming something went awry and return a timeout error.
socketProxyStopWaitTime = 3 * time.Second

// consulGRPCFallbackPort is the last resort fallback port to use in
// combination with the Consul HTTP config address when creating the
// socket.
consulGRPCFallbackPort = "8502"
)

var (
Expand All @@ -45,10 +50,20 @@ type consulGRPCSocketHook struct {
proxy *grpcSocketProxy
}

func newConsulGRPCSocketHook(logger hclog.Logger, alloc *structs.Allocation, allocDir *allocdir.AllocDir, config *config.ConsulConfig) *consulGRPCSocketHook {
func newConsulGRPCSocketHook(
logger hclog.Logger, alloc *structs.Allocation, allocDir *allocdir.AllocDir,
config *config.ConsulConfig, nodeAttrs map[string]string) *consulGRPCSocketHook {

// Attempt to find the gRPC port via the node attributes, otherwise use the
// default fallback.
consulGRPCPort, ok := nodeAttrs["consul.grpc"]
if !ok {
consulGRPCPort = consulGRPCFallbackPort
}

return &consulGRPCSocketHook{
alloc: alloc,
proxy: newGRPCSocketProxy(logger, allocDir, config),
proxy: newGRPCSocketProxy(logger, allocDir, config, consulGRPCPort),
logger: logger.Named(consulGRPCSockHookName),
}
}
Expand Down Expand Up @@ -119,21 +134,29 @@ type grpcSocketProxy struct {
allocDir *allocdir.AllocDir
config *config.ConsulConfig

// consulGRPCFallbackPort is the port to use if the operator did not
// specify a gRPC config address.
consulGRPCFallbackPort string

ctx context.Context
cancel func()
doneCh chan struct{}
runOnce bool
}

func newGRPCSocketProxy(logger hclog.Logger, allocDir *allocdir.AllocDir, config *config.ConsulConfig) *grpcSocketProxy {
func newGRPCSocketProxy(
logger hclog.Logger, allocDir *allocdir.AllocDir, config *config.ConsulConfig,
consulGRPCFallbackPort string) *grpcSocketProxy {

ctx, cancel := context.WithCancel(context.Background())
return &grpcSocketProxy{
allocDir: allocDir,
config: config,
ctx: ctx,
cancel: cancel,
doneCh: make(chan struct{}),
logger: logger,
allocDir: allocDir,
config: config,
consulGRPCFallbackPort: consulGRPCFallbackPort,
ctx: ctx,
cancel: cancel,
doneCh: make(chan struct{}),
logger: logger,
}
}

Expand Down Expand Up @@ -173,7 +196,7 @@ func (p *grpcSocketProxy) run(alloc *structs.Allocation) error {
p.config.Addr, err)
}

destAddr = net.JoinHostPort(host, "8502")
destAddr = net.JoinHostPort(host, p.consulGRPCFallbackPort)
}

hostGRPCSocketPath := filepath.Join(p.allocDir.AllocDir, allocdir.AllocGRPCSocket)
Expand Down
8 changes: 4 additions & 4 deletions client/allocrunner/consul_grpc_sock_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestConsulGRPCSocketHook_PrerunPostrun_Ok(t *testing.T) {
defer cleanup()

// Start the unix socket proxy
h := newConsulGRPCSocketHook(logger, alloc, allocDir, consulConfig)
h := newConsulGRPCSocketHook(logger, alloc, allocDir, consulConfig, map[string]string{})
require.NoError(t, h.Prerun())

gRPCSock := filepath.Join(allocDir.AllocDir, allocdir.AllocGRPCSocket)
Expand Down Expand Up @@ -116,7 +116,7 @@ func TestConsulGRPCSocketHook_Prerun_Error(t *testing.T) {
{
// An alloc without a Connect proxy sidecar should not return
// an error.
h := newConsulGRPCSocketHook(logger, alloc, allocDir, consulConfig)
h := newConsulGRPCSocketHook(logger, alloc, allocDir, consulConfig, map[string]string{})
require.NoError(t, h.Prerun())

// Postrun should be a noop
Expand All @@ -126,7 +126,7 @@ func TestConsulGRPCSocketHook_Prerun_Error(t *testing.T) {
{
// An alloc *with* a Connect proxy sidecar *should* return an error
// when Consul is not configured.
h := newConsulGRPCSocketHook(logger, connectAlloc, allocDir, consulConfig)
h := newConsulGRPCSocketHook(logger, connectAlloc, allocDir, consulConfig, map[string]string{})
require.EqualError(t, h.Prerun(), "consul address must be set on nomad client")

// Postrun should be a noop
Expand All @@ -136,7 +136,7 @@ func TestConsulGRPCSocketHook_Prerun_Error(t *testing.T) {
{
// Updating an alloc without a sidecar to have a sidecar should
// error when the sidecar is added.
h := newConsulGRPCSocketHook(logger, alloc, allocDir, consulConfig)
h := newConsulGRPCSocketHook(logger, alloc, allocDir, consulConfig, map[string]string{})
require.NoError(t, h.Prerun())

req := &interfaces.RunnerUpdateRequest{
Expand Down
53 changes: 51 additions & 2 deletions client/fingerprint/consul.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package fingerprint
import (
"fmt"
"strconv"
"strings"
"time"

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"
)

Expand All @@ -15,6 +17,14 @@ const (
consulUnavailable = "unavailable"
)

var (
// consulGRPCPortChangeVersion is the Consul version which made a breaking
// change to the way gRPC API listeners are created. This means Nomad must
// perform different fingerprinting depending on which version of Consul it
// is communicating with.
consulGRPCPortChangeVersion = version.Must(version.NewVersion("1.14.0"))
)

// ConsulFingerprint is used to fingerprint for Consul
type ConsulFingerprint struct {
logger log.Logger
Expand Down Expand Up @@ -97,7 +107,7 @@ func (f *ConsulFingerprint) initialize(req *FingerprintRequest) error {
"consul.datacenter": f.dc,
"consul.segment": f.segment,
"consul.connect": f.connect,
"consul.grpc": f.grpc,
"consul.grpc": f.grpc(consulConfig.Scheme),
"consul.ft.namespaces": f.namespaces,
}
}
Expand Down Expand Up @@ -173,11 +183,50 @@ func (f *ConsulFingerprint) connect(info agentconsul.Self) (string, bool) {
return strconv.FormatBool(c), ok
}

func (f *ConsulFingerprint) grpc(info agentconsul.Self) (string, bool) {
func (f *ConsulFingerprint) grpc(scheme string) func(info agentconsul.Self) (string, bool) {
return func(info agentconsul.Self) (string, bool) {

// The version is needed in order to understand which config object to
// query. This is because Consul 1.14.0 added a new gRPC port which
// broke the previous behaviour.
v, ok := info["Config"]["Version"].(string)
if !ok {
return "", false
}

consulVersion, err := version.NewVersion(v)
if err != nil {
return "", false
}

// If the Consul agent being fingerprinted is running a version less
// than 1.14.0 we use the original single gRPC port.
if consulVersion.Core().LessThan(consulGRPCPortChangeVersion.Core()) {
return f.grpcPort(info)
}

// Now that we know we are querying a Consul agent running v1.14.0 or
// greater, we need to select the correct port parameter from the
// config depending on whether we have been asked to speak TLS or not.
switch strings.ToLower(scheme) {
case "https":
return f.grpcTLSPort(info)
default:
return f.grpcPort(info)
}
}
}

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

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

func (f *ConsulFingerprint) namespaces(info agentconsul.Self) (string, bool) {
return strconv.FormatBool(agentconsul.Namespaces(info)), true
}
74 changes: 68 additions & 6 deletions client/fingerprint/consul_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,29 +296,91 @@ func TestConsulFingerprint_grpc(t *testing.T) {

fp := newConsulFingerPrint(t)

t.Run("grpc set", func(t *testing.T) {
s, ok := fp.grpc(agentconsul.Self{
t.Run("grpc set pre-1.14 http", func(t *testing.T) {
s, ok := fp.grpc("http")(agentconsul.Self{
"Config": {"Version": "1.13.3"},
"DebugConfig": {"GRPCPort": 8502.0}, // JSON numbers are floats
})
require.True(t, ok)
require.Equal(t, "8502", s)
})

t.Run("grpc disabled", func(t *testing.T) {
s, ok := fp.grpc(agentconsul.Self{
t.Run("grpc disabled pre-1.14 http", func(t *testing.T) {
s, ok := fp.grpc("http")(agentconsul.Self{
"Config": {"Version": "1.13.3"},
"DebugConfig": {"GRPCPort": -1.0}, // JSON numbers are floats
})
require.True(t, ok)
require.Equal(t, "-1", s)
})

t.Run("grpc missing", func(t *testing.T) {
_, ok := fp.grpc(agentconsul.Self{
t.Run("grpc set pre-1.14 https", func(t *testing.T) {
s, ok := fp.grpc("https")(agentconsul.Self{
"Config": {"Version": "1.13.3"},
"DebugConfig": {"GRPCPort": 8502.0}, // JSON numbers are floats
})
require.True(t, ok)
require.Equal(t, "8502", s)
})

t.Run("grpc disabled pre-1.14 https", func(t *testing.T) {
s, ok := fp.grpc("https")(agentconsul.Self{
"Config": {"Version": "1.13.3"},
"DebugConfig": {"GRPCPort": -1.0}, // JSON numbers are floats
})
require.True(t, ok)
require.Equal(t, "-1", s)
})

t.Run("grpc set post-1.14 http", func(t *testing.T) {
s, ok := fp.grpc("http")(agentconsul.Self{
"Config": {"Version": "1.14.0"},
"DebugConfig": {"GRPCPort": 8502.0}, // JSON numbers are floats
})
require.True(t, ok)
require.Equal(t, "8502", s)
})

t.Run("grpc disabled post-1.14 http", func(t *testing.T) {
s, ok := fp.grpc("http")(agentconsul.Self{
"Config": {"Version": "1.14.0"},
"DebugConfig": {"GRPCPort": -1.0}, // JSON numbers are floats
})
require.True(t, ok)
require.Equal(t, "-1", s)
})

t.Run("grpc disabled post-1.14 https", func(t *testing.T) {
s, ok := fp.grpc("https")(agentconsul.Self{
"Config": {"Version": "1.14.0"},
"DebugConfig": {"GRPCTLSPort": -1.0}, // JSON numbers are floats
})
require.True(t, ok)
require.Equal(t, "-1", s)
})

t.Run("grpc set post-1.14 https", func(t *testing.T) {
s, ok := fp.grpc("https")(agentconsul.Self{
"Config": {"Version": "1.14.0"},
"DebugConfig": {"GRPCTLSPort": 8503.0}, // JSON numbers are floats
})
require.True(t, ok)
require.Equal(t, "8503", s)
})

t.Run("grpc missing http", func(t *testing.T) {
_, ok := fp.grpc("http")(agentconsul.Self{
"DebugConfig": {},
})
require.False(t, ok)
})

t.Run("grpc missing https", func(t *testing.T) {
_, ok := fp.grpc("https")(agentconsul.Self{
"DebugConfig": {},
})
require.False(t, ok)
})
}

func TestConsulFingerprint_namespaces(t *testing.T) {
Expand Down
6 changes: 4 additions & 2 deletions website/content/docs/configuration/consul.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ configuring Nomad to talk to Consul via DNS such as consul.service.consul

- `grpc_address` `(string: "127.0.0.1:8502")` - Specifies the address to the local
Consul agent for `gRPC` requests, given in the format `host:port`. Note that
Consul does not enable the [`grpc`](https://developer.hashicorp.com/consul/docs/agent/config/config-files#_grpc_port)
listener by default.
Consul does not enable the [`grpc`][grpc_port] or [`grpc_tls`][grpctls_port]
listeners by default.

- `key_file` `(string: "")` - Specifies the path to the private key used for
Consul communication. If this is set then you need to also set `cert_file`.
Expand Down Expand Up @@ -227,3 +227,5 @@ namespace "nomad-ns" {
[consul]: https://www.consul.io/ 'Consul by HashiCorp'
[bootstrap]: https://learn.hashicorp.com/tutorials/nomad/clustering 'Automatic Bootstrapping'
[go-sockaddr/template]: https://pkg.go.dev/github.com/hashicorp/go-sockaddr/template
[grpc_port]: https://developer.hashicorp.com/consul/docs/agent/config/config-files#grpc_port
[grpctls_port]: https://developer.hashicorp.com/consul/docs/agent/config/config-files#grpc_tls_port
6 changes: 5 additions & 1 deletion website/content/docs/integrations/consul-connect.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ $ consul agent -dev

To use service mesh on a non-dev Consul agent, you will minimally need to enable the
GRPC port and set `connect` to enabled by adding some additional information to
your Consul client configurations, depending on format.
your Consul client configurations, depending on format. Consul agents running TLS
and a version greater than [1.14.0](https://releases.hashicorp.com/consul/1.14.0)
should set the `grpc_tls` configuration parameter instead of `grpc`. Please see
the Consul [port documentation](consul_ports) for further reference material.

For HCL configurations:

Expand Down Expand Up @@ -352,3 +355,4 @@ dashes (`-`) are converted to underscores (`_`) in environment variables so
[gh-9907]: https://github.com/hashicorp/nomad/issues/9907
[`Local`]: https://developer.hashicorp.com/consul/docs/security/acl/acl-tokens#token-attributes
[anon_token]: https://developer.hashicorp.com/consul/docs/security/acl/acl-tokens#special-purpose-tokens
[consul_ports]: https://developer.hashicorp.com/consul/docs/agent/config/config-files#ports