agent: prevent very old servers re-joining a cluster with stale data

[loshz]

Description

This PR introduces the concept of Server Metadata, which is server specific information written to a file stored in the configured data directory. During this initial phase, we only store the last seen timestamp in Unix format:

consul/agent/consul/server_metadata.go

Lines 17 to 19 in 5103a1a

	type ServerMetadata struct {
	LastSeenUnix int64 `json:"last_seen_unix"`
	}

2 new agent methods for attempting to prevent old servers from rejoining an existing cluster.

• persistServerMetadata(): Periodically write a server's metadata to a file in the configured data directory every hour.
• checkServerLastSeen(): Attempt to read a server's last seen file and check the Unix timestamp against a configurable max age. If the last seen file does not exist, we treat this as an initial startup and return no error.

Example

We attempt to start a previously running server with the following last seen timestamp 1672531200 (2023-Jan-01 00:00:00).

Setting the new config to server_rejoin_age_max = "3d" will prevent this server from starting and require an operator to manually remove this file from the data directory if they need to force the server to rejoin.

Testing & Reproduction steps

• Added unit tests for new Server Metatdata helper funcs.
• Added unit tests for agent Server Metatdata implementation.

Links

• https://hashicorp.atlassian.net/browse/NET-3772
• https://github.com/hashicorp/consul/blob/main/docs/config/checklist-adding-config-fields.md

PR Checklist

• Write validation function for new config field.
• updated test coverage
• external facing docs updated
• appropriate backport labels added
• not a security concern

[banks]

Amazing stuff Dan!

I was originally thinking it might be cleanest to wrap this file handling stuff in a sub-package in the agent package. Then I saw how many actual lines of code it was and was less sure.

But I think I'd still lean towards that because:

• agent.go is way to big and has all sorts of different logic in. We've been trying to remove stuff from here for years allbeit very slowly.
• although it's trivial, the logic around reading and writing files, possibly handling misformated (e.g. because of corruption or crash) files etc. would all be so much easier to test in a small package in a unit test vs against a whole agent.

Youd still need the same call points in agent.go so it's only like 20 lines of code to extract but I think it would be a little cleaner that way and a bit easier to test more.

I could also imagine a future where we end up expanding this a bit - e.g. also adding a second timestamp for "last connected to a leader" which might help if servers are up but partitioned for a long time before being decomissioned. I don't think we should do that yet, but it would be much nicer to add stuff like that to a small independent package than agent.go if we do it.

[banks]

While it wouldn't be the end of the world if we fail to read this on a crash, do you think it's worth attempting to write it more robustly? (e.g. write to a temp file and rename)

[boxofrad]

Looks great, nice job! 👏🏻

Don't forget to update the docs. We probably want to call out that you can't set server_rejoin_age_max to anything less than an hour because of the frequency we write the file out to disk.

[jmurret]

Nice work. I just had a question on whether persistServerMetadata() could silently fail many times and then checkServerLastSeen() would silently continue because it thought it has an assumption about it just booting up.

[jmurret]

Is it possible that persistServerMetadata() never wrote the file because it encountered errors writing the file? do we need some kind of check between first startup time and now in this so we figure out "it must have had an opportunity to write a file"? Or possibly an inmemory counter of how many times persistServerMetadata() has run so we should expect a file?

[loshz]

Answered here. But I will update the code comments to explicitly call this out.

[loshz]

Nice work. I just had a question on whether persistServerMetadata() could silently fail many times and then checkServerLastSeen() would silently continue because it thought it has an assumption about it just booting up.

checkServerLastSeen() will always occur first and only once on startup. So even if persistServerMetadata() fails multiple times during a previously running server, we wouldn't have a reliable way of telling and therefore shouldn't block a proceeding server startup attempt.

[jmurret]

Looks great. Nice work. Just left a note about one hanging TODO but approved. 🎉

[loshz]

I did some manual testing via Docker compose and everything seems to be working as expected:

Default server_rejoin_age_max with a last_seen_unix of 2 weeks:

==> Starting Consul agent...
...
2023-05-15T09:41:14.616Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (168h0m0s) - consider wiping your data dir"
2023-05-15T09:41:24.616Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (168h0m0s) - consider wiping your data dir"
2023-05-15T09:41:34.616Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (168h0m0s) - consider wiping your data dir"
2023-05-15T09:41:44.617Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (168h0m0s) - consider wiping your data dir"
2023-05-15T09:41:54.617Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (168h0m0s) - consider wiping your data dir"
2023-05-15T09:42:04.618Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (168h0m0s) - consider wiping your data dir"
2023-05-15T09:42:14.620Z [ERROR] agent: Error starting agent: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (168h0m0s) - consider wiping your data dir"
2023-05-15T09:42:14.620Z [INFO]  agent: Exit code: code=1
exited with code 1

Default server_rejoin_age_max with a last_seen_unix of 1 hour:

==> Starting Consul agent...
...
2023-05-15T10:40:15.679Z [INFO]  agent: Consul agent running!

server_rejoin_age_max set to 10 hours with a last_seen_unix of 2 weeks:

==> Starting Consul agent...
...
2023-05-15T10:44:55.587Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (10h0m0s) - consider wiping your data dir"
2023-05-15T10:45:05.587Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (10h0m0s) - consider wiping your data dir"
2023-05-15T10:45:15.588Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (10h0m0s) - consider wiping your data dir"
2023-05-15T10:45:25.589Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (10h0m0s) - consider wiping your data dir"
2023-05-15T10:45:35.590Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (10h0m0s) - consider wiping your data dir"
2023-05-15T10:45:45.591Z [ERROR] agent: startup error: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (10h0m0s) - consider wiping your data dir"
2023-05-15T10:45:55.592Z [ERROR] agent: Error starting agent: error="refusing to rejoin cluster because server has been offline for more than the configured server_rejoin_age_max (10h0m0s) - consider wiping your data dir"
2023-05-15T10:45:55.592Z [INFO]  agent: Exit code: code=1
exited with code 1

server_rejoin_age_max set to 10 hours with a last_seen_unix of 1 hour:

==> Starting Consul agent...
...
2023-05-15T10:42:05.568Z [INFO]  agent: Consul agent running!

server_rejoin_age_max set to 1 hour:

==> 1 error occurred:
 	* server_rejoin_age_max: duration '1h' cannot be less than: 6h0m0s


exited with code 1

[markusschuh]

A feedback too small for a new enhancement issue on its own:

The default of the new parameter server_rejoin_age_max is set to

server_rejoin_age_max = "168h"

I'd say, the documentation should use this value - and not translate it to "7d", which is an unsupported format. This is an unnecessary challenge for the less experienced reader - though I agree, it is a small one as the supported syntax is documented at https://developer.hashicorp.com/consul/docs/agent/config/config-files#time-to-live-values

[david-yu]

Thanks for the suggestion @markusschuh PR is here: #18154