Dynamic Server Lists/Client Bootstrapping #1201

Removes four redundant copies of the method in the process.

There were two `ConsulConfig` structs running around, one of them needed to go away. Rely on the package's path to provide context for the type of AgentConfig.

A follow up commit to the previous rename. More to come.

The enclosed struct provides the necessary context

These parameters are used to bootstrap Nomad servers, not Consul servers.

Flattening and normalizing the various Consul config structures and services has led to an import cycle. Break this by creating a new package that is intended to be terminal in the import DAG.

Syncer describes the responsibility and actions of the type.

Implement client/rpc_proxy.RpcProxy.

nomad/structs/config/consul.go's ConsulConfig is the canonical definition for all things Consul now.

Reduce down to its lowest common helper function denominator.

When a deadline timer of 2x Server's last requested TTL expires, begin polling Consul for Nomad Servers.

Somewhere along the lines when rebasing I mis-merged a patch.

There were conflicting directives previously, both consul.addr and consul.address were required to achieve the desired behavior. The documentation said `consul.address` was the canonical name for the parameter, so consolidate configuration parameters to `consul.address`.

…orrect.

While documented, the consul.timeout parameter wasn't ever set except one-off in the Consul fingerprinter.

A follow up commit to the previous rename. More to come.

Implement client/rpc_proxy.RpcProxy.

nomad/structs/config/consul.go's ConsulConfig is the canonical definition for all things Consul now.

Also rename `NewRpcProxy()` to just `New()` to avoid package stutter.

These ranges aren't too useful with the default 600s rebalance, but will be useful if that default ever changes in the future.

Rename c.serverRpcAddr to serverRpcAddr. This will be broken out into in additional set of services in a subsequent commit.

In order to give clients a fighting chance to talk to the right port, differentiate RPC services from HTTP services by registering two services with different tags. This yields `rpc.nomad-server.service.consul` and `http.nomad-server.service.consul` which is immensely more useful to clients attempting to bootstrap their world.

"There can be only one."

Only one pump and periodic loop now.

Servers that are part of a different datacenter are added as backup servers instead of primary servers.

If Nomad is heartbeating during a transition from using backup servers to Nomad servers, make Nomad the canonical source of servers and flush the list of servers populated from Consul.

Hat tip to Alex for pointing this out (vs patching mapstructure)

… `nomad`.

This will be unused in this PR.

…nor). Reduce future confusion by introducing a minor version that is gossiped out via the `mvn` Serf tag (Minor Version Number, `vsn` is already being used for to communicate `Major Version Number`). Background: hashicorp/consul/issues/1346#issuecomment-151663152

Resolves an import cycle in testing and is more appropriate because the default should reside next to its struct definition.

Change the unit test to only test if the consul link exists, not the value of the link. The old test was hostname specific and therefore would always be different based on the environment running the tests.

When an agent is running a server, the list of servers includes the Raft peers. When the agent is running a client (which is always the case?), include a list of the servers found in the Client's RpcProxy. Dedupe and provide a unique list back to the caller.

Add similar logic in Agent `serverConfig()` to set up the `serverSerfAddr` the same as `serverHttpAddr` and `serverRpcAddr`.

`consul.address` does, but not `consul.addr`.

This endpoint shouldn't be hit often, but this could be useful in logs down the road.

…package move.

Overly zealous search && replace at work here.

Requested by: alex

Now that hostnames are validated on input, switch to IPs since they bypass DNS resolution.

Be mindful of the cost of taking a snapshot from the statestore and reuse the snapshot if one has already been taken.

Also remove the space from the "name" of the node

@alex

Suggested by: @alex

Requested by: Alex

With an over abundance of caution, preevnt future copy/pasta by using the right locks when bootstrapping a Client. Strictly speaking this is not necessary, but it makes explicit the locking semantics and guards against future concurrent or parallel initialization.

@dadgar

Pointed out by: @dadgar

While breaking the API within this PR, break out the individual arguments to RefreshServerLists. The servers parameter is reusing `structs.NodeServerInfo` for the time being, but this can be revisited if the needs of the strucutre diverge in the future.

@dadgar

Requested by: @dadgar

@dadgar

Requsted by: @dadgar

@dadgar

Requested by: @dadgar

@dadgar

Requested by: @dadgar

In addition to the API changing, consul.Syncer can now be signaled to shutdown via the Shutdown() method, which will call the Run()'ing sync task to exit gracefully.

Previously this would immediately default to '127.0.0.1' if the config was set to `:some-port-number`. Now it uses the BindAddr if available. Also, if the `port` option is set to just a port` number (e.g. '1234'), attempt to parse the port number by itself to allow statically configured ports to work, even when no host is specified.

I *KNEW* I should have done this when I wrote it, but didn't want to go back and audit the handlers to include the appropriate return handling, but now that the code is taking shape, make this change.

SyncServices() immediately attempts to sync whatever information the process has with Consul. Previously this method would take an argument of the exclusive list of services that should exist, however this is not condusive to having a Nomad Client and Nomad Server share the same consul.Syncer.

This attribute isn't actually an identifier because it can represent a collection of services. Rename `serviceIdentifier` to `serviceRegPrefix which more accurately conveys the intention of this Syncer attribute. While here, also rename `SetServiceIdentifier()` to `SetServiceRegPrefix()` and `GenerateServiceIdentifier()` to `GenerateServicePrefix()`.

This has been done to allow the Server and Client to reuse the same Syncer because the Agent may be running Client, Server, or both simultaneously and we only want one Syncer object alive in the agent.

This call is obsolete by a future commit that changes the canonical source of truth to be consul.AgentServiceRegistration structs, which means it is not necessary to construct AgentServiceRegistration objects every time a registration is made, we just reuse the existing object.

…once This should be handled via a sync.Once primative, but I don't want to unpack that atm.

Nuke PopulateServiceIDs() now that it's also no longer needed.

Decompose Client and Server registration into `setupClient()` and `setupServer()`, respectively.

Now the right way to register services with the Syncer is to call `SetServices(groupName, []*services)`. This was required to allow the Syncer to sync either the Client, Server, or Both using a single Syncer.

Apologies in advance for the variable thrash, the fingerprinter is no longer used to gate whether or not Consul is available any more.

…eader. It's possible that a Nomad Client is heartbeating with a Nomad server that has become issolated from the quorum of Nomad Servers. When 3x the heartbeatTTL has been exceeded, append the Consul server list to the primary primary server list. When the next RPCProxy rebalance occurs, there is a chance one of the servers discovered from Consul will be in the majority. When client reattaches to a Nomad Server in the majority, it will include a heartbeat and will reset the TTLs *AND* will clear the primary server list to include only values from the heartbeat.

…ervice The source of truth is the local Nomad Agent. Any services not local that have a matching prefix are removed. Changed services are re-registered and missing services are re-added.

The source of truth is the local Nomad Agent. Any checks are not local that have a matching prefix are removed. Changed checks are re-registered and missing checks are re-added.

ConsulService is the configuration for a Consul Service

This is wrong and should be the Agent's Consul Config. This is a step in the right direction, so committing to mark the necessary future change.

trackedServices, delegateChecks, trackedChecks, and checkRunners should all be covered. This lock needs to be reasonably narrow and can't use defer due to possible recursive locking concerns further downstream from the call sites.

Log once when Consul goes away, and log when Consul comes back.

…nse()

This logic was already present elsewhere and was missed in this one place.

…the existing server (vs nil when the server already exists).

…ewServerEndpoint()`

Executors can `Shutdown()` before calling `SyncServices()`.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic Server Lists/Client Bootstrapping #1201

Dynamic Server Lists/Client Bootstrapping #1201

Commits on Jun 10, 2016

Commits on Jun 11, 2016