-
Notifications
You must be signed in to change notification settings - Fork 507
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
wallyworld
approved these changes
Jan 4, 2021
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Includes the following PRs:
Conflicts: