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

Conversation

@wuqixuan
Copy link
Contributor

@wuqixuan wuqixuan commented Jul 3, 2015

Now support detecting the existing machine-id on startup.

Fixes #1241 #615

heart/heart.go Outdated

Choose a reason for hiding this comment

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

Grammar: s/exist/exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, has grammar issue, already updated.

server/server.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Add some context to this error message. Perhaps "Failed machine heartbeat: %v"

Now support detecting the existing machine-id on startup.

Fixes coreos#1241 coreos#615
@wuqixuan
Copy link
Contributor Author

@bcwaldon ,totally agree with your comments, updated as per your suggestion. Currently the result is like:

localhost fleet # ./fleetd&
[2] 610
localhost fleet # INFO fleetd.go:64: Starting fleetd version 0.10.2+git
INFO fleetd.go:165: Using default config file /etc/fleet/fleet.conf
INFO server.go:157: Establishing etcd connectivity
ERROR server.go:165: Server register machine failed: 105: Key already exists (/_coreos.com/fleet/machines/06ecd4f7fc964b1f9ee07e89a48dbcfc/object) [31455]
ERROR server.go:165: Server register machine failed: 105: Key already exists (/_coreos.com/fleet/machines/06ecd4f7fc964b1f9ee07e89a48dbcfc/object) [31455]
ERROR server.go:165: Server register machine failed: 105: Key already exists (/_coreos.com/fleet/machines/06ecd4f7fc964b1f9ee07e89a48dbcfc/object) [31456]
ERROR server.go:165: Server register machine failed: 105: Key already exists (/_coreos.com/fleet/machines/06ecd4f7fc964b1f9ee07e89a48dbcfc/object) [31459]

@wuqixuan
Copy link
Contributor Author

@bcwaldon , the solution with my latest code has one problem. If the fleetd is killed by sigkill which can not be catched, the fleetd's registration will always be failing.
Please don't merge now. I will think over in these two days.

@wuqixuan
Copy link
Contributor Author

Gently ping @jonboulle , I thought over again about the modification. I think the solution is fine. If the fleetd is killed by sigkill which can not be catched, the fleetd's registration will fail until the TTL is timeout. So my code is correct I think.

@jonboulle
Copy link
Contributor

This seems pretty reasonable, but since it's quite an important change, can we get a functional test for it?

@kayrus kayrus added this to the v0.13.0 milestone Apr 18, 2016
dongsupark pushed a commit to endocode/fleet that referenced this pull request Apr 19, 2016
A new test TestDetectMachineId checks if a etcd registration fails
when a duplicated entry for /etc/machine-id gets registered to
different machines. Note that it's expected to fail in this case.

Goal of the test is to cover the improvement patch by @wuqixuan
("fleetd: Detecting the existing machine-id").

See also coreos#1288,
coreos#1241,
coreos#615.

Suggested-by: Olaf Buddenhagen <[email protected]>
Cc: wuqixuan <[email protected]>
dongsupark pushed a commit to endocode/fleet that referenced this pull request Apr 20, 2016
A new test TestDetectMachineId checks if a etcd registration fails
when a duplicated entry for /etc/machine-id gets registered to
different machines. Note that it's expected to fail in this case.

Goal of the test is to cover the improvement patch by @wuqixuan
("fleetd: Detecting the existing machine-id").

See also coreos#1288,
coreos#1241,
coreos#615.

Suggested-by: Olaf Buddenhagen <[email protected]>
Cc: wuqixuan <[email protected]>
dongsupark pushed a commit to endocode/fleet that referenced this pull request Apr 21, 2016
A new test TestDetectMachineId checks if a etcd registration fails
when a duplicated entry for /etc/machine-id gets registered to
different machines. Note that it's expected to fail in this case.

Goal of the test is to cover the improvement patch by @wuqixuan
("fleetd: Detecting the existing machine-id").

See also coreos#1288,
coreos#1241,
coreos#615.

Suggested-by: Olaf Buddenhagen <[email protected]>
Cc: wuqixuan <[email protected]>
dongsupark pushed a commit to endocode/fleet that referenced this pull request Apr 21, 2016
A new test TestDetectMachineId checks if a etcd registration fails
when a duplicated entry for /etc/machine-id gets registered to
different machines. Note that it's expected to fail in this case.

Goal of the test is to cover the improvement patch by @wuqixuan
("fleetd: Detecting the existing machine-id").

See also coreos#1288,
coreos#1241,
coreos#615.

Suggested-by: Olaf Buddenhagen <[email protected]>
Cc: wuqixuan <[email protected]>
Cc: Djalal Harouni <[email protected]>
dongsupark pushed a commit to endocode/fleet that referenced this pull request Apr 21, 2016
A new test TestDetectMachineId checks if a etcd registration fails
when a duplicated entry for /etc/machine-id gets registered to
different machines. Note that it's expected to fail in this case.

Goal of the test is to cover the improvement patch by @wuqixuan
("fleetd: Detecting the existing machine-id").

See also coreos#1288,
coreos#1241,
coreos#615.

Suggested-by: Olaf Buddenhagen <[email protected]>
Cc: wuqixuan <[email protected]>
Cc: Djalal Harouni <[email protected]>
@tixxdz
Copy link
Contributor

tixxdz commented Apr 21, 2016

Fixed by #1561

Closing, thanks @wuqixuan for the patches.

@tixxdz tixxdz closed this Apr 21, 2016
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.

6 participants