Merge pull request #8063 from patrickdillon/create-cmd-context

OCPBUGS-32091: Add Top-level Context for Create Commands

Author: openshift-merge-bot[bot]

cmd/openshift-install/agent.go

@@ -1,6 +1,8 @@
 package main
 
 import (
+	"context"
+
 	"github.com/spf13/cobra"
 
 	"github.com/openshift/installer/cmd/openshift-install/agent"
@@ -14,7 +16,7 @@ import (
 	"github.com/openshift/installer/pkg/asset/password"
 )
 
-func newAgentCmd() *cobra.Command {
+func newAgentCmd(ctx context.Context) *cobra.Command {
 	agentCmd := &cobra.Command{
 		Use:   "agent",
 		Short: "Commands for supporting cluster installation using agent installer",
@@ -23,7 +25,7 @@ func newAgentCmd() *cobra.Command {
 		},
 	}
 
-	agentCmd.AddCommand(newAgentCreateCmd())
+	agentCmd.AddCommand(newAgentCreateCmd(ctx))
 	agentCmd.AddCommand(agent.NewWaitForCmd())
 	agentCmd.AddCommand(newAgentGraphCmd())
 	return agentCmd
@@ -115,8 +117,7 @@ var (
 	agentTargets = []target{agentConfigTarget, agentManifestsTarget, agentImageTarget, agentPXEFilesTarget, agentConfigImageTarget, agentUnconfiguredIgnitionTarget}
 )
 
-func newAgentCreateCmd() *cobra.Command {
-
+func newAgentCreateCmd(ctx context.Context) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "create",
 		Short: "Commands for generating agent installation artifacts",
@@ -127,7 +128,7 @@ func newAgentCreateCmd() *cobra.Command {
 
 	for _, t := range agentTargets {
 		t.command.Args = cobra.ExactArgs(0)
-		t.command.Run = runTargetCmd(t.assets...)
+		t.command.Run = runTargetCmd(ctx, t.assets...)
 		cmd.AddCommand(t.command)
 	}
 

cmd/openshift-install/create.go

@@ -68,6 +68,7 @@ const (
 	exitCodeBootstrapFailed
 	exitCodeInstallFailed
 	exitCodeOperatorStabilityFailed
+	exitCodeInterrupt
 
 	// coStabilityThreshold is how long a cluster operator must have Progressing=False
 	// in order to be considered stable. Measured in seconds.
@@ -127,12 +128,10 @@ var (
 			Short: "Create an OpenShift cluster",
 			// FIXME: add longer descriptions for our commands with examples for better UX.
 			// Long:  "",
-			PostRun: func(_ *cobra.Command, _ []string) {
-				// Setup a context that is canceled when the user presses Ctrl+C,
-				// or SIGTERM and SIGINT are received, this allows for a clean shutdown.
-				ctx, cancel := context.WithCancel(context.TODO())
-				defer cancel()
-				logrus.RegisterExitHandler(cancel)
+			PostRun: func(cmd *cobra.Command, _ []string) {
+
+				// Get the context that was set in newCreateCmd.
+				ctx := cmd.Context()
 
 				exitCode, err := clusterCreatePostRun(ctx)
 				if err != nil {
@@ -167,7 +166,7 @@ func clusterCreatePostRun(ctx context.Context) (int, error) {
 	}
 
 	// Handle the case when the API server is not reachable.
-	if err := handleUnreachableAPIServer(config); err != nil {
+	if err := handleUnreachableAPIServer(ctx, config); err != nil {
 		logrus.Fatal(fmt.Errorf("unable to handle api server override: %w", err))
 	}
 
@@ -176,7 +175,7 @@ func clusterCreatePostRun(ctx context.Context) (int, error) {
 	//
 	timer.StartTimer("Bootstrap Complete")
 	if err := waitForBootstrapComplete(ctx, config); err != nil {
-		bundlePath, gatherErr := runGatherBootstrapCmd(command.RootOpts.Dir)
+		bundlePath, gatherErr := runGatherBootstrapCmd(ctx, command.RootOpts.Dir)
 		if gatherErr != nil {
 			logrus.Error("Attempted to gather debug logs after installation failure: ", gatherErr)
 		}
@@ -277,7 +276,7 @@ func newClientError(errorInfo error) *clusterCreateError {
 	}
 }
 
-func newCreateCmd() *cobra.Command {
+func newCreateCmd(ctx context.Context) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "create",
 		Short: "Create part of an OpenShift cluster",
@@ -288,22 +287,25 @@ func newCreateCmd() *cobra.Command {
 
 	for _, t := range targets {
 		t.command.Args = cobra.ExactArgs(0)
-		t.command.Run = runTargetCmd(t.assets...)
+		t.command.Run = runTargetCmd(ctx, t.assets...)
 		cmd.AddCommand(t.command)
 	}
 
 	return cmd
 }
 
-func runTargetCmd(targets ...asset.WritableAsset) func(cmd *cobra.Command, args []string) {
+func runTargetCmd(ctx context.Context, targets ...asset.WritableAsset) func(cmd *cobra.Command, args []string) {
 	runner := func(directory string) error {
 		fetcher := assetstore.NewAssetsFetcher(directory)
-		return fetcher.FetchAndPersist(targets)
+		return fetcher.FetchAndPersist(ctx, targets)
 	}
 
 	return func(cmd *cobra.Command, args []string) {
 		timer.StartTimer(timer.TotalTimeElapsed)
 
+		// Set the context to be used in the PostRun function.
+		cmd.SetContext(ctx)
+
 		cleanup := command.SetupFileHook(command.RootOpts.Dir)
 		defer cleanup()
 
@@ -855,15 +857,15 @@ func meetsStabilityThreshold(progressing *configv1.ClusterOperatorStatusConditio
 	return progressing.Status == configv1.ConditionFalse && time.Since(progressing.LastTransitionTime.Time).Seconds() > coStabilityThreshold
 }
 
-func handleUnreachableAPIServer(config *rest.Config) error {
+func handleUnreachableAPIServer(ctx context.Context, config *rest.Config) error {
 	assetStore, err := assetstore.NewStore(command.RootOpts.Dir)
 	if err != nil {
 		return fmt.Errorf("failed to create asset store: %w", err)
 	}
 
 	// Ensure that the install is expecting the user to provision their own DNS solution.
 	installConfig := &installconfig.InstallConfig{}
-	if err := assetStore.Fetch(installConfig); err != nil {
+	if err := assetStore.Fetch(ctx, installConfig); err != nil {
 		return fmt.Errorf("failed to fetch %s: %w", installConfig.Name(), err)
 	}
 	switch installConfig.Config.Platform.Name() { //nolint:gocritic
@@ -876,7 +878,7 @@ func handleUnreachableAPIServer(config *rest.Config) error {
 	}
 
 	lbConfig := &lbconfig.Config{}
-	if err := assetStore.Fetch(lbConfig); err != nil {
+	if err := assetStore.Fetch(ctx, lbConfig); err != nil {
 		return fmt.Errorf("failed to fetch %s: %w", lbConfig.Name(), err)
 	}
 

cmd/openshift-install/gather.go

@@ -34,7 +34,7 @@ import (
 	_ "github.com/openshift/installer/pkg/gather/gcp"
 )
 
-func newGatherCmd() *cobra.Command {
+func newGatherCmd(ctx context.Context) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "gather",
 		Short: "Gather debugging data for a given installation failure",
@@ -47,7 +47,7 @@ to debug the installation failures`,
 			return cmd.Help()
 		},
 	}
-	cmd.AddCommand(newGatherBootstrapCmd())
+	cmd.AddCommand(newGatherBootstrapCmd(ctx))
 	return cmd
 }
 
@@ -58,15 +58,15 @@ var gatherBootstrapOpts struct {
 	skipAnalysis bool
 }
 
-func newGatherBootstrapCmd() *cobra.Command {
+func newGatherBootstrapCmd(ctx context.Context) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "bootstrap",
 		Short: "Gather debugging data for a failing-to-bootstrap control plane",
 		Args:  cobra.ExactArgs(0),
 		Run: func(_ *cobra.Command, _ []string) {
 			cleanup := command.SetupFileHook(command.RootOpts.Dir)
 			defer cleanup()
-			bundlePath, err := runGatherBootstrapCmd(command.RootOpts.Dir)
+			bundlePath, err := runGatherBootstrapCmd(ctx, command.RootOpts.Dir)
 			if err != nil {
 				logrus.Fatal(err)
 			}
@@ -87,14 +87,14 @@ func newGatherBootstrapCmd() *cobra.Command {
 	return cmd
 }
 
-func runGatherBootstrapCmd(directory string) (string, error) {
+func runGatherBootstrapCmd(ctx context.Context, directory string) (string, error) {
 	assetStore, err := assetstore.NewStore(directory)
 	if err != nil {
 		return "", errors.Wrap(err, "failed to create asset store")
 	}
 	// add the default bootstrap key pair to the sshKeys list
 	bootstrapSSHKeyPair := &tls.BootstrapSSHKeyPair{}
-	if err := assetStore.Fetch(bootstrapSSHKeyPair); err != nil {
+	if err := assetStore.Fetch(ctx, bootstrapSSHKeyPair); err != nil {
 		return "", errors.Wrapf(err, "failed to fetch %s", bootstrapSSHKeyPair.Name())
 	}
 	tmpfile, err := os.CreateTemp("", "bootstrap-ssh")
@@ -118,7 +118,7 @@ func runGatherBootstrapCmd(directory string) (string, error) {
 
 	if ha.Bootstrap == "" && len(ha.Masters) == 0 {
 		config := &installconfig.InstallConfig{}
-		if err := assetStore.Fetch(config); err != nil {
+		if err := assetStore.Fetch(ctx, config); err != nil {
 			return "", errors.Wrapf(err, "failed to fetch %s", config.Name())
 		}
 

cmd/openshift-install/main.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"context"
 	"flag"
 	"io"
 	"os"
@@ -12,8 +13,10 @@ import (
 	terminal "golang.org/x/term"
 	"k8s.io/klog"
 	klogv2 "k8s.io/klog/v2"
+	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
 
 	"github.com/openshift/installer/cmd/openshift-install/command"
+	"github.com/openshift/installer/pkg/clusterapi"
 )
 
 func main() {
@@ -36,18 +39,22 @@ func main() {
 func installerMain() {
 	rootCmd := newRootCmd()
 
+	// Perform a graceful shutdown upon interrupt or at exit.
+	ctx := handleInterrupt(signals.SetupSignalHandler())
+	logrus.RegisterExitHandler(shutdown)
+
 	for _, subCmd := range []*cobra.Command{
-		newCreateCmd(),
+		newCreateCmd(ctx),
 		newDestroyCmd(),
 		newWaitForCmd(),
-		newGatherCmd(),
+		newGatherCmd(ctx),
 		newAnalyzeCmd(),
 		newVersionCmd(),
 		newGraphCmd(),
 		newCoreOSCmd(),
 		newCompletionCmd(),
 		newExplainCmd(),
-		newAgentCmd(),
+		newAgentCmd(ctx),
 	} {
 		rootCmd.AddCommand(subCmd)
 	}
@@ -96,3 +103,26 @@ func runRootCmd(cmd *cobra.Command, args []string) {
 		logrus.Fatal(errors.Wrap(err, "invalid log-level"))
 	}
 }
+
+// handleInterrupt executes a graceful shutdown then exits in
+// the case of a user interrupt. It returns a new context that
+// will be cancelled upon interrupt.
+func handleInterrupt(signalCtx context.Context) context.Context {
+	ctx, cancel := context.WithCancel(context.Background())
+
+	// If the context from the signal handler is done,
+	// an interrupt has been received, so shutdown & exit.
+	go func() {
+		<-signalCtx.Done()
+		logrus.Warn("Received interrupt signal")
+		shutdown()
+		cancel()
+		logrus.Exit(exitCodeInterrupt)
+	}()
+
+	return ctx
+}
+
+func shutdown() {
+	clusterapi.System().Teardown()
+}

pkg/asset/cluster/cluster.go

@@ -42,6 +42,7 @@ type Cluster struct {
 }
 
 var _ asset.WritableAsset = (*Cluster)(nil)
+var _ asset.Generator = (*Cluster)(nil)
 
 // Name returns the human-friendly name of the asset.
 func (c *Cluster) Name() string {
@@ -78,7 +79,7 @@ func (c *Cluster) Dependencies() []asset.Asset {
 }
 
 // Generate launches the cluster and generates the terraform state file on disk.
-func (c *Cluster) Generate(parents asset.Parents) (err error) {
+func (c *Cluster) GenerateWithContext(ctx context.Context, parents asset.Parents) (err error) {
 	if InstallDir == "" {
 		logrus.Fatalf("InstallDir has not been set for the %q asset", c.Name())
 	}
@@ -136,7 +137,7 @@ func (c *Cluster) Generate(parents asset.Parents) (err error) {
 	if err != nil {
 		return fmt.Errorf("error getting infrastructure provider: %w", err)
 	}
-	files, err := provider.Provision(InstallDir, parents)
+	files, err := provider.Provision(ctx, InstallDir, parents)
 	if files != nil {
 		c.FileList = append(c.FileList, files...) // append state files even in case of failure
 	}
@@ -165,3 +166,9 @@ func (c *Cluster) Load(f asset.FileFetcher) (found bool, err error) {
 
 	return false, nil
 }
+
+// Generate is implemented so the Cluster Asset maintains compatibility
+// with the Asset interface. It should never be called.
+func (c *Cluster) Generate(_ asset.Parents) (err error) {
+	panic("Cluster.Generate was called instead of Cluster.GenerateWithContext")
+}

pkg/asset/generator.go

@@ -0,0 +1,31 @@
+package asset
+
+import (
+	"context"
+)
+
+// Generator is used to generate assets.
+type Generator interface {
+	// Generate generates this asset given
+	// the states of its parent assets.
+	GenerateWithContext(context.Context, Parents) error
+}
+
+// generatorAdapter wraps an asset to provide the
+// Generate with context function.
+type generatorAdapter struct {
+	a Asset
+}
+
+// NewDefaultGenerator creates a new adapter to generate
+// an asset with a context.
+func NewDefaultGenerator(a Asset) Generator {
+	return &generatorAdapter{a: a}
+}
+
+// Generate calls Generate on an asset, dropping the context
+// to maintain compatibility with assets that do not implement
+// generate with context.
+func (a *generatorAdapter) GenerateWithContext(_ context.Context, p Parents) error {
+	return a.a.Generate(p)
+}

pkg/asset/store.go

@@ -1,11 +1,15 @@
 package asset
 
+import (
+	"context"
+)
+
 // Store is a store for the states of assets.
 type Store interface {
 	// Fetch retrieves the state of the given asset, generating it and its
 	// dependencies if necessary. When purging consumed assets, none of the
 	// assets in assetsToPreserve will be purged.
-	Fetch(assetToFetch Asset, assetsToPreserve ...WritableAsset) error
+	Fetch(ctx context.Context, assetToFetch Asset, assetsToPreserve ...WritableAsset) error
 
 	// Destroy removes the asset from all its internal state and also from
 	// disk if possible.

pkg/asset/store/assetcreate_test.go

@@ -1,6 +1,7 @@
 package store
 
 import (
+	"context"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -100,7 +101,7 @@ func TestCreatedAssetsAreNotDirty(t *testing.T) {
 			}
 
 			for _, a := range tc.targets {
-				if err := assetStore.Fetch(a, tc.targets...); err != nil {
+				if err := assetStore.Fetch(context.TODO(), a, tc.targets...); err != nil {
 					t.Fatalf("failed to fetch %q: %v", a.Name(), err)
 				}
 
@@ -125,7 +126,7 @@ func TestCreatedAssetsAreNotDirty(t *testing.T) {
 			for _, a := range tc.targets {
 				name := a.Name()
 				newAsset := reflect.New(reflect.TypeOf(a).Elem()).Interface().(asset.WritableAsset)
-				if err := newAssetStore.Fetch(newAsset, tc.targets...); err != nil {
+				if err := newAssetStore.Fetch(context.TODO(), newAsset, tc.targets...); err != nil {
 					t.Fatalf("failed to fetch %q in new store: %v", a.Name(), err)
 				}
 				assetState := newAssetStore.assets[reflect.TypeOf(a)]