Merge pull request juju#12480 from benhoyt/merge-28-29

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
2733284198 · Jan 4, 2021 · 859e2b4 · 859e2b4
2 parents 983aff0 + 8d23db3
commit 859e2b4
Show file tree

Hide file tree

Showing 25 changed files with 396 additions and 213 deletions.
diff --git a/apiserver/common/modelwatcher_test.go b/apiserver/common/modelwatcher_test.go
@@ -4,6 +4,7 @@
 package common_test
 
 import (
+	"context"
 	"fmt"
 
 	"github.com/juju/cmd/cmdtesting"
@@ -103,7 +104,7 @@ func (*modelWatcherSuite) TestModelConfigFetchError(c *gc.C) {
 func testingEnvConfig(c *gc.C) *config.Config {
 	env, err := bootstrap.PrepareController(
 		false,
-		modelcmd.BootstrapContext(cmdtesting.Context(c)),
+		modelcmd.BootstrapContext(context.Background(), cmdtesting.Context(c)),
 		jujuclient.NewMemStore(),
 		bootstrap.PrepareParams{
 			ControllerConfig: testing.FakeControllerConfig(),

diff --git a/apiserver/facades/agent/fanconfigurer/fanconfigurer_test.go b/apiserver/facades/agent/fanconfigurer/fanconfigurer_test.go
@@ -4,6 +4,7 @@
 package fanconfigurer_test
 
 import (
+	"context"
 	"fmt"
 
 	"github.com/juju/cmd/cmdtesting"
@@ -131,7 +132,7 @@ func (s *fanconfigurerSuite) TestFanConfigFetchError(c *gc.C) {
 func testingEnvConfig(c *gc.C) *config.Config {
 	env, err := bootstrap.PrepareController(
 		false,
-		modelcmd.BootstrapContext(cmdtesting.Context(c)),
+		modelcmd.BootstrapContext(context.Background(), cmdtesting.Context(c)),
 		jujuclient.NewMemStore(),
 		bootstrap.PrepareParams{
 			ControllerConfig: testing.FakeControllerConfig(),

diff --git a/caas/kubernetes/provider/bootstrap.go b/caas/kubernetes/provider/bootstrap.go
@@ -36,6 +36,7 @@ import (
 	k8sannotations "github.com/juju/juju/core/annotations"
 	"github.com/juju/juju/core/watcher"
 	"github.com/juju/juju/environs"
+	"github.com/juju/juju/environs/bootstrap"
 	"github.com/juju/juju/mongo"
 )
 
@@ -315,7 +316,7 @@ func (c *controllerStack) doCleanUp() {
 	}
 }
 
-// Deploy creates all resources for controller stack.
+// Deploy creates all resources for the controller stack.
 func (c *controllerStack) Deploy() (err error) {
 	// creating namespace for controller stack, this namespace will be removed by broker.DestroyController if bootstrap failed.
 	nsName := c.broker.GetCurrentNamespace()
@@ -324,6 +325,20 @@ func (c *controllerStack) Deploy() (err error) {
 		return errors.Annotate(err, "creating namespace for controller stack")
 	}
 
+	// Check context manually for cancellation between each step (not ideal,
+	// but it avoids wiring context absolutely everywhere).
+	isDone := func() bool {
+		select {
+		case <-c.ctx.Context().Done():
+			return true
+		default:
+			return false
+		}
+	}
+	if isDone() {
+		return bootstrap.Cancelled()
+	}
+
 	defer func() {
 		if err != nil {
 			c.doCleanUp()
@@ -334,36 +349,58 @@ func (c *controllerStack) Deploy() (err error) {
 	if err = c.createControllerService(); err != nil {
 		return errors.Annotate(err, "creating service for controller")
 	}
+	if isDone() {
+		return bootstrap.Cancelled()
+	}
 
 	// create shared-secret secret for controller pod.
 	if err = c.createControllerSecretSharedSecret(); err != nil {
 		return errors.Annotate(err, "creating shared-secret secret for controller")
 	}
+	if isDone() {
+		return bootstrap.Cancelled()
+	}
 
 	// create server.pem secret for controller pod.
 	if err = c.createControllerSecretServerPem(); err != nil {
 		return errors.Annotate(err, "creating server.pem secret for controller")
 	}
+	if isDone() {
+		return bootstrap.Cancelled()
+	}
 
 	// create mongo admin account secret for controller pod.
 	if err = c.createControllerSecretMongoAdmin(); err != nil {
 		return errors.Annotate(err, "creating mongo admin account secret for controller")
 	}
+	if isDone() {
+		return bootstrap.Cancelled()
+	}
 
 	// create bootstrap-params configmap for controller pod.
 	if err = c.ensureControllerConfigmapBootstrapParams(); err != nil {
 		return errors.Annotate(err, "creating bootstrap-params configmap for controller")
 	}
+	if isDone() {
+		return bootstrap.Cancelled()
+	}
 
 	// Note: create agent config configmap for controller pod lastly because agentConfig has been updated in previous steps.
 	if err = c.ensureControllerConfigmapAgentConf(); err != nil {
 		return errors.Annotate(err, "creating agent config configmap for controller")
 	}
+	if isDone() {
+		return bootstrap.Cancelled()
+	}
 
 	// create statefulset to ensure controller stack.
 	if err = c.createControllerStatefulset(); err != nil {
 		return errors.Annotate(err, "creating statefulset for controller")
 	}
+	if isDone() {
+		return bootstrap.Cancelled()
+	}
+
 	return nil
 }
 

diff --git a/caas/kubernetes/provider/cloud.go b/caas/kubernetes/provider/cloud.go
@@ -365,7 +365,7 @@ Add yourself to that group before trying again:
 		}
 	}
 	if len(requiredAddons) > 0 {
-		return errors.Errorf("required addons not enabled for microk8s, run 'microk8s.enable %s'", strings.Join(requiredAddons, " "))
+		return errors.Errorf("required addons not enabled for microk8s, run 'microk8s enable %s'", strings.Join(requiredAddons, " "))
 	}
 	return nil
 }

diff --git a/caas/kubernetes/provider/cloud_test.go b/caas/kubernetes/provider/cloud_test.go
@@ -220,7 +220,7 @@ func (s *cloudSuite) TestEnsureMicroK8sSuitableStorageDisabled(c *gc.C) {
 		"RunCommands",
 		exec.RunParams{Commands: "microk8s.status --wait-ready --timeout 15 --yaml"}).Returns(
 		&exec.ExecResponse{Code: 0, Stdout: []byte(microk8sStatusStorageDisabled)}, nil)
-	c.Assert(provider.EnsureMicroK8sSuitable(s.runner), gc.ErrorMatches, `required addons not enabled for microk8s, run 'microk8s.enable storage'`)
+	c.Assert(provider.EnsureMicroK8sSuitable(s.runner), gc.ErrorMatches, `required addons not enabled for microk8s, run 'microk8s enable storage'`)
 }
 
 func (s *cloudSuite) TestEnsureMicroK8sSuitableDNSDisabled(c *gc.C) {
@@ -232,7 +232,7 @@ func (s *cloudSuite) TestEnsureMicroK8sSuitableDNSDisabled(c *gc.C) {
 		"RunCommands",
 		exec.RunParams{Commands: "microk8s.status --wait-ready --timeout 15 --yaml"}).Returns(
 		&exec.ExecResponse{Code: 0, Stdout: []byte(microk8sStatusDNSDisabled)}, nil)
-	c.Assert(provider.EnsureMicroK8sSuitable(s.runner), gc.ErrorMatches, `required addons not enabled for microk8s, run 'microk8s.enable dns'`)
+	c.Assert(provider.EnsureMicroK8sSuitable(s.runner), gc.ErrorMatches, `required addons not enabled for microk8s, run 'microk8s enable dns'`)
 }
 
 func (s *cloudSuite) TestEnsureMicroK8sSuitableNotInGroup(c *gc.C) {

diff --git a/caas/kubernetes/provider/k8s.go b/caas/kubernetes/provider/k8s.go
@@ -431,7 +431,7 @@ please choose a different hosted model name then try again.`, hostedModelName),
 		}
 		return errors.Annotate(
 			controllerStack.Deploy(),
-			"creating controller stack for controller",
+			"creating controller stack",
 		)
 	}
 

diff --git a/cmd/juju/commands/bootstrap.go b/cmd/juju/commands/bootstrap.go
@@ -5,6 +5,7 @@ package commands
 
 import (
 	"bufio"
+	"context"
 	"fmt"
 	"os"
 	"path"
@@ -43,7 +44,7 @@ import (
 	"github.com/juju/juju/environs/bootstrap"
 	environscloudspec "github.com/juju/juju/environs/cloudspec"
 	"github.com/juju/juju/environs/config"
-	"github.com/juju/juju/environs/context"
+	envcontext "github.com/juju/juju/environs/context"
 	"github.com/juju/juju/environs/sync"
 	"github.com/juju/juju/feature"
 	"github.com/juju/juju/juju"
@@ -427,7 +428,7 @@ func (c *bootstrapCommand) Init(args []string) (err error) {
 // BootstrapInterface provides bootstrap functionality that Run calls to support cleaner testing.
 type BootstrapInterface interface {
 	// Bootstrap bootstraps a controller.
-	Bootstrap(ctx environs.BootstrapContext, environ environs.BootstrapEnviron, callCtx context.ProviderCallContext, args bootstrap.BootstrapParams) error
+	Bootstrap(ctx environs.BootstrapContext, environ environs.BootstrapEnviron, callCtx envcontext.ProviderCallContext, args bootstrap.BootstrapParams) error
 
 	// CloudDetector returns a CloudDetector for the given provider,
 	// if the provider supports it.
@@ -444,7 +445,7 @@ type BootstrapInterface interface {
 
 type bootstrapFuncs struct{}
 
-func (b bootstrapFuncs) Bootstrap(ctx environs.BootstrapContext, env environs.BootstrapEnviron, callCtx context.ProviderCallContext, args bootstrap.BootstrapParams) error {
+func (b bootstrapFuncs) Bootstrap(ctx environs.BootstrapContext, env environs.BootstrapEnviron, callCtx envcontext.ProviderCallContext, args bootstrap.BootstrapParams) error {
 	return bootstrap.Bootstrap(ctx, env, callCtx, args)
 }
 
@@ -662,7 +663,7 @@ to create a new model to deploy %sworkloads.
 		return errors.Trace(err)
 	}
 
-	cloudCallCtx := context.NewCloudCallContext()
+	cloudCallCtx := envcontext.NewCloudCallContext()
 	// At this stage, the credential we intend to use is not yet stored
 	// server-side. So, if the credential is not accepted by the provider,
 	// we cannot mark it as invalid, just log it as an informative message.
@@ -747,7 +748,29 @@ to create a new model to deploy %sworkloads.
 
 	bootstrapCfg.controller[controller.ControllerName] = c.controllerName
 
-	bootstrapCtx := modelcmd.BootstrapContext(ctx)
+	// Handle Ctrl-C during bootstrap by asking the bootstrap process to stop
+	// early (and the above will then clean up resources).
+	interrupted := make(chan os.Signal, 1)
+	defer close(interrupted)
+	ctx.InterruptNotify(interrupted)
+	defer ctx.StopInterruptNotify(interrupted)
+	stdCtx, cancel := context.WithCancel(context.Background())
+	go func() {
+		for range interrupted {
+			select {
+			case <-stdCtx.Done():
+				// Ctrl-C already pressed
+				return
+			default:
+				// Newline prefix is intentional, so output appears as
+				// "^C\nCtrl-C pressed" instead of "^CCtrl-C pressed".
+				_, _ = fmt.Fprintln(ctx.GetStderr(), "\nCtrl-C pressed, stopping bootstrap and cleaning up resources")
+				cancel()
+			}
+		}
+	}()
+
+	bootstrapCtx := modelcmd.BootstrapContext(stdCtx, ctx)
 	bootstrapPrepareParams := bootstrap.PrepareParams{
 		ModelConfig:      bootstrapCfg.bootstrapModel,
 		ControllerConfig: bootstrapCfg.controller,
@@ -867,18 +890,6 @@ See `[1:] + "`juju kill-controller`" + `.`)
 		}
 	}()
 
-	// Block interruption during bootstrap. Providers may also
-	// register for interrupt notification so they can exit early.
-	interrupted := make(chan os.Signal, 1)
-	defer close(interrupted)
-	ctx.InterruptNotify(interrupted)
-	defer ctx.StopInterruptNotify(interrupted)
-	go func() {
-		for range interrupted {
-			ctx.Infof("Interrupt signalled: waiting for bootstrap to exit")
-		}
-	}()
-
 	// If --metadata-source is specified, override the default tools metadata source so
 	// SyncTools can use it, and also upload any image metadata.
 	if c.MetadataSource != "" {
@@ -956,7 +967,7 @@ See `[1:] + "`juju kill-controller`" + `.`)
 
 	bootstrapFuncs := getBootstrapFuncs()
 	if err = bootstrapFuncs.Bootstrap(
-		modelcmd.BootstrapContext(ctx),
+		bootstrapCtx,
 		environ,
 		cloudCallCtx,
 		bootstrapParams,
@@ -980,17 +991,16 @@ See `[1:] + "`juju kill-controller`" + `.`)
 	// for the controller's machine agent to be ready to accept commands
 	// before exiting this bootstrap command.
 	return waitForAgentInitialisation(
-		ctx,
+		bootstrapCtx,
 		&c.ModelCommandBase,
 		isCAASController,
 		c.controllerName,
-		c.hostedModelName,
 	)
 }
 
 func (c *bootstrapCommand) controllerDataRefresher(
 	environ environs.BootstrapEnviron,
-	cloudCallCtx *context.CloudCallContext,
+	cloudCallCtx *envcontext.CloudCallContext,
 	bootstrapCfg bootstrapConfigs,
 ) error {
 
@@ -1600,7 +1610,9 @@ func handleBootstrapError(ctx *cmd.Context, cleanup func() error) {
 	defer close(ch)
 	go func() {
 		for range ch {
-			_, _ = fmt.Fprintln(ctx.GetStderr(), "Cleaning up failed bootstrap")
+			// Newline prefix is intentional, so output appears as
+			// "^C\nCtrl-C pressed" instead of "^CCtrl-C pressed".
+			_, _ = fmt.Fprintln(ctx.GetStderr(), "\nCtrl-C pressed, cleaning up failed bootstrap")
 		}
 	}()
 	logger.Debugf("cleaning up after failed bootstrap")

diff --git a/cmd/juju/commands/bootstrap_test.go b/cmd/juju/commands/bootstrap_test.go
@@ -120,7 +120,7 @@ func (s *BootstrapSuite) SetUpTest(c *gc.C) {
 		panic("tests must call setupAutoUploadTest or otherwise patch envtools.BundleTools")
 	})
 
-	s.PatchValue(&waitForAgentInitialisation, func(*cmd.Context, *modelcmd.ModelCommandBase, bool, string, string) error {
+	s.PatchValue(&waitForAgentInitialisation, func(environs.BootstrapContext, *modelcmd.ModelCommandBase, bool, string) error {
 		return nil
 	})
 
@@ -2144,7 +2144,7 @@ func (s *BootstrapSuite) TestBootstrapSetsControllerOnBase(c *gc.C) {
 
 	// Record the controller name seen by ModelCommandBase at the end of bootstrap.
 	var seenControllerName string
-	s.PatchValue(&waitForAgentInitialisation, func(_ *cmd.Context, base *modelcmd.ModelCommandBase, _ bool, controllerName, _ string) error {
+	s.PatchValue(&waitForAgentInitialisation, func(_ environs.BootstrapContext, base *modelcmd.ModelCommandBase, _ bool, controllerName string) error {
 		seenControllerName = controllerName
 		return nil
 	})