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.

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

[JUJU-3738] - Fix/backend ref count #15654

Merged
merged 15 commits into from
Jun 2, 2023
Merged

Conversation

ycliuhw
Copy link
Member

@ycliuhw ycliuhw commented May 25, 2023

This PR fixes secret backend refCount issues for:

  • model import(increment on target controller)/export(decrement on source controller) during model migration;
  • model removal;
  • model migration abort;
  • secret drain;

drive-by:

  • added upgrade steps to create at least one refCount for each secret backend;
  • fixed a secret backend model config issue for model migration(including the backend ID in the model data for importing process to handle a potential backend renaming in the target controller);
  • fixed a potential data race for the secret drain worker that could happen if the secret backend changed too frequently in a short time period.

Checklist

  • Code style: imports ordered, good names, simple structure, etc
  • Comments saying why design decisions were made
  • Go unit tests, with comments saying what you're testing
  • Integration tests, with comments saying what you're testing
  • doc.go added or updated in changed packages

QA steps

juju add-secret-backend myvault vault --config ./vault.yaml

juju deploy snappass-test

for i in {1..5}; do juju exec --unit snappass-test/0  -- secret-add --owner unit owned-by=easyrsa/0; done

for i in {1..5}; do juju exec --unit snappass-test/0  -- secret-add owned-by=easyrsa; done

juju model-config secret-backend=myvault

juju model-config secret-backend
myvault

export backend_id=$(juju show-secret-backend myvault | yq .myvault.id)

juju bootstrap microk8s k2

juju add-secret-backend myvault1 vault --config ./vault.yaml --import-id $backend_id

juju switch k1:t1

juju migrate k1:t1 k2

juju switch k2:t1

juju model-config secret-backend
myvault1

# check refCount in mongo on the source controller;
juju:PRIMARY> db.globalRefcounts.find({_id: "secretbackend#revisions#64782241fe5c050027770143"}).pretty()
{
	"_id" : "secretbackend#revisions#64782241fe5c050027770143",
	"refcount" : 0,
	"txn-revno" : NumberLong(11)
}

# check refCount in mongo on the target controller;
juju:PRIMARY> db.globalRefcounts.find({_id: "secretbackend#revisions#64782241fe5c050027770143"}).pretty()
{
	"_id" : "secretbackend#revisions#64782241fe5c050027770143",
	"refcount" : 8,
	"txn-revno" : NumberLong(3)
}

juju remove-secret-backend myvault1
ERROR backend "myvault1" still contains secret content

juju destroy-model -y --debug --destroy-storage k2:t1

# check refCount in mongo on target controller again;
juju:PRIMARY> db.globalRefcounts.find({_id: "secretbackend#revisions#64782241fe5c050027770143"}).pretty()
{
	"_id" : "secretbackend#revisions#64782241fe5c050027770143",
	"refcount" : 0,
	"txn-revno" : NumberLong(3)
}

juju remove-secret-backend myvault1

Documentation changes

No

Bug reference

No

@ycliuhw ycliuhw changed the title Fix/backend ref count [JUJU-3738] - Fix/backend ref count May 25, 2023
@ycliuhw ycliuhw requested a review from wallyworld May 25, 2023 06:54
@hpidcock hpidcock added the 3.1 label May 26, 2023
@ycliuhw
Copy link
Member Author

ycliuhw commented Jun 1, 2023

/build

1 similar comment
@ycliuhw
Copy link
Member Author

ycliuhw commented Jun 1, 2023

/build

Copy link
Member

@wallyworld wallyworld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, just a couple of small things to fix.

state/secretbackends.go Outdated Show resolved Hide resolved
state/secrets.go Outdated Show resolved Hide resolved
state/secrets_test.go Show resolved Hide resolved
@ycliuhw
Copy link
Member Author

ycliuhw commented Jun 2, 2023

/merge

@jujubot jujubot merged commit 4e5ae3c into juju:3.1 Jun 2, 2023
18 of 20 checks passed
@wallyworld wallyworld mentioned this pull request Jun 8, 2023
jujubot added a commit that referenced this pull request Jun 8, 2023
#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
jujubot added a commit that referenced this pull request Jun 9, 2023
#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
jujubot added a commit that referenced this pull request Jun 9, 2023
#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants