cmd/openshift-install: RunE -> logrus.Fatal

Before this commit, we had a number of callers with the following
pattern:

  func doSomething(args) error {
    cleanup, err := setupFileHook(rootOpts.dir)
    if err != nil {
      return errors.Wrap(err, "failed to setup logging hook")
    }
    defer cleanup()

    err := someHelper()
    if err != nil {
      return err
    }

    return nil
  }

The problem with that approach is than any errors returned by
doSomething will not be logged to .openshift_install.log, and we
definitely want those errors logged to the file.  With this commit,
I've shuffled things around to ensure we call logrus.Fatal(err) before
the deferred cleanup() fires.

cmd/openshift-install/create.go

@@ -91,23 +91,31 @@ var (
 			Short: "Create an OpenShift cluster",
 			// FIXME: add longer descriptions for our commands with examples for better UX.
 			// Long:  "",
-			PostRunE: func(_ *cobra.Command, _ []string) error {
+			PostRun: func(_ *cobra.Command, _ []string) {
 				ctx := context.Background()
+
+				cleanup := setupFileHook(rootOpts.dir)
+				defer cleanup()
+
 				config, err := clientcmd.BuildConfigFromFlags("", filepath.Join(rootOpts.dir, "auth", "kubeconfig"))
 				if err != nil {
-					return errors.Wrap(err, "loading kubeconfig")
+					logrus.Fatal(errors.Wrap(err, "loading kubeconfig"))
 				}
 
 				err = destroyBootstrap(ctx, config, rootOpts.dir)
 				if err != nil {
-					return err
+					logrus.Fatal(err)
 				}
+
 				consoleURL, err := waitForConsole(ctx, config, rootOpts.dir)
 				if err != nil {
-					return err
+					logrus.Fatal(err)
 				}
 
-				return logComplete(rootOpts.dir, consoleURL)
+				err = logComplete(rootOpts.dir, consoleURL)
+				if err != nil {
+					logrus.Fatal(err)
+				}
 			},
 		},
 		assets: []asset.WritableAsset{&cluster.TerraformVariables{}, &kubeconfig.Admin{}, &cluster.Cluster{}},
@@ -126,22 +134,16 @@ func newCreateCmd() *cobra.Command {
 	}
 
 	for _, t := range targets {
-		t.command.RunE = runTargetCmd(t.assets...)
+		t.command.Run = runTargetCmd(t.assets...)
 		cmd.AddCommand(t.command)
 	}
 
 	return cmd
 }
 
-func runTargetCmd(targets ...asset.WritableAsset) func(cmd *cobra.Command, args []string) error {
-	return func(cmd *cobra.Command, args []string) error {
-		cleanup, err := setupFileHook(rootOpts.dir)
-		if err != nil {
-			return errors.Wrap(err, "failed to setup logging hook")
-		}
-		defer cleanup()
-
-		assetStore, err := asset.NewStore(rootOpts.dir)
+func runTargetCmd(targets ...asset.WritableAsset) func(cmd *cobra.Command, args []string) {
+	runner := func(directory string) error {
+		assetStore, err := asset.NewStore(directory)
 		if err != nil {
 			return errors.Wrapf(err, "failed to create asset store")
 		}
@@ -155,7 +157,7 @@ func runTargetCmd(targets ...asset.WritableAsset) func(cmd *cobra.Command, args
 				err = errors.Wrapf(err, "failed to fetch %s", a.Name())
 			}
 
-			if err2 := asset.PersistToFile(a, rootOpts.dir); err2 != nil {
+			if err2 := asset.PersistToFile(a, directory); err2 != nil {
 				err2 = errors.Wrapf(err2, "failed to write asset (%s) to disk", a.Name())
 				if err != nil {
 					logrus.Error(err2)
@@ -170,17 +172,21 @@ func runTargetCmd(targets ...asset.WritableAsset) func(cmd *cobra.Command, args
 		}
 		return nil
 	}
+
+	return func(cmd *cobra.Command, args []string) {
+		cleanup := setupFileHook(rootOpts.dir)
+		defer cleanup()
+
+		err := runner(rootOpts.dir)
+		if err != nil {
+			logrus.Fatal(err)
+		}
+	}
 }
 
 // FIXME: pulling the kubeconfig and metadata out of the root
 // directory is a bit cludgy when we already have them in memory.
 func destroyBootstrap(ctx context.Context, config *rest.Config, directory string) (err error) {
-	cleanup, err := setupFileHook(rootOpts.dir)
-	if err != nil {
-		return errors.Wrap(err, "failed to setup logging hook")
-	}
-	defer cleanup()
-
 	client, err := kubernetes.NewForConfig(config)
 	if err != nil {
 		return errors.Wrap(err, "creating a Kubernetes client")

cmd/openshift-install/destroy.go

@@ -30,26 +30,28 @@ func newDestroyClusterCmd() *cobra.Command {
 	return &cobra.Command{
 		Use:   "cluster",
 		Short: "Destroy an OpenShift cluster",
-		RunE:  runDestroyCmd,
-	}
-}
+		Run: func(_ *cobra.Command, _ []string) {
+			cleanup := setupFileHook(rootOpts.dir)
+			defer cleanup()
 
-func runDestroyCmd(cmd *cobra.Command, args []string) error {
-	cleanup, err := setupFileHook(rootOpts.dir)
-	if err != nil {
-		return errors.Wrap(err, "failed to setup logging hook")
+			err := runDestroyCmd(rootOpts.dir)
+			if err != nil {
+				logrus.Fatal(err)
+			}
+		},
 	}
-	defer cleanup()
+}
 
-	destroyer, err := destroy.New(logrus.StandardLogger(), rootOpts.dir)
+func runDestroyCmd(directory string) error {
+	destroyer, err := destroy.New(logrus.StandardLogger(), directory)
 	if err != nil {
 		return errors.Wrap(err, "Failed while preparing to destroy cluster")
 	}
 	if err := destroyer.Run(); err != nil {
 		return errors.Wrap(err, "Failed to destroy cluster")
 	}
 
-	store, err := asset.NewStore(rootOpts.dir)
+	store, err := asset.NewStore(directory)
 	if err != nil {
 		return errors.Wrapf(err, "failed to create asset store")
 	}
@@ -65,8 +67,14 @@ func newDestroyBootstrapCmd() *cobra.Command {
 	return &cobra.Command{
 		Use:   "bootstrap",
 		Short: "Destroy the bootstrap resources",
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return bootstrap.Destroy(rootOpts.dir)
+		Run: func(cmd *cobra.Command, args []string) {
+			cleanup := setupFileHook(rootOpts.dir)
+			defer cleanup()
+
+			err := bootstrap.Destroy(rootOpts.dir)
+			if err != nil {
+				logrus.Fatal(err)
+			}
 		},
 	}
 }

cmd/openshift-install/log.go

@@ -44,14 +44,14 @@ func (h *fileHook) Fire(entry *logrus.Entry) error {
 	return err
 }
 
-func setupFileHook(baseDir string) (func(), error) {
+func setupFileHook(baseDir string) func() {
 	if err := os.MkdirAll(baseDir, 0755); err != nil {
-		return nil, errors.Wrap(err, "failed to create base directory for logs")
+		logrus.Fatal(errors.Wrap(err, "failed to create base directory for logs"))
 	}
 
 	logfile, err := os.OpenFile(filepath.Join(baseDir, ".openshift_install.log"), os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0666)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to open log file")
+		logrus.Fatal(errors.Wrap(err, "failed to open log file"))
 	}
 
 	originalHooks := logrus.LevelHooks{}
@@ -68,5 +68,5 @@ func setupFileHook(baseDir string) (func(), error) {
 	return func() {
 		logfile.Close()
 		logrus.StandardLogger().ReplaceHooks(originalHooks)
-	}, nil
+	}
 }