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

Fix upgrade charm revision for application resources #414

Merged
merged 3 commits into from
Feb 28, 2024

Conversation

hmlanigan
Copy link
Member

@hmlanigan hmlanigan commented Feb 27, 2024

Description

Fix update charm revision for an application resource as done in the juju client. Resolve the charm as if it's being created instead so that revision and channel do not conflict. There are potential issues written out in the code. However it's a better solution that the current breakage. Juju changes are needed to fix correctly.

Additional validation added for updating a charm such as ensuring the you cannot update the revision and channel are both updated and that the new charm revision supports the architecture and operating system of the current charm revision.

Included new acceptance test writing method introduce by @anvial in a PR which hasn't landed yet. This cherry picked commit also has the test needed to verify this change.

Fixes: #413

Type of change

  • Bug fix (non-breaking change which fixes an issue)

QA steps

As documented in bug and in new acceptance test.

TF_ACC=1 TEST_CLOUD=lxd go test -v ./internal/provider/ -parallel=1 -run TestAcc_ResourceApplication

Additional notes

JUJU-5585

hmlanigan and others added 3 commits February 27, 2024 16:12
The provider has no way to determine which takes precendence.
Mold the origin used for charm update if a new revision is specified by
the user. Details in comments. Otherwise juju will find the latest
revision in the channel to use.

Also added checks to ensure that the requested revision supports the
architecture and operating system currently deployed.
Cherry picked commit, fixed typo in directory name and removed config
change from test as it won't work here. Will be added later.
@hmlanigan hmlanigan added this to the 0.11.0 milestone Feb 27, 2024
@hmlanigan hmlanigan assigned hmlanigan and unassigned hmlanigan Feb 27, 2024
Copy link
Member

@anvial anvial left a comment

Choose a reason for hiding this comment

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

Awesome! TY

Can we add the fix and check for Revision and Config ordering. Just to close Tom's BR.

I've prepared pr for you branch.
hmlanigan#1

@hmlanigan hmlanigan merged commit b3ce20c into juju:main Feb 28, 2024
16 of 23 checks passed
@cderici cderici mentioned this pull request Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update of an application revision fails
2 participants