Skip to content

Commit

Permalink
move meta/attribute reads into client factory function
Browse files Browse the repository at this point in the history
  • Loading branch information
tgross committed Jun 26, 2024
1 parent de1d286 commit 4384852
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 13 deletions.
2 changes: 1 addition & 1 deletion client/allocrunner/alloc_runner_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (ar *allocRunner) initRunnerHooks(config *clientconfig.Config) error {
allocdir: ar.allocDir,
widmgr: ar.widmgr,
consulConfigs: ar.clientConfig.GetConsulConfigs(hookLogger),
consulClientConstructor: consul.NewConsulClientFactory(config.Node),
consulClientConstructor: consul.NewConsulClientFactory(config),
hookResources: ar.hookResources,
envBuilder: newEnvBuilder,
logger: hookLogger,
Expand Down
2 changes: 1 addition & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3003,7 +3003,7 @@ func (c *Client) deriveSIToken(ctx context.Context, alloc *structs.Allocation, t
// https://www.consul.io/docs/internals/security.html

consulConfigs := c.config.GetConsulConfigs(c.logger)
consulClientConstructor := consulApiShim.NewConsulClientFactory(c.Node())
consulClientConstructor := consulApiShim.NewConsulClientFactory(c.config)

tg := alloc.Job.LookupTaskGroup(alloc.TaskGroup)
tgNs := tg.Consul.GetNamespace()
Expand Down
4 changes: 4 additions & 0 deletions client/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1019,3 +1019,7 @@ func (c *Config) GetDefaultConsul() *structsc.ConsulConfig {
func (c *Config) GetDefaultVault() *structsc.VaultConfig {
return c.VaultConfigs[structs.VaultDefaultCluster]
}

func (c *Config) GetNode() *structs.Node {
return c.Node
}
24 changes: 14 additions & 10 deletions client/consul/consul.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
consulapi "github.com/hashicorp/consul/api"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-multierror"

"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/helper/useragent"
"github.com/hashicorp/nomad/nomad/structs"
Expand Down Expand Up @@ -89,17 +88,15 @@ type consulClient struct {
// ConsulClientFunc creates a new Consul client for the specific Consul config
type ConsulClientFunc func(config *config.ConsulConfig, logger hclog.Logger) (Client, error)

// NodeGetter breaks a circular dependency between client/config.Config and this
// package
type NodeGetter interface {
GetNode() *structs.Node
}

// NewConsulClientFactory returns a ConsulClientFunc that closes over the
// partition
func NewConsulClientFactory(node *structs.Node) ConsulClientFunc {

// these node values will be evaluated at the time we create the hooks, so
// we don't need to worry about them changing out from under us
partition := node.Attributes["consul.partition"]
preflightCheckTimeout := durationFromMeta(
node, "consul.token_preflight_check.timeout", time.Second*10)
preflightCheckBaseInterval := durationFromMeta(
node, "consul.token_preflight_check.base", time.Millisecond*500)
func NewConsulClientFactory(nodeGetter NodeGetter) ConsulClientFunc {

return func(config *config.ConsulConfig, logger hclog.Logger) (Client, error) {
if config == nil {
Expand All @@ -108,6 +105,13 @@ func NewConsulClientFactory(node *structs.Node) ConsulClientFunc {

logger = logger.Named("consul").With("name", config.Name)

node := nodeGetter.GetNode()
partition := node.Attributes["consul.partition"]
preflightCheckTimeout := durationFromMeta(
node, "consul.token_preflight_check.timeout", time.Second*10)
preflightCheckBaseInterval := durationFromMeta(
node, "consul.token_preflight_check.base", time.Millisecond*500)

c := &consulClient{
config: config,
logger: logger,
Expand Down
11 changes: 10 additions & 1 deletion client/consul/consul_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/hashicorp/nomad/helper/testlog"
"github.com/hashicorp/nomad/helper/uuid"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/nomad/structs/config"
"github.com/shoenig/test/must"
)
Expand Down Expand Up @@ -65,6 +66,12 @@ func newMockConsulServer() *mockConsulServer {
return srv
}

type testClientCfg struct{ node *structs.Node }

func (c *testClientCfg) GetNode() *structs.Node {
return c.node
}

// TestConsul_TokenPreflightCheck verifies the retry logic for
func TestConsul_TokenPreflightCheck(t *testing.T) {

Expand All @@ -74,7 +81,9 @@ func TestConsul_TokenPreflightCheck(t *testing.T) {
node := mock.Node()
node.Meta["consul.token_preflight_check.timeout"] = "100ms"
node.Meta["consul.token_preflight_check.base"] = "10ms"
factory := NewConsulClientFactory(node)
clientCfg := &testClientCfg{node}

factory := NewConsulClientFactory(clientCfg)

cfg := &config.ConsulConfig{
Addr: consulSrv.httpSrv.URL,
Expand Down

0 comments on commit 4384852

Please sign in to comment.