feat: ensure errors are logged using slog

This commit removes the usage of cobra.CheckError in favor of returning errors to the caller and log errors using slog. Signed-off-by: ludo <[email protected]>
controlplaneio · Mar 7, 2024 · ba97ef5 · ba97ef5
1 parent f779743
commit ba97ef5
Show file tree

Hide file tree

Showing 10 changed files with 128 additions and 59 deletions.
diff --git a/cmd/container/main.go b/cmd/container/main.go
@@ -5,11 +5,10 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/spf13/cobra"
-
 	"github.com/controlplaneio/simulator/v2/core/tools"
 	"github.com/controlplaneio/simulator/v2/internal/cli"
 	"github.com/controlplaneio/simulator/v2/internal/config"
+	"github.com/controlplaneio/simulator/v2/internal/logging"
 )
 
 func main() {
@@ -75,6 +74,7 @@ func main() {
 		),
 	)
 
-	err := simulator.Execute()
-	cobra.CheckErr(err)
+	if err := simulator.Execute(); err != nil {
+		logging.LogFatal("Simulator CLI returned an error", err)
+	}
 }
diff --git a/cmd/simulator/main.go b/cmd/simulator/main.go
@@ -6,13 +6,12 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/spf13/cobra"
-
 	"github.com/controlplaneio/simulator/v2/core/aws"
 	"github.com/controlplaneio/simulator/v2/core/tools"
 	"github.com/controlplaneio/simulator/v2/internal/cli"
 	"github.com/controlplaneio/simulator/v2/internal/config"
 	"github.com/controlplaneio/simulator/v2/internal/docker"
+	"github.com/controlplaneio/simulator/v2/internal/logging"
 )
 
 const (
@@ -167,8 +166,9 @@ func main() {
 		}),
 	)
 
-	err = simulator.Execute()
-	cobra.CheckErr(err)
+	if err := simulator.Execute(); err != nil {
+		logging.LogFatal("Simulator CLI returned an error", err)
+	}
 }
 
 func mkDirsIfNotExist(dirs ...string) {

diff --git a/internal/cli/ami.go b/internal/cli/ami.go
@@ -2,6 +2,7 @@ package cli
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"os/signal"
 	"syscall"
@@ -32,11 +33,13 @@ func WithAMIListCmd(manager aws.AMIManager) SimulatorCmdOptions {
 	amiListCmd := &cobra.Command{
 		Use:   "list",
 		Short: "List simulator AMIs",
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(_ *cobra.Command, _ []string) error {
 			ctx := context.Background()
 
 			amis, err := manager.List(ctx)
-			cobra.CheckErr(err)
+			if err != nil {
+				return fmt.Errorf("unable to list simulator AMIs: %w", err)
+			}
 
 			table := tablewriter.NewWriter(os.Stdout)
 
@@ -64,6 +67,7 @@ func WithAMIListCmd(manager aws.AMIManager) SimulatorCmdOptions {
 				table.SetRowLine(true)
 			}
 			table.Render()
+			return nil
 		},
 	}
 
@@ -77,12 +81,15 @@ func WithAMIDeleteCmd(manager aws.AMIManager) SimulatorCmdOptions {
 		Use:   "delete [ami id]",
 		Short: "Delete a simulator AMI",
 		Args:  cobra.MinimumNArgs(1),
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(_ *cobra.Command, args []string) error {
 			ctx := context.Background()
 			id := args[0]
 
 			err := manager.Delete(ctx, id)
-			cobra.CheckErr(err)
+			if err != nil {
+				return fmt.Errorf("unable to delete simulator AMI: %w", err)
+			}
+			return nil
 		},
 	}
 
@@ -98,20 +105,16 @@ func WithAmiBuildCmd(builder tools.AMIBuilder) SimulatorCmdOptions {
 		Use:   "build [name]",
 		Short: "Build the packer image",
 		Args:  cobra.MinimumNArgs(1),
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(_ *cobra.Command, args []string) error {
 			ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
+			defer stop()
 
-			var err error
 			id := args[0]
-
-			// go func() {
-			err = builder.Build(ctx, id)
-			// }()
-			//
-			// <-ctx.Done() //TODO: Check out quitting
-			stop()
-
-			cobra.CheckErr(err)
+			err := builder.Build(ctx, id)
+			if err != nil {
+				return fmt.Errorf("unable to build packer image: %w", err)
+			}
+			return nil
 		},
 	}
 

diff --git a/internal/cli/bucket.go b/internal/cli/bucket.go
@@ -2,6 +2,7 @@ package cli
 
 import (
 	"context"
+	"fmt"
 	"log/slog"
 	"os"
 
@@ -29,7 +30,7 @@ func WithBucketCmd(opts ...SimulatorCmdOptions) SimulatorCmdOptions {
 func WithCreateBucketCmd(config config.Config, manager aws.BucketManager) SimulatorCmdOptions {
 	bucketCreateCommand := &cobra.Command{
 		Use: "create",
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(_ *cobra.Command, _ []string) error {
 			ctx := context.Background()
 
 			if config.Bucket == "" {
@@ -38,7 +39,10 @@ func WithCreateBucketCmd(config config.Config, manager aws.BucketManager) Simula
 			}
 
 			err := manager.Create(ctx, config.Bucket)
-			cobra.CheckErr(err)
+			if err != nil {
+				return fmt.Errorf("unable to create bucket: %w", err)
+			}
+			return nil
 		},
 	}
 
@@ -50,11 +54,14 @@ func WithCreateBucketCmd(config config.Config, manager aws.BucketManager) Simula
 func WithDeleteBucketCmd(config config.Config, manager aws.BucketManager) SimulatorCmdOptions {
 	bucketCreateCommand := &cobra.Command{
 		Use: "delete",
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(_ *cobra.Command, _ []string) error {
 			ctx := context.Background()
 
 			err := manager.Delete(ctx, config.Bucket)
-			cobra.CheckErr(err)
+			if err != nil {
+				return fmt.Errorf("unable to delete bucket: %w", err)
+			}
+			return nil
 		},
 	}
 

diff --git a/internal/cli/config.go b/internal/cli/config.go
@@ -24,13 +24,16 @@ func WithConfigCmd(conf config.Config) SimulatorCmdOptions {
 	configCmd.PersistentFlags().BoolVar(&dev, "dev", false, "developer mode")
 	configCmd.PersistentFlags().BoolVar(&rootless, "rootless", false, "docker running in rootless mode")
 
-	configCmd.Run = func(_ *cobra.Command, _ []string) {
+	configCmd.RunE = func(_ *cobra.Command, _ []string) error {
 		if printDir {
 			dir, err := config.SimulatorDir()
-			cobra.CheckErr(err)
+			if err != nil {
+				return fmt.Errorf("unable to get simulator config directory: %w", err)
+			}
+
 			//nolint: forbidigo
 			fmt.Println(dir)
-			return
+			return nil
 		}
 
 		if name != "" {
@@ -46,7 +49,9 @@ func WithConfigCmd(conf config.Config) SimulatorCmdOptions {
 			conf.Container.Image = "controlplane/simulator:dev"
 
 			baseDir, err := os.Getwd()
-			cobra.CheckErr(err)
+			if err != nil {
+				return fmt.Errorf("unable to current working directory: %w", err)
+			}
 
 			conf.BaseDir = baseDir
 		} else {
@@ -58,7 +63,10 @@ func WithConfigCmd(conf config.Config) SimulatorCmdOptions {
 		conf.Container.Rootless = rootless
 
 		err := conf.Write()
-		cobra.CheckErr(err)
+		if err != nil {
+			return fmt.Errorf("unable to write simulator config to disk: %w", err)
+		}
+		return nil
 	}
 
 	return func(command *cobra.Command) {

diff --git a/internal/cli/container.go b/internal/cli/container.go
@@ -2,6 +2,7 @@ package cli
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/spf13/cobra"
 
@@ -28,10 +29,13 @@ func WithContainerPullCmd(config config.Config, client *docker.Client) Simulator
 	imagePullCmd := &cobra.Command{
 		Use:   "pull",
 		Short: "Pull the Simulator Container Image",
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(_ *cobra.Command, _ []string) error {
 			ctx := context.Background()
 			err := client.PullImage(ctx, config.Container.Image)
-			cobra.CheckErr(err)
+			if err != nil {
+				return fmt.Errorf("unable to pull simulator container image: %w", err)
+			}
+			return nil
 		},
 	}
 

diff --git a/internal/cli/infra.go b/internal/cli/infra.go
@@ -2,6 +2,7 @@ package cli
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"os/signal"
 	"syscall"
@@ -30,13 +31,20 @@ func WithInfraCreateCmd(manager tools.InfraManager, opts ...SimulatorCmdOptions)
 	infraCreateCmd := &cobra.Command{
 		Use:   "create",
 		Short: "Create simulator infrastructure",
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(cmd *cobra.Command, _ []string) error {
 			ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
 			defer stop()
 
-			stateBucket, stateKey, name := getTerraformFlags(cmd)
-			err := manager.Create(ctx, stateBucket, stateKey, name)
-			cobra.CheckErr(err)
+			stateBucket, stateKey, name, err := getTerraformFlags(cmd)
+			if err != nil {
+				return fmt.Errorf("unable to get terraform flags: %w", err)
+			}
+
+			err = manager.Create(ctx, stateBucket, stateKey, name)
+			if err != nil {
+				return fmt.Errorf("unable to create simulator infrastructure: %w", err)
+			}
+			return nil
 		},
 	}
 
@@ -53,13 +61,20 @@ func WithInfraDestroyCmd(manager tools.InfraManager, opts ...SimulatorCmdOptions
 	infraDestroyCmd := &cobra.Command{
 		Use:   "destroy",
 		Short: "Destroy simulator infrastructure",
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(cmd *cobra.Command, _ []string) error {
 			ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
 			defer stop()
 
-			stateBucket, stateKey, name := getTerraformFlags(cmd)
-			err := manager.Destroy(ctx, stateBucket, stateKey, name)
-			cobra.CheckErr(err)
+			stateBucket, stateKey, name, err := getTerraformFlags(cmd)
+			if err != nil {
+				return fmt.Errorf("unable to get terraform flags: %w", err)
+			}
+
+			err = manager.Destroy(ctx, stateBucket, stateKey, name)
+			if err != nil {
+				return fmt.Errorf("unable to destroy simulator infrastructure: %w", err)
+			}
+			return nil
 		},
 	}
 
@@ -72,16 +87,23 @@ func WithInfraDestroyCmd(manager tools.InfraManager, opts ...SimulatorCmdOptions
 	}
 }
 
-func getTerraformFlags(cmd *cobra.Command) (string, string, string) {
+func getTerraformFlags(cmd *cobra.Command) (string, string, string, error) {
 	stateBucket, err := cmd.Flags().GetString("stateBucket")
-	cobra.CheckErr(err)
+	if err != nil {
+		return "", "", "", fmt.Errorf("unable to get stateBucket flag: %w", err)
+	}
 
 	stateKey, err := cmd.Flags().GetString("stateKey")
-	cobra.CheckErr(err)
+	if err != nil {
+		return "", "", "", fmt.Errorf("unable to get stateKey flag: %w", err)
+	}
 
 	name, err := cmd.Flags().GetString("name")
-	cobra.CheckErr(err)
-	return stateBucket, stateKey, name
+	if err != nil {
+		return "", "", "", fmt.Errorf("unable to get name flag: %w", err)
+	}
+
+	return stateBucket, stateKey, name, nil
 }
 
 func WithFlag(name, value, usage string) SimulatorCmdOptions {