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

2.9 into develop #12403

Merged
merged 54 commits into from Dec 4, 2020
Merged

2.9 into develop #12403

merged 54 commits into from Dec 4, 2020

Conversation

SimonRichardson
Copy link
Member

@SimonRichardson SimonRichardson commented Dec 4, 2020

Merges 2.9 branch into develop.

Clean merge as well.

a034011 (upstream/2.9, origin/2.9, 2.9) Merge pull request #12395 from SimonRichardson/charmhub-charm-bundle
ec4d550 Merge pull request #12399 from ycliuhw/2.8-into-2.9
5d3f751 Merge pull request #12396 from SimonRichardson/charm-not-found-error
75ba13f Merge pull request #12394 from manadart/2.9-iaas-netget-efficiency
acd9c2f Merge pull request #12376 from SimonRichardson/charmhub-platform-origin
9a0e367 Merge pull request #12393 from hpidcock/timer-leak
e223eba Merge pull request #12383 from manadart/2.9-lxd-constraints
8c86b1c (move-charm-selector) Merge pull request #12391 from wallyworld/merge-2.8-20201202
1a258c3 Merge pull request #12384 from hmlanigan/resources-client

QA steps

Tests pass

stgraber and others added 30 commits November 30, 2020 13:56
This fixes the version parsing amongst other things which then fixes the
MiB handling for Juju contraints.

Closes https://bugs.launchpad.net/juju/+bug/1905827

Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
Includes the new client, response structures and ListResourceRevisons.
A new api endpoint for charmhub.
juju#12384

Includes the new client, response structures and ListResourceRevisons.
A new api endpoint for charmhub from: http://api.snapcraft.io/docs/charms.html#list_resource_revisions

There may be changes up coming to have the same property names, avoid hash-sha256 vs hash-sha256.

Will be used by `juju resources` in the future.

A few drive-by debug message fixes.

## QA steps

No change to current charmhub client behavior.
juju#12389

k8s clusters can have "insecure-tls-skip-verify" = true.
Add a new attribute to the Juju cloud entity - SkipTLSVerify.
It defaults to false (secure) and is only currently set when adding a k8s cloud.
Other provides may gain support for it in the future.

## QA steps

run juju add-k8s with a kubeconfig with nsecure-tls-skip-verify: true
juju bootstrap to the k8s cluster
juju show-cloud to see that skip-tls-verify: true

## Bug reference

https://bugs.launchpad.net/bugs/1905977
juju#12390

A preupgrade check was added which did not account for the fact that there could be a k8s model, not just an IAAS model.

## QA steps

bootstrap to k8s
juju upgrade-model

## Bug reference

https://bugs.launchpad.net/bugs/1906360
juju#12391

Merge 2.8 with these PRs:

juju#12389 Support k8s clusters with insecure-tls-skip-verify
juju#12390 When doing pre upgrade checks, allow for k8s models

```
# Conflicts:
# cmd/juju/cloud/show.go
# cmd/juju/cloud/show_test.go
```

## QA steps

See PRs
juju#12383

Supercedes juju#12375

The same commits are cherry-picked on to 2.9 instead of develop (3.0), so that we can release patch changes in the imminent 
release.

Addresses https://bugs.launchpad.net/juju/+bug/1905827

## QA steps

For the MB/MiB fix:
- Bootstrap LXD
- `juju add-machine --constraints "mem=4G cores=2"`
- Using the instance ID from juju status: `lxc config show <ID> | grep limits` should show:
```
 limits.cpu: "2"
 limits.memory: 4096MiB
```

Root disk:
- `juju add-machine --constraints "root-disk=4G root-disk-source=default"`
- `lxc config show <ID>` should include:
```
devices:
 root:
 path: /
 pool: default
 size: 4096MiB
 type: disk
```

Arch:
- Launch an arm64 instance on AWS with a keypair for SSH.
- Modify the associated security group to allow SSH from your machine and all TCP traffic to 8443.
- SSH to the instance:
 - `sudo snap refresh lxd --channel latest/stable`
 - `lxd init`. use sensible defaults, the public DNS name for the cluster address, and a trust password.
- Add a new LXD cloud/credentials for this new LXD cluster.
- `GOARCH=arm64 bootstrap ... --build-agent --bootstrap-constraints "arch=arm64"` will hang waiting for an address (can't access the internal bridge network), but via SSH `lxc list` will show that we successfully asked the server for an instance of a different arch to our client.
The following is the very start of funnelling through architecture. It
makes some big changes by forcing Origin to contain a platform. This
platform now ensures that we pass an architecture, os and series. As
this is just the beginning there are areas that are missing gaps:

 1. Bundle support (charms don't have constraints)
 2. Model migration

Once those two are solved (model migration should be easy enough) then
we can wrap this up.

One problem we have though is that charmhub just fails to download a
charm even if we're sending data for architecture and os and series.
The following links together the importing and exporting of charm origin
platform. Channel had been done, but interestingly the channel tests
weren't filled in. This was obviously my bad and I've installed some
tests for channel as well.
The issue here is that charmhub API wants architecture but don't. They
asked up to implement architecture and now they said they wanted not to.
SO we pass architecture everywhere and then at the last second we
hardcode it to "all".
The type allows us to reason about what the type is. We never store a
bundle type in state, so we never really need to export it or import it
either. That way we can polyfill the information in during an import
run.
The following ensures that we get the download information for a given
bundle. EXCEPT that bundles can't be installed via the refresh API so
more work is required to get that information from the info endpoint and
serve that to the user.
The following moves the selector into a common location so that we can
reuse it in the API server.

The code is really sound, except it doesn't handle revisions yes, that
can come later.
The refresh command was closing the API connection trying to get the
charm hub URL.

Additional tests also fixed.
The following finally removes and cleans up the series for the mocks.
juju#12393

time.After internals are not cleaned up until the timer fires.
In certain usage of time.After where long timeouts + fast firing channels can cause a large number of timers to be created.

## QA steps

Run unit tests.

## Documentation changes

N/A

## Bug reference

N/A
This avoids potential double calls to this method when network-get is
called in a relation context.
SimonRichardson and others added 24 commits December 3, 2020 11:05
The comment was rather tautological so clean it up to better explain
what the payload is for.
The following just ensures we use the default architecture instead of a
string.
Rebuild the schema to get the new GetDownloadInfos.
There is a lint issue with using unconvert to a string. It wasn't
required.
The following cleans up the grammatical errors of a series of error
messages.
If we use the isolation suite we can insure that we are hardened to
environment issues.
This needs an upgrade step, which will follow.
…origin

juju#12376

The following updates the deploy code to allow for platform to reside in
origin. With this change, it's now possible to correctly wire up the bundle
support and model migration changes. This helps with the wire up of the
architecture to platform so we can effectively run model migrations with
architecture support.

### Architecture

Architecture is wired through the deployment code by using the arch
constraint supplied and pushing it into the charm origin and pushing
that to the server.

There is a hack where the arch sent to the charmhub is hardcoded to
"all" because of a major issue in the API that is currently being resolved.

### Bundle support

Bundle support is wired up so that we call the FindDownloadURL on the
server. The bundle URL along with the origin is passed back so we can
extract it from the client. Originally I was going to create a generic repo
adapter, but it causes even more churn. Instead, I create one locally that
correctly loads the bundle and returns it to the deployer.

### Selector

The selector code that was crafted for downloads was repurposed for
finding the correct bundle URL. It was moved to `charmhub/selector` so
that client and server could use it.

There is a gap in the selector code that will be fixed in another PR that
will fix the issue around revisions and selecting the correct one based on
either the charm URL or the origin.

### Model migration

The changes here just exposed the platform to the model migration.

## QA steps

There are no bundles that have charmhub charms in them. This PR is only
to ensure we get the correct bundle.

```sh
$ export JUJU_DEV_FEATURE_FLAGS=charm-hub
$ juju bootstrap lxd test --config charm-hub-url="https://api.staging.snapcraft.io"
$ juju deploy wiki-simple
Located bundle "ch:wiki-simple-4"
Resolving charm via charmstore: cs:trusty/mysql-55
Resolving charm via charmstore: cs:trusty/mediawiki-5
Executing changes:
- upload charm cs:mysql-55 for series trusty
- deploy application mysql on trusty using cs:mysql-55
- set annotations for mysql
- upload charm cs:trusty/mediawiki-5 for series trusty
- deploy application wiki on trusty using cs:trusty/mediawiki-5
- set annotations for wiki
- add relation wiki:db - mysql:db
- add unit mysql/0 to new machine 0
- add unit wiki/0 to new machine 1
Deploy of bundle completed.

```

### Fallback architecture and series.

```sh
$ export JUJU_DEV_FEATURE_FLAGS=charm-hub
$ juju bootstrap lxd test
$ juju deploy ubuntu
```

### TBA

## Documentation changes

Architecture is now funneled through the cli and to the controller so we download
the correct charm binary blob.
When attempting to resolve an origin we should populate all the fields
of the origin so that we can correctly send them to and from the server.
These items where missed because of the changes to architecture, but
have now been correctly filled in.

Deploying a local charmhub charm is now possible thanks to this.
juju#12394

This change mirrors that made for CAAS units in juju#12351.

It mitigates inefficiencies for calls to `network-get` where interrogation of link-layer devices based on spaces can be conducted twice.

Instead, we pre-populate the info based on the unit applications space bindings, then look them up where we need them.

## QA steps

Change is mechanical in nature - unit tests are green.

## Documentation changes

None.

## Bug reference

N/A
The not found charm error message was terrible, it offered no support
for users with a cryptic error about it not conforming to a URL.

The following change fixes that by checking that the file doesn't exist
and if it doesn't warn them about that, rather than a malformed url that
nobody cares about.
This is one that should have been picked up before, but was missed
previously.
juju#12396

The not found charm error message was terrible, it offered no support
for users with a cryptic error about it not conforming to a URL.

The following change fixes that by checking that the file doesn't exist
and if it doesn't warn them about that, rather than a malformed url that
nobody cares about.

## QA steps

```sh
$ juju deploy ./bad.charm
ERROR no charm was found at "./bad.charm"
```
juju#12398

*Fix lint issue in test;*

## Checklist

 - [ ] ~Requires a [pylibjuju](https://github.com/juju/python-libjuju) change~
 - [ ] ~Added [integration tests](https://github.com/juju/juju/tree/develop/tests) for the PR~
 - [ ] ~Added or updated [doc.go](https://discourse.jujucharms.com/t/readme-in-packages/451) related to packages changed~
 - [x] Comments answer the question of why design decisions were made

## QA steps

No

## Documentation changes

No

## Bug reference

No
juju#12399

Merge 2.8 forward to 2.9 with the following patches:
- juju#12398 from ycliuhw:fix/lint-issue
juju#12395

When attempting to resolve an origin we should populate all the fields
of the origin so that we can correctly send them to and from the server.
These items were missed because of the changes to the architecture, but
have now been correctly filled in.

Deploying a local charmhub charm is now possible thanks to this.

## QA steps

```sh
$ export JUJU_DEV_FEATURE_FLAGS=charm-hub
$ juju bootstrap lxd test --config charm-hub-url="https://api.staging.snapcraft.io"
$ juju deploy ./testcharms/charm-hub/bundles/wordpress-simple/bundle.yaml
Resolving charm via charmhub: ch:mysql
Resolving charm via charmhub: ch:wordpress
Executing changes:
- upload charm ch:wordpress-9
- deploy application wordpress using ch:wordpress-9
ERROR cannot deploy bundle: cannot deploy application "wordpress": cannot add application "wordpress": series "kubernetes" in a non container model not valid
```

Ensure that mysql got installed correctly.

```sh
$ juju status
Model Controller Cloud/Region Version SLA Timestamp
default test lxd/default 2.9-rc3.1 unsupported 15:54:57Z

App Version Status Scale Charm Store Rev OS Message
mysql unknown 0 mysql charmhub 58 ubuntu
```
Copy link
Member

@manadart manadart left a comment

Choose a reason for hiding this comment

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

I like to list a patch summary in the description, generated with:

git log upstream/develop..upstream/2.9 --first-parent --oneline

@SimonRichardson
Copy link
Member Author

$$merge$$

@jujubot jujubot merged commit 3c8cf09 into juju:develop Dec 4, 2020
@SimonRichardson SimonRichardson deleted the 2.9-into-develop branch July 12, 2021 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants