-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
k8s: Fix potential edge case where a second cluster could get started #10268
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -59,6 +66,33 @@ spec: | |||
app: cockroachdb | |||
annotations: | |||
pod.alpha.kubernetes.io/initialized: "true" | |||
pod.alpha.kubernetes.io/init-containers: '[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a link to some docs you could put in here to explain "how things work"? In particular, cockroach-k8s-init
will be run with what parameters and what stdin?
The cockroach-k8s-init
container has an entry point /peer-finder
, but then there's also the on-start.sh
script which does the actual work but it's not clear how it's invoked. A short overview would be helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
9f533e6
to
c97bcab
Compare
* Add an init container that checks for whether any other peers exist * Re-enable the tolerate-unready-endpoints option. It's bprashanth's recommendation, and makes the init container less likely to miss anything. * Switch from joining `cockroachdb` to joining `cockroachdb-public`. This is needed due to re-enabling `tolerate-undready-endpoints`. I wish we could just directly re-use the peer-finder container without having to wrap it, but I already burned too much time trying to get it to accepts non-trivial commands in its `--on-start` parameter. Almost everything I tried would just get treated to a "No such file or directory" error.
@a-robinson, I can't quite tell: Is there any impact here on our kubernetes docs? https://www.cockroachlabs.com/docs/orchestrate-cockroachdb-with-kubernetes.html |
@jseldess - nope, this shouldn't require any docs changes. All it's really doing from the user's perspective is fixing an edge case that could only come up if the first node lost its data. |
Alright, these same changes made it through review in the core k8s repo (kubernetes/kubernetes#35922) and through initial review in the charts repo (helm/charts#167), so I think it's good to go. |
recommendation, and makes the init container less likely to miss
anything.
cockroachdb
to joiningcockroachdb-public
.This is needed due to re-enabling
tolerate-unready-endpoints
.I wish we could just directly re-use the peer-finder container without
having to wrap it, but I already burned too much time trying to get it
to accepts non-trivial commands in its
--on-start
parameter. Almosteverything I tried would just get treated to a "No such file or
directory" error.
@tschottdorf @bprashanth
This change is