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

Bundle diff output with charm url names #12641

Merged
merged 3 commits into from Feb 18, 2021

Conversation

SimonRichardson
Copy link
Member

The following allows the diff'ing of bundles when using charmhub charm
urls. The internal representation of charmhub urls are different from
what users of juju see. So prefix, arch, series charm urls should not be
shown and therefor should not be used for diff'ing exported bundles.

The code is actually simple, use the charmurl name for charmhub charms
and just update to the latest library changes. With the addition of a
few tests, the following should be possible:

$ juju diff-bundle <(juju export-bundle)
{}

QA steps

$ juju bootstrap lxd test
$ juju deploy ubuntu --channel="latest/edge"
$ juju diff-bundle <(juju export-bundle)
{}

Bug reference

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

The following allows the diff'ing of bundles when using charmhub charm
urls. The internal representation of charmhub urls are different from
what users of juju see. So prefix, arch, series charm urls should not be
shown and therefor should not be used for diff'ing exported bundles.

The code is actually simple, use the charmurl name for charmhub charms
and just update to the latest library changes. With the addition of a
few tests, the following should be possible:

    $ juju diff-bundle <(juju export-bundle)
    {}
@aieri
Copy link

aieri commented Feb 17, 2021

With the addition of a
few tests, the following should be possible:

$ juju diff-bundle <(juju export-bundle)
{}

Kind of a sidenote, but juju diff-bundle <(juju export-bundle) is already not providing the expected result due to https://bugs.launchpad.net/juju/+bug/1913631 (independently from charmhub url issues)

@SimonRichardson
Copy link
Member Author

@aieri that should be fixed here #12638

@hmlanigan
Copy link
Member

hmlanigan commented Feb 17, 2021

From QA: export bundle is exporting the risk only, not the track/risk combo where needed.

$ juju deploy ubuntu-juju-qa --channel 2.0/stable
Located charm "ubuntu-juju-qa" in charm-hub, revision 12
Deploying "ubuntu-juju-qa" from charm-hub charm "ubuntu-juju-qa", revision 12 in channel 2.0/stable
$ juju export-bundle
...
ubuntu-juju-qa:
charm: ubuntu-juju-qa
channel: stable
series: bionic
...

This can be a follow on PR. Makes me wonder if bundle-diff is working correctly with channels too. "edge" != "2.0/edge"

@hmlanigan
Copy link
Member

Partial fix for https://bugs.launchpad.net/juju/+bug/1915844 also

Copy link
Member

@hmlanigan hmlanigan left a comment

Choose a reason for hiding this comment

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

QA on charm piece good.

cmd/juju/application/diffbundle_test.go Outdated Show resolved Hide resolved
@hpidcock hpidcock added the 2.9 label Feb 18, 2021
The following ensures that we correctly handle bundles. Charm channel
from status is now used to help guide that through to the creation of a
bundle model, so that we can then apply a diff against it.

The code is simple, it's just pushing properties through the layers.
@SimonRichardson
Copy link
Member Author

$$merge$$

@jujubot jujubot merged commit 8950c48 into juju:2.9 Feb 18, 2021
@SimonRichardson SimonRichardson deleted the diff-bundle-with-channel branch February 18, 2021 12:50
@wallyworld wallyworld mentioned this pull request Feb 19, 2021
jujubot added a commit that referenced this pull request Feb 22, 2021
#12657

Merge 2.9 with these PRs:

#12566 Make `coreos/go-systemd' buildable on windows
#12594 [Charmhub] - `Juju info` updates based on upcoming api changes.
#12596 Pebble support with *-workload-ready hook.
#12602 Static analysis checks
#12604 [2.9] Do not attempt to set proxy settings for windows/centos workloads
#12605 [2.9] Use positional arguments for azure clients
#12607 Clean up model migration code
#12608 Allow bundle to update existing offers
#12609 EC2 fix panic
#12614 Errcheck linter fixes
#12615 Allow bundle to update existing offers
#12617 Fixes permission for model operator.
#12619 Add set-series integration test
#12620 Fix asserts used when finalising an action
#12621 Relocates DNS and resolve.conf parsing from network to core/network
#12622 Move NetworkConfigSource from api/common to core/network
#12623 Indirection for network config sourced addresses
#12627 Ensure relation units
#12629 Record why a cloud credential gets marked invalid and expose via show-model, status
#12630 Fix double upgraded
#12633 Update how channel is created in ApplicationsInfo.
#12634 Fix uniter loop when a relation is removed out of bandge case.
#12637 Improve maas cli output handling in acceptance tests
#12638 Bundle export with channel
#12641 Bundle diff output with charm url names
#12643 Expose charm channel in juju status
#12644 Fix service type translation for k8s applications
#12647 Fix acceptance test bug in get_uptime
#12650 Ensure we normalize the charm url in bundle handler
#12651 Use python 3.6 compat fix for handling maas cli output
#12652 Use python 3.6 compatible way to capture subprocess stdout/err output
#12653 missing container status of creating
#12654 Move 2.8.10 upgrade step to 2.8.9


```
# Conflicts:
# api/backups/restore.go
# apiserver/facades/client/backups/restore.go
# caas/kubernetes/provider/operator.go
# cloudconfig/podcfg/image.go
# cmd/containeragent/main_nix.go
# cmd/containeragent/main_test.go
# cmd/containeragent/unit/agent.go
# cmd/containeragent/unit/agent_test.go
# cmd/juju/action/run.go
# cmd/juju/action/showoutput.go
# cmd/juju/commands/exec.go
# cmd/juju/commands/exec_test.go
# go.mod
# go.sum
# juju/testing/conn.go
# state/action_test.go
# state/applicationoffers.go
# state/backups/files.go
# state/backups/restore_test.go
# state/upgrades.go
# worker/machineactions/worker.go
# worker/restorewatcher/manifold.go
# worker/restorewatcher/worker.go
# worker/uniter/runner/jujuc/mocks/context_relation_mock.go
# worker/uniter/util_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
5 participants