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

Merge 2.9 branch into develop #12481

Merged
merged 15 commits into from
Jan 5, 2021
Merged

Merge 2.9 branch into develop #12481

merged 15 commits into from
Jan 5, 2021

Conversation

benhoyt
Copy link
Contributor

@benhoyt benhoyt commented Jan 4, 2021

Includes the following PRs:

Conflicts:

  • environs/bootstrap/bootstrap_test.go

joedborg and others added 15 commits December 17, 2020 10:49
juju#12473

## Documentation changes

Simply removes the stop between microk8s and subcommand to reflect the MicroK8s CLI. Both will work on Linux (so internal Juju calls to MicroK8s don't need to change), but only the space method will work on MacOS and Windows wrappers.

## Notes

This was rebased off 2.8 as requested in juju#12467
This fixes bootstrap failing with the following output:

Contacting Juju controller at 10.20.46.16 to verify accessibility...
ERROR unable to contact api server after 1 attempts: unable to connect to API: dial tcp 10.20.46.16:17070: connect: connection refused
If they do press Ctrl-C a second time, it'll show the "cleaning up"
message. Example output (Ctrl-C pressed 3 times):

^C
Ctrl-C pressed, stopping bootstrap and cleaning up resources
ERROR contacting controller (cancelled): unable to connect to API: dial tcp 10.99.244.80:17070: i/o timeout
^C
Ctrl-C pressed, cleaning up failed bootstrap
^C
Ctrl-C pressed, cleaning up failed bootstrap
Also move ctx.Done() check in BootstrapInstance to immediately after
non-nil err from StartInstance (simpler: not dependent on zones).
juju#12475

*NOTE: this is a refactored version of juju#12462: this version adds a `Context()` method to the existing `environs.BootstrapContext` interface so we're only passing a single "context" around, and to reduce churn.*

Bootstrap is a bit of a pain to stop/cancel half way through, even if it's obviously failing it keeps retrying stuff for a long time (eg: waitForAgentInitialization would take 10 hours before it gave up). Need to handle Ctrl-C and terminate/cleanup.

This PR adds a `context.Context` to the existing `BootstrapContext` interface, and checks `ctx.Done()` in retry loops and at various points (except in WaitSSH, where we can actually select on `ctx.Done()` properly). We're doing it in this relatively hacky way for now to avoid wiring context up through *everything*, e.g., providers, the old AWS library, etc.

In any case, Ctrl-C is now handled in several parts of the bootstrap:

* BootstrapInstance
* WaitSSH
* waitForAgentInitialization
* K8s Deploy() step

I've tested both a working bootstrap and a `^C`-interrupted bootstrap on lxd, k8s (minikube), and AWS clouds.

Sample output of interrupting `waitForAgentInitialization`:

```
# set up minikube as per https://discourse.charmhub.io/t/using-juju-with-minikube/3975/3
$ juju bootstrap kubernetes k8s
Creating Juju controller "k8s" on kubernetes
WARNING bootstrapping to an unknown kubernetes cluster should be used with option --config controller-service-type. See juju help bootstrap
Fetching Juju Dashboard 0.3.0
Creating k8s resources for controller "controller-k8s"
Downloading images
Starting controller pod
Bootstrap agent now started
Contacting Juju controller at 10.97.223.174 to verify accessibility...
 # hangs due to load balancer error
^C
Ctrl-C pressed, stopping bootstrap and cleaning up resources
 # max 3 second delay here
ERROR contacting controller (cancelled): dial tcp 10.97.223.174:17070: i/o timeout
 # during cleanup, I pressed Ctrl-C a couple more times just for kicks
^C
Ctrl-C pressed, stopping bootstrap and cleaning up resources

Ctrl-C pressed, cleaning up failed bootstrap
^C
Ctrl-C pressed, cleaning up failed bootstrap
```

For reference, juju@c946eff is the commit where Ctrl-C "handling" was introduced (basically to ignore it).
Applications have been wildly used as a replacement for services since
1.x timeframe. The following displays a warning that services have been
deprecated and superseded by applications.

Version 3 of Juju will remove services completely allowing for a much
more cleaner code path for handling bundles.

As we don't actually store bundles within the database, we can do this
without a migration step.

Most of the bundle yamls within Juju codebase have now correctly been
converted to prevent the warning being emitted, paving the way for Juju
3.
…rning

juju#12478

Applications have been wildly used as a replacement for services since
1.x timeframe. The following displays a warning that services have been
deprecated and superseded by applications.

Version 3 of Juju will remove services completely allowing for a much
more cleaner code path for handling bundles.

As we don't actually store bundles within the database, we can do this
without a migration step.

Most of the bundle yaml within Juju codebase has now correctly been
converted to prevent the warning from being emitted, paving the way for
Juju 3.

## QA steps

Save the following bundle:

```yaml
services:
 ubuntu-lite:
 charm: cs:~jameinel/ubuntu-lite-7
 num_units: 1
```

## Bootstrap

```sh
$ juju bootstrap lxd test
```

### Local bundle.yaml

```sh
$ juju deploy ./bundle.yaml
WARNING "services" key found in bundle file is deprecated, superseded by "applications" key.
Located charm "ubuntu-lite" in charm-store, revision 7
Executing changes:
- upload charm ubuntu-lite from charm-store
- deploy application ubuntu-lite from charm-store
- add unit ubuntu-lite/0 to new machine 0
Deploy of bundle completed.
```

### Store bundle

```sh
juju deploy cs:~juju-qa/bundle/basic-0
Located bundle "basic" in charm-store, revision 0
WARNING "services" key found in bundle file is deprecated, superseded by "applications" key.
Located charm "ubuntu" in charm-store, revision 12
Executing changes:
- upload charm ubuntu from charm-store for series bionic
- deploy application ubuntu from charm-store on bionic
- set annotations for ubuntu
- set constraints for ubuntu-lite to ""
- set annotations for ubuntu-lite
- add unit ubuntu/0 to new machine 1
Deploy of bundle completed.
```
Conflicts:
cmd/juju/commands/bootstrap.go
provider/common/bootstrap_test.go
provider/rackspace/environ_test.go
juju#12480

Includes the following PRs:

* juju#12473 - Update MicroK8s hints to reflect current CLI
* juju#12475 - Handle Ctrl-C during bootstrap (refactored)

Conflicts:

* cmd/juju/commands/bootstrap.go
* provider/common/bootstrap_test.go
* provider/rackspace/environ_test.go
Conflicts:
environs/bootstrap/bootstrap_test.go
@benhoyt
Copy link
Contributor Author

benhoyt commented Jan 5, 2021

$$merge$$

@jujubot jujubot merged commit d4c3422 into juju:develop Jan 5, 2021
@benhoyt benhoyt deleted the merge-29-develop branch January 5, 2021 02:09
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.

5 participants