search

juju-solutions / interface-etcd

1 stars 5 forks source link

Adds a peer-departed state #5

Closed lazypower closed 8 years ago

lazypower commented 8 years ago

Related to https://github.com/juju-solutions/layer-etcd/issues/5

lazypower commented 8 years ago

I'm on the fence about this. This code in the interface works as is, but @johnsca says the -broken relation in reactive is broken. I'm only using the state as it gets fired on the single machine departing the cluster, which triggers self destruction, i mean, de-registration.

mbruzek commented 8 years ago

@johnsca What is the problem with the "-broken" hook that @chuckbutler talks about in the comment above?

mbruzek commented 8 years ago

I would think you would want to do this in the "-departed" hook, giving the code time to unregister with the cluster. My understanding of "-broken" is they relationship is completely severed. I am not against using "-broken" here, but this looks like a perfect example of "-departed" to me.

The relevant code reactive code that uses this state: https://github.com/juju-solutions/layer-etcd/blob/master/reactive/etcd.py#L315

mbruzek commented 8 years ago

@chuckbutler pointed me to the conversation about this bug/problem: https://lists.ubuntu.com/archives/juju/2015-December/006245.html

I am +1 to this code if it works (as I am sure it does), but I am concerned as my understanding of the "-departed" is appropriate here. We would want each unit to run some code to remove the dying unit from their "peer list" configuration, as well as the dying unit to "unregister" itself from the master/leader or cluster.

johnsca commented 8 years ago

LGTM

mbruzek commented 8 years ago

+1 to this change, I like the removal of not_unless and making it easy to understand what is going on.