When an offer is consumed a second time, delete the old proxy #13354
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
benhoyt
approved these changes
Sep 23, 2021
apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go
Outdated
Show resolved
Hide resolved
wallyworld
force-pushed
the
refresh-consume-proxy
branch
from
834538d
to
7616f39
Compare
|
|
1 similar comment
|
|
jujubot
added a commit
that referenced
this pull request
Sep 28, 2021
#13361 Merge from 2.9 to bring forward: - #13360 from wallyworld/simplestreams-compression - #13359 from manadart/2.9-lxd-container-images - #13352 from tlm/aws-instance-profile - #13358 from jujubot/increment-to-2.9.16 - #13354 from wallyworld/refresh-consume-proxy - #13353 from wallyworld/cmr-consume-fixes - #13346 from SimonRichardson/raft-api-client - #13349 from wallyworld/remove-orphaned-cmrapps - #13348 from benhoyt/fix-secretrotate-tests - #13119 from SimonRichardson/pass-context - #13342 from SimonRichardson/raft-facade - #13341 from ycliuhw/feature/quay.io Conflicts (easy resolution): - apiserver/common/crossmodel/interface.go - apiserver/errors/errors.go - apiserver/params/apierror.go - apiserver/testserver/server.go - scripts/win-installer/setup.iss - snap/snapcraft.yaml - version/version.go
jujubot
added a commit
that referenced
this pull request
Feb 3, 2023
…e-version #15145 Under #13354, we added `ConsumeVersion` to remote application proxies in order that we could detect when an application with the same name consumed an offer a second time. However, this new member was never added to the `desciption` package, and so was dropped and defaulted to 0 (zero) during model migrations. This meant that when migrated CMRs were re-established post migration, we hit [this condition](https://github.com/juju/juju/blob/2c7465f373c463e3b0d2421a4847ef20ee320b84/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go#L224), ensuring that the existing remote application was force-destroyed before being re-created. This logic by itself is fine, but deep in the logic for force deletion we find [this](https://github.com/juju/juju/blob/5132716900d95187e8f7e90809f53876e6e52a8b/state/relation.go#L487), which schedules a cleanup (force deletion) of the relation. The cumulative effect has been for migrated CMRs to mysteriously disappear soon after migration completion. Here, we update to the latest version of the `description` dependency, which includes the `ConsumeVersion` member, and ensure that it is copied over during migration. This prevents the clean-up of associated relations following migration. ## QA steps - Set up the offer, consumer and targets. ``` for c in "cns" "src" "dst"; do juju bootstrap lxd $c --no-gui; done juju switch src; juju deploy ./testcharms/charms/dummy-source --config token=INITIAL; juju offer dummy-source:sink juju switch cns; juju consume src:admin/default.dummy-source; juju deploy ./testcharms/charms/dummy-sink; juju relate dummy-source dummy-sink juju switch dst; juju destroy-model default -y ``` - Switch back to _src_ and wait for quiescence. - `juju migrate default dst`, and wait for the migration to complete. - `juju switch dst; juju config dummy-source token=DIFFERENT`. - `juju switch cns`, then check via status that the token value change ends up reflected by _dummy-sink_. ## Documentation changes None. ## Bug reference https://bugs.launchpad.net/juju/+bug/2003708
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The offering model creates a proxy for the consuming app. If the consuming side does a
remove-saasand then consumes the same offer again, the offering model might retain stale info, especially if it is down then the consuming side does a force delete.This PR uses an incrementing version on the consuming side. If the offering side gets a new connection and the version is newer than what it has, it force deletes the consuming proxy and starts again. This is analogous to what is done on the consuming side then the offer is force removed and then consumed again.
This is same to do without a facade bump because the default version will be 0.
QA steps
I tested this on top of #13353 but rebased against 2.9 to make this PR.
Deploy a cmr scenario like in #13349 and on the consuming side, remove the saas app and consume again.
Ensure relation hooks are run on both sides of the relation.
Bug reference
https://bugs.launchpad.net/charm-ceph-mon/+bug/1940983