Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Remove unnecessary etcd systemd Alias directive#2483

Merged
jackfrancis merged 1 commit intoAzure:masterfrom
carlpett:etcd-alias-fix
Mar 19, 2018
Merged

Remove unnecessary etcd systemd Alias directive#2483
jackfrancis merged 1 commit intoAzure:masterfrom
carlpett:etcd-alias-fix

Conversation

@carlpett
Copy link
Contributor

What this PR does / why we need it:
This PR removes the Alias directive from the systemd unit file for etcd. Since the specified alias is the same as the name of the unit itself, systemd enable etcd fails.

Which issue this PR fixes
fixes #2282 (although it is already marked as fixed...)

Special notes for your reviewer:
No

Release note:

Fix etcd systemd unit

@jackfrancis
Copy link
Member

Hi @carlpett, thanks. To properly triage this as a bug: I just built a cluster off of HEAD, and got this:

$ systemctl is-enabled etcd.service
enabled

@carlpett
Copy link
Contributor Author

Sorry @jackfrancis, I didn't understand - was this a question to me?

On our clusters built with 0.13, systemctl status etcd showed something similar to Loaded: loaded (/etc/systemd/system/etcd.service; disabled; vendor preset: disabled) (I don't have the output available any more, and we fixed locally it on all masters). We didn't use is-enabled, but it should have reported disabled in this case.

As a more detailed description of the error, systemctl enable etcd gives Failed to enable unit: Invalid Argument (wonderfully clear...) if the alias is there. The reason being it tries to create two symlinks with the same name, etcd (from the unit name) and etcd (from the alias).

@jackfrancis
Copy link
Member

I was wondering if etcd.service was not being enabled at present, so I built a cluster to verify. (I was able to verify that master/HEAD is not broken.)

Unfortunately v0.13.0 is the affected version with this bug. It sounds like (1) you have an action to manually fix this on your clusters and (2) if the alias duplication which this PR removes hadn't been there #1 would have been easier for you.

Does that sound right?

I'll run this through E2E and approve/merge accordingly.

Thanks!

Copy link
Member

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jackfrancis jackfrancis merged commit 9d4e870 into Azure:master Mar 19, 2018
@carlpett
Copy link
Contributor Author

Ah, right.
We manually fixed our clusters this morning, so that's taken care of :)

@carlpett
Copy link
Contributor Author

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0.13.0 masters don't survive reboot

2 participants