Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[JUJU-3968] Fix token generation for offered apps that also consume #15714

Merged
merged 1 commit into from
Jun 8, 2023
Merged

[JUJU-3968] Fix token generation for offered apps that also consume #15714

merged 1 commit into from
Jun 8, 2023

Conversation

wallyworld
Copy link
Member

@wallyworld wallyworld commented Jun 8, 2023 •
edited
Loading

Fixes a cross model relations corner case:

Model 1
app A
offer app A

Model 2
app B
app C
offer app C

Model 1
relate A->C

Model 2
relate B->A

The unit relation hooks for C do not fire.
The root cause is that only one cmr token for app A is created and used for both the offer relation and consumer relation. So model 2 gets a duplicate remote entity token and the resolution of token to entity tag fails for the C->A relation.

The fix is to create the token for offering relations keyed on application offer tag, not application tag. This allows the same app to be in both an offering and consuming relation to a given model since different tokens are created for the different roles of the app.

There was also an unused app token parameter in the ingress address change watcher which was removed. This is lucky as it avoids the need to look up offers in the firewaller worker.

QA steps

I used modified dummy-source and dummy-sink charms with extra endpoints to set up a cross model scenario as per the description. After the relations were created, show-status-log on each of the various units showed relation created and joined hooks being run. Previously, the relation joined hooks for one of the units would have been missing.

Bug reference

https://bugs.launchpad.net/juju/+bug/2022855

hpidcock
hpidcock approved these changes Jun 8, 2023
View reviewed changes
worker/firewaller/firewaller.go Outdated Show resolved Hide resolved
worker/firewaller/firewaller.go Outdated Show resolved Hide resolved
apiserver/common/crossmodel/crossmodel.go Outdated Show resolved Hide resolved
@wallyworld
Copy link
Member Author

/merge

@jujubot jujubot merged commit 6390036 into juju:2.9 Jun 8, 2023
17 checks passed
@wallyworld wallyworld mentioned this pull request Jun 8, 2023
Merged
jujubot added a commit that referenced this pull request Jun 8, 2023
@jujubot
Merge pull request #15716 from wallyworld/merge-2.9-20230608
e7ecd93 
#15716

Merge 2.9

[Merge pull request](6390036) #15714 [from wallyworld/offer-consume=sameapp](6390036)

Conflicts was new auth param.

```
# Conflicts:
# apiserver/common/crossmodel/crossmodel.go
# apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go
# apiserver/facades/controller/remoterelations/remoterelations.go
#
```

## QA steps

See PRs
@wallyworld wallyworld mentioned this pull request Jun 8, 2023
Merged
jujubot added a commit that referenced this pull request Jun 8, 2023
@jujubot
Merge pull request #15717 from wallyworld/merge-3.1-20230608
b53560a 
#15717

Merge 3.1

#15676 [from wallyworld/newer-clients-migrate](7c1f884)
#15672 [from hpidcock/bump-juju-description-v3.0.15](acec126)
#15683 [from hpidcock/fix-persistent-storage-test](460dd21)
#15673 [from barrettj12/check-merge](5c253c3)
#15677 [from barrettj12/invalid-offer](3cb3f8b)
#15654 [from ycliuhw/fix/backendRefCount](4e5ae3c)
#15692 [from wallyworld/fix-secrets-cmr](a1fb0c4)
[t](840bc09) #15709 [from wallyworld/hook-secret-revison](840bc09)
#15701 [from hpidcock/fix-upgrade-podspec-sidecar](d465c93)
#15681 [from anvial/JUJU-3882-fix-test-deploy-test-…](f54c1ad)
#15714 [from wallyworld/offer-consume=sameapp](6390036)

Conflicts were upgrade steps - the 3.1.3 step has been moved to 3.2.1.
Also an auth tweak to crossmodelrelaltions.

```
# Conflicts:
# apiserver/common/crossmodel/auth_test.go
# apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go
# rpc/params/apierror.go
# state/upgrades.go
# state/upgrades_test.go
# upgrades/backend.go
#
```

## QA steps

See PRs
@wallyworld wallyworld mentioned this pull request Jun 9, 2023
Merged
jujubot added a commit that referenced this pull request Jun 9, 2023
@jujubot
Merge pull request #15719 from wallyworld/20230609
78b635d 
#15719

Merge 3.2

No conflicts. Upgrade steps were deleted.

#15636 [from jack-w-shaw/JUJU-3822_replace_ubuntu_w…](1a2759d)
#15650 [from SimonRichardson/fix-machine-test-logs-aws](c316413)
#15652 [from barrettj12/ojson](4436840)
#15653 [from barrettj12/teardown-logs](ee886dd)
#15633 [from anvial/JUJU-3821-fix-test-deploy-aks-c…](0bb6df2)
#15657 [from anvial/JUJU-3842-fix-deploy-aks-clean-…](f16dd80)
#15660 [from hpidcock/ignore-delete-network-interface](686b6f2)
#15661 [from hpidcock/fix-data-race-getosfromseries](f639f09)
#15662 [from hpidcock/fix-relation-departing-test](68ef945)
#15663 [from hpidcock/fix-test-branch](c14bcb3)
#15313 [from tlm/teardown-2.0](243fd66)
#15542 [from masnax/project-fix](8ba03ec)
#15676 [from wallyworld/newer-clients-migrate](7c1f884)
#15672 [from hpidcock/bump-juju-description-v3.0.15](acec126)
#15683 [from hpidcock/fix-persistent-storage-test](460dd21358378e7d1204348c37a9ba26e49b5871)https://github.com/juju/juju/pull/15673 [from barrettj12/check-merge](5c253c3)
#15677 [from barrettj12/invalid-offer](3cb3f8b)
#15654 [from ycliuhw/fix/backendRefCount](4e5ae3c)
#15692 [from wallyworld/fix-secrets-cmr](a1fb0c4)
#15709 [from wallyworld/hook-secret-revison](840bc09)
#15701 [from hpidcock/fix-upgrade-podspec-sidecar](d465c93)
#15681 [from anvial/JUJU-3882-fix-test-deploy-test-…](f54c1ad)
#15573 [from jack-w-shaw/JUJU-3447_implement_ModelF…](f7082ca)
#15714 [from wallyworld/offer-consume=sameapp](6390036)

## QA steps

See PRs
@wallyworld wallyworld mentioned this pull request Jun 9, 2023
Merged
jujubot added a commit that referenced this pull request Jun 9, 2023
@jujubot
Merge pull request #15721 from wallyworld/merge-3.3-20230609
541db75 
#15721

Merge 3.3


#15636 [from jack-w-shaw/JUJU-3822_replace_ubuntu_w…](1a2759d)
#15650 [from SimonRichardson/fix-machine-test-logs-aws](c316413)
#15652 [from barrettj12/ojson](4436840)
#15653 [from barrettj12/teardown-logs](ee886dd)
#15633 [from anvial/JUJU-3821-fix-test-deploy-aks-c…](0bb6df2)
#15657 [from anvial/JUJU-3842-fix-deploy-aks-clean-…](f16dd80)
#15660 [from hpidcock/ignore-delete-network-interface](686b6f2)
#15661 [from hpidcock/fix-data-race-getosfromseries](f639f09)
#15662 [from hpidcock/fix-relation-departing-test](68ef945)
#15663 [from hpidcock/fix-test-branch](c14bcb3)
#15313 [from tlm/teardown-2.0](243fd66)
#15542 [from masnax/project-fix](8ba03ec)
#15676 [from wallyworld/newer-clients-migrate](7c1f884)
#15672 [from hpidcock/bump-juju-description-v3.0.15](acec126)
#15683 [from hpidcock/fix-persistent-storage-test](460dd21358378e7d1204348c37a9ba26e49b5871)https://github.com/juju/juju/pull/15673 [from barrettj12/check-merge](5c253c3)
#15677 [from barrettj12/invalid-offer](3cb3f8b)
#15654 [from ycliuhw/fix/backendRefCount](4e5ae3c)
#15692 [from wallyworld/fix-secrets-cmr](a1fb0c4)
#15709 [from wallyworld/hook-secret-revison](840bc09)
#15701 [from hpidcock/fix-upgrade-podspec-sidecar](d465c93)
#15681 [from anvial/JUJU-3882-fix-test-deploy-test-…](f54c1ad)
#15573 [from jack-w-shaw/JUJU-3447_implement_ModelF…](f7082ca)
#15714 [from wallyworld/offer-consume=sameapp](6390036)

Conflicts
```
# Conflicts:
# apiserver/facades/client/client/client_test.go
# featuretests/package_test.go
#
```

## QA steps

See PRs
@manadart manadart changed the title [JUJU-3968] Fix token generation for offered apps which also consume [JUJU-3968] Fix token generation for offered apps that also consume Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hpidcock hpidcock hpidcock approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@wallyworld @hpidcock @jujubot