-
Notifications
You must be signed in to change notification settings - Fork 7
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
Upgrade the Mediator's ACA-Py instance to Release 0.8.1 #25
Comments
The upgrade should be run pretty soon after the upgrade. It just re-saves the connection records (in order to add a new tag) - I'm not sure what is the impact if the new tags aren't available, possibly it won't be able to find the connection it's looking for, |
I'm not sure it matters, but better to be safe than sorry. I think this search only happens in the midst of the invitation process, and the fix is to return only the invitation uses that are at that state. |
I'm getting an error running the upgrade:
I don't see that file or the |
The default path is setup here: https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/commands/upgrade.py#L25 You should be able to run the upgrade without providing a |
|
|
Hmmm that's an issue with the docker build I think ... |
Do you have a way to manually copy the file to the mediator image? |
The issue looks more like and issue with the |
Yeah, just a pain. |
I'll create an issue in the aca-py repo to make sure this file getting included in the release |
Thanks |
Running into issues with the upgrade. Seems to be connectivity issues with the wallet database. Investigating. Edit: Issue was security policy related, the required labels were not applied to the debug pod I'm using.
|
Error is now:
Looks like you can only specify versions that are explicitly listed in the upgrade configuration yaml. |
This seemed to work:
|
Any notes about an upgrade to the documentation on upgrade would be appreciated. Current “docs” are here in the Changelog for the 0.8.0 release. I don’t see any general documentation on “Upgrade”, but I didn’t look too hard. It sounds like we need to do a new 0.8.1 immediately with the missing file. Please let me know what needs to be in that file. Please put details in: openwallet-foundation/acapy#2178 Or open a PR to add the necessary documents that would have helped you. Thanks. |
The only tripping points were the |
Our Aries Mediator Service |
I've run into a problem upgrading the
@ianco, Any thoughts? |
It seems to think the wallet database is already at version |
The error message is the same as we saw with the test failure, right? Weird that this was not hit in Dev — such fun. Is it because we were running 0.8.0-rc1 there? It’s considered the same as 0.8.0? |
We need to relax the rules on the upgrade. This upgrade is non-destructive (it just re-saves the connection records to update the tags) so shouldn't be an issue if it's re-run. However it's weird that the storage version was changed to Not sure about the |
Both |
Given that the YML file has never shipped with ACA-Py, it’s doubtful an upgrade has ever been run. @ianco, can you get the code fixed to work, and explained? |
I suggest:
Maybe also look at the logic that determines whether the upgrade should run - it looks like it expects the exact |
@WadeBarnes — can you try running the command with |
I did. Because the version is recorded in storage, it ignores anything you pass in via You get:
where |
Can you tell from the code (a) what goes into the storage, and (b) if the comparison that is being used would detect 0.8.0-rc0 as later than 0.8.0? I’m certain that the value is If a value is recorded in secure storage, we should be able to see that, don’t you think? We don’t really need an unencrypted tag if we assume that no one is sneaking around updating it another way. |
The version record in the wallet is updated here, in the upgrade command: It looks like it just saves the version from There's a helper method here for updating the version in the wallet but it's not called anywhere: ... and it looks like the Conductor will bail on startup if the version doesn't match: (... unless there is no version record in the wallet, in which case aca-py will startup happily) |
All the contents of the wallet are encrypted so we can't inspect anything ... we can't connect to the wallet database directly and see anything, and the version info stored in the wallet isn't exposed anywhere via the api |
Just realized @WadeBarnes that you used The “old” (from) version either comes from Storage, or if not found, comes from the It also looks like the version is never added to storage unless an upgrade has been run at least once — the add record is only (AFAICS) in the upgrade command. And I certainly can’t see that the value Frustrating... |
I mean — we should be able to see where the data is updated in the code such that we don’t have to see the literal value. |
Sure but that doesn't tell us what version is currently running, for example what is the storage version in the production mediator right now? We can't tell ... |
The only way to get the version into storage is to run the upgrade, and since it is pretty unlikely an upgrade has been run on this instance before, there should be no version in storage. How is that routine returning a value? This seems like the same issue we had with the integration test just before release. What did you do to fix that @ianco? Is it possible we’re seeing the same thing? |
No, having to specify an explicit version contained in a file you can't see easily does not make sense. I mentioned that here. I had originally specified I'm thinking it would be way better if I'm a little concerned now about the Conductor bailing on startup when the version in storage does not match the running version. Our automated upgrades have been working to this point because we've never done and upgrade, so there is no version record in storage. Now that we've done an upgrade the version is in storage so all future automated deployments to our (say |
I think we need to be working together on this on a call. I’ll add @shaangill025. @ianco — not sure if you are working today or not, so you may not be able to join. Completely agree with you about bailing on startup if the upgrade is not done. That seems like a bad idea. But at the same time, if the upgrade is executed by 5 instances of ACA-Py on the same storage, what will happen…? For now, we need to understand why the query of secure storage is saying we are on version 0.8.0, when an upgrade has never been run... |
The version likely got in there when I ran the upgrade the first time. What I posted was not actually the first run. I had originally run it with The result at that point was something like this simulated log output:
I then tried running the upgrade command with |
OK — that’s good to know. And that command would have happily skipped the update and updated the storage version. Phew…we know how we got here. Now we need to decide what we need to do, and what to do for the long term. I think we switch the logic on the check such that the The bigger question that perhaps we don’t have to decide today is how we fix the automated deployment issue that Wade highlights above. That needs safe automation. Meeting set for this morning. |
After taking a look at the code for
If there was no version record in storage initially then running with |
Issue: How to force an upgrade:
Issue: What if the from-version is not in the
Issue: Once a version is in storage, cannot move to a newer version without running the
Issue: On a new deployment, there is no version put into storage.
Issue: No version in storage in existing deployments.
Issue: No warning currently produced when version in storage is not found.
Issue: Can we run an upgrade automatically when deploying a new version. Current behaviour is to abort the startup.
Issue: The current expectation is that in the YML, the version for the change is the "from" -- e.g. when upgrading from a designated release.
|
All mediator instances have been successfully upgrade to |
Please do a normal upgrade (through Dev, Test and then Prod) of the ACA-Py released used in the BC Wallet mediator to release 0.8.0. This will pick up change PR #2147 - Fix: messages stuck in mediator.
Once done, pleaseDuring the upgrade, it is required to apply the upgrade outlined in the 0.8.0 Release notes to do with PR #2116. Suggest confirming with an ACA-Py dev that the upgrade can be applied later versus as the update is made. Here are the upgrade notes from the 0.8.0 Release Notes about this:The text was updated successfully, but these errors were encountered: