Adds node ID integrity checking to the catalog and the LAN and WAN clusters. #2832
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Working through some test fallout from this, not quite ready for merge. |
sean-
reviewed
Mar 27, 2017
consul/state/catalog.go
Outdated
| } | ||
| if existing != nil { | ||
| n = existing.(*structs.Node) | ||
| fmt.Printf("XXX %#v\n", *n) |
There was a problem hiding this comment.
^^ ?
Also, q.Q() > fmt.Printf() 😉
Ended up removing the leader_test.go server address change test as part of this. The join was failing becase we were using a new node name with the new logic here, but realized this was hitting some of the memberlist conflict logic and not working as we expected. We need some additional work to fully support address changes, so removed the test for now.
|
I'm working on upgrading to 0.8.0 and I'm testing by spinning up 3 centos docker containers running docker-in-docker (on Docker Mac), and then each launching the consul container. What's odd is that all 3 nodes appear to come up with the same Node ID which is causing them all to throw errors and fail to join the cluster. Here is my config: Here are some of the logs showing the errors: I'm guessing it's something to do with this commit but I could be wrong. Any help would be appreciated. |
|
Hi @micahlmartin please take a look at the discussion on #2877. You can set the node ID to something unique by adding something like |
|
@slackpad That fixed it. Thanks for the quick response. |
|
@slackpad that's a neat idea, but it doesn't work with Kubernetes pods because there seems to be no support for command substitution. I'd like to run Consul in Kubernetes but I can't upgrade to 0.8.0 right now, because of this issue. Do you have any suggestion how to ensure Consul gets a new node-id each time the container restarts? Of course I could just build my own consul docker image where I first fill an env variable with a uuid but I was hoping to just use the vanilla images. I'd be happy to test a few things, if you have any ideas. 👍 |
|
@dictvm please take a look at #2877 (comment), that's probably the easiest solution for folks running containers. Do you think that would fix your particular issue with k8s? |
|
@slackpad I'm pretty sure it would, thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since node ID integrity is critical for the 0.8 Raft features, we need to ensure that node IDs are unique. We also add integrity checking for the catalog so we can start relying on node IDs there in future versions of Consul.