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-1494] Fast refresh charm #14368

Merged
merged 3 commits into from
Jul 23, 2022
Merged

Conversation

hmlanigan
Copy link
Member

Check charm origin ID before attempting to refresh charmhub charms. Prevents a case where refresh called before charm is downloaded.

Account for change to refresh in updated run_deploy_revision_upgrade.

Drive by fix of run_deploy_revision_fail.

QA steps

(cd tests ; ./main.sh -v deploy test_deploy_revision )

Avoid timing window where the ID hasn't been set as the charm hasn't
been downloaded yet. A very small window.
Adjust error message on revision without channel to match juju 3.0
@anvial
Copy link
Member

anvial commented Jul 22, 2022

Tested locally: https://pastebin.canonical.com/p/DpkkpJC2Xg/

Test passed successfully.

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.

Seems ok as a first step to deal with the issue. Ideally we'd refresh internally but being explicit to the user about the status is good to get started.

@hmlanigan
Copy link
Member Author

Seems ok as a first step to deal with the issue. Ideally we'd refresh internally but being explicit to the user about the status is good to get started.

I thought about it, but seemed over engineering at this point. As the code comment says, we can add retry.

@hmlanigan
Copy link
Member Author

Unfortunately we cannot check on the server side too, as AddCharm is used by both deploy and refresh.

@hmlanigan
Copy link
Member Author

/merge

1 similar comment
@wallyworld
Copy link
Member

/merge

@jujubot jujubot merged commit 6e28a9b into juju:develop Jul 23, 2022
@hmlanigan hmlanigan deleted the fast-refresh-charm branch July 25, 2022 13:07
jujubot added a commit that referenced this pull request Oct 19, 2022
…regression

#14788

A few things were lost in merges from 2.9 into develop or 3.0, bring them back.
* loop to ensure that deploy by revision refresh is working with async charm download, from #14368
* latest vs default track, from #14470

## QA steps

*Commands to run to verify that the change works.*

From #14470:
```sh
$ juju deploy mysql-router
Located charm "mysql-router" in charm-hub, revision 34
Deploying "mysql-router" from charm-hub charm "mysql-router", revision 34 in channel 8.0/stable on focal
$ juju deploy mysql-router --channel latest mysql-router-latest
Located charm "mysql-router" in charm-hub, revision 15
Deploying "mysql-router-latest" from charm-hub charm "mysql-router", revision 15 in channel latest/stable on focal

# the channel here should be latest/stable, however there is a bug in charmhub server right now.
$ juju deploy ubuntu
Located charm "ubuntu" in charm-hub, revision 20
Deploying "ubuntu" from charm-hub charm "ubuntu", revision 20 in channel stable on focal
```

Other QA
```sh
(cd tests ; ./main.sh -v deploy test_deploy_revision)
(cd tests ; ./main.sh -v deploy test_deploy_bundles)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants