Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Stop workers when relation is suspended #7862
Conversation
| @@ -37,7 +37,7 @@ func PublishRelationChange(backend Backend, relationTag names.Tag, change params | ||
| // Update the relation suspended status. | ||
| currentStatus := rel.Suspended() | ||
| - if change.Suspended != nil && currentStatus != *change.Suspended { | ||
| + if !dyingOrDead && change.Suspended != nil && currentStatus != *change.Suspended { |
axw
Sep 19, 2017
Member
does a relation have to be unsuspended before it can be removed? just wondering why we would prevent this on Dying
wallyworld
Sep 19, 2017
Owner
Dying takes precedence. If a relation were suspended for example, and then someone removes it, we just want it to die. There's no need for any further processing of suspended status.
| - Assert: bson.D{{"life", Alive}}, | ||
| - Update: bson.D{{"$inc", bson.D{{"unitcount", -1}}}}, | ||
| - }) | ||
| + if ru.checkUnitLife { |
axw
Sep 19, 2017
Member
the txn.Op below isn't just checking life, it's also decrementing unitcount. I don't think you want to stop decrementing...
wallyworld
Sep 19, 2017
Owner
We do because the same code prevents the increment in enter scope. Without this code, the unitcount gets out of sync. For ru's belonging to the remote app side, we don't want to monitor unit count at all.
| @@ -1194,10 +1194,23 @@ func (fw *Firewaller) relationLifeChanged(tag names.RelationTag) error { | ||
| } | ||
| rel := results[0].Result | ||
| - dead := notfound || rel.Life == params.Dead | ||
| + dead := notfound || rel.Life == params.Dead || rel.Suspended |
axw
Sep 19, 2017
Member
please rename to something more appropriate, it no longer means "dead" (removed implies it was dead, so that one was OK)
alternatively just check known && (dead || rel.Suspended). you check rel.Suspended inside anyway
| - Assert: bson.D{{"life", Alive}}, | ||
| - Update: bson.D{{"$inc", bson.D{{"unitcount", -1}}}}, | ||
| - }) | ||
| + if ru.checkUnitLife { |
axw
Sep 19, 2017
Member
the txn.Op below isn't just checking life, it's also decrementing unitcount. I don't think you want to stop decrementing...
wallyworld
Sep 19, 2017
Owner
We do because the same code prevents the increment in enter scope. Without this code, the unitcount gets out of sync. For ru's belonging to the remote app side, we don't want to monitor unit count at all.
wallyworld
deleted a comment from
jujubot
Sep 19, 2017
|
$$merge$$ |
|
Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju |
wallyworld commentedSep 19, 2017
•
Edited 1 time
-
wallyworld
Sep 19, 2017
Description of change
When a relation is suspended, stop the workers which watch for unit settings changes. Leave the watcher for life/status changes active so that if/when the relation resumes, things can be restarted.
Similarly, the firewaller will immediately revoke ingress access for suspended relations.
As a driveby, when a relation unit leaves scope, do not decrement the unit count if it is for the cross model end of a relation. We already did this on enter scope.
QA steps
bootstrap a CMR scenario.
suspend a relation and ensure that the firewall ports on the offering side are closed and that the relation is properly suspended.
resume the relation and ensure that everything hooks up again as expected
suspend the relation and ensure it can be deleted while suspended, from either model