Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Fix etcd startup #1270

Merged
merged 2 commits into from
May 2, 2018
Merged

Conversation

Confushion
Copy link
Contributor

After applying the fix for issue #1206, timing/race issues were still showing when creating etcd clusters with more than 1 instance.

This patch fixes the race condition by having etcdadm-reconfigure.service depend on service var-lib-etcd2.mount instead of format-etcd2-volume.service (var-lib-etcd2.mount already depends on that service).

Also a potential circulair dependency on cfn-etcd-environment.service has been eliminated.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 1, 2018
@Confushion Confushion mentioned this pull request May 1, 2018
@codecov-io
Copy link

Codecov Report

Merging #1270 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1270   +/-   ##
=======================================
  Coverage   36.63%   36.63%           
=======================================
  Files          63       63           
  Lines        3882     3882           
=======================================
  Hits         1422     1422           
  Misses       2242     2242           
  Partials      218      218

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 185ccc5...889e48b. Read the comment docs.

@mumoshu
Copy link
Contributor

mumoshu commented May 2, 2018

@Confushion Good catch! This change does make sense to me.
Just wanted make sure, but did you actually get to bring new clusters with this change?
Anything you want me to test by hand?
Thanks anyway for your contribution 👍

@mumoshu mumoshu merged commit a1ec225 into kubernetes-retired:master May 2, 2018
@Confushion
Copy link
Contributor Author

Yes, I managed to bring up 2 new clusters with 3 etcd instances each.

Although still with setting ‘waitSignal.enabled: false’, but that seems to be a separate issue...

@mumoshu
Copy link
Contributor

mumoshu commented May 2, 2018

@Confushion Thanks for the confirmation.

So, are you still seeing issues when the wait signal is enabled?
Would you mind sharing logs(even a full output from journalctl can be helpful), statuses or anything you think important?
I'd like to sort these out, too!

@mumoshu mumoshu added this to the v0.9.10 milestone May 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants