Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
remote-relations: fix add/remove/add relation #7996
Conversation
| @@ -90,7 +90,9 @@ func FormatTabular(writer io.Writer, forceColor bool, value interface{}) error { | ||
| store = "unknown" | ||
| urlPath = app.OfferURL | ||
| } | ||
| - p(appName, app.StatusInfo.Current, store, urlPath) | ||
| + w.Print(appName) | ||
| + w.PrintStatus(app.StatusInfo.Current) |
| - } | ||
| - if err := w.remoteModelFacade.PublishRelationChange(change); err != nil { | ||
| - return errors.Annotatef(err, "publishing relation departed %+v to remote model %v", change, w.remoteModelUUID) | ||
| + if err := worker.Stop(relation.remoteRrw); err != nil { |
wallyworld
Oct 31, 2017
Owner
If we check localRuw is nil below, we should check is remoteRuw is nil here
axw
Oct 31, 2017
Member
that's unnecessary, relations[key] would be empty if remoteRuw were nil (also this is ~copy-and-paste from the old method)
| + relation.remoteRuw = nil | ||
| + delete(relations, key) | ||
| + | ||
| + // For the unit watchers, check to see if these are nil before stopping. |
| @@ -8,6 +8,7 @@ import ( | ||
| "gopkg.in/juju/charm.v6-unstable/hooks" | ||
| + "github.com/juju/errors" |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju |
jujubot
merged commit 459c62e
into
juju:develop
Oct 31, 2017
1 check passed
continuous-integration/jenkins/pr-merge
This commit looks good
Details
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
axw commentedOct 31, 2017
Description of change
Fix two issues with removal of remote relations:
for local units. This ensures that, at a minimum,
the relation scope documents do not get orphaned,
and prevents the relation from being removed too
early
remote relation workers; this can prevent the
propagation of relation unit departures. Instead,
stop the workers when the relation is removed
Also, as a drive-by, add colourisation to the SAAS
entries' status in "juju status".
QA steps
juju bootstrap localhost l1 & juju bootstrap localhost l2 juju deploy -m l1:default mediawiki & juju deploy -m l2:default mysql sleep 10s juju switch l2 juju offer mysql:db juju switch l1 juju relate mediawiki:db l2:admin/default.mysqlThen:
(check the hooks fire, relation removed)
(check relation re-added, hooks fire)
(check the hooks fire, relation removed)
(check relation re-added, hooks fire)
Documentation changes
None.
Bug reference
https://bugs.launchpad.net/juju/+bug/1727162