Address review comments

Signed-off-by: Tom Pantelis <[email protected]>

cmd/subctl/deploybroker.go

@@ -43,20 +43,20 @@ var deployBroker = &cobra.Command{
 	Use:   "deploy-broker",
 	Short: "Deploys the broker",
 	Run: func(cmd *cobra.Command, args []string) {
+		status := cli.NewReporter()
+
 		config, err := restConfigProducer.ForCluster()
-		exit.OnError("Error creating REST config", err)
+		exit.OnError(status.Error(err, "Error creating REST config"))
 
 		clientProducer, err := client.NewProducerFromRestConfig(config)
-		exit.OnError("Error creating client producer", err)
+		exit.OnError(status.Error(err, "Error creating client producer"))
 
-		status := cli.NewReporter()
 		err = deploy.Broker(&deployflags, clientProducer, status)
-		if err == nil {
-			err = broker.WriteInfoToFile(config, deployflags.BrokerNamespace, ipsecSubmFile,
-				stringset.New(deployflags.BrokerSpec.Components...), deployflags.BrokerSpec.DefaultCustomDomains, status)
-		}
+		exit.OnError(err)
 
-		exit.OnError("", err)
+		err = broker.WriteInfoToFile(config, deployflags.BrokerNamespace, ipsecSubmFile,
+			stringset.New(deployflags.BrokerSpec.Components...), deployflags.BrokerSpec.DefaultCustomDomains, status)
+		exit.OnError(err)
 	},
 }
 

internal/cli/status.go

@@ -251,6 +251,9 @@ func (s *Status) Warning(message string, a ...interface{}) {
 
 func (s *Status) Error(err error, message string, args ...interface{}) error {
 	err = errors.Wrapf(err, message, args...)
+	if err == nil {
+		return nil
+	}
 
 	capitalizeFirst := func(str string) string {
 		for i, v := range str {

internal/exit/exit.go

@@ -24,23 +24,23 @@ import (
 	"github.com/submariner-io/submariner-operator/pkg/version"
 )
 
-// OnError will print your error nicely and exit in case of error.
-func OnError(message string, err error) {
+// OnError exits in case of error.
+func OnError(err error) {
 	if err != nil {
-		msg := ""
-		if message != "" {
-			msg = fmt.Sprintf("%s: %s", message, err)
-		}
-
-		WithErrorMsg(msg)
+		printVersion()
+		os.Exit(1)
 	}
 }
 
-// WithErrorMsg will print the message and quit the program with an error code.
-func WithErrorMsg(message string) {
+// WithMessage will print the message and quit the program with an error code.
+func WithMessage(message string) {
 	fmt.Fprintln(os.Stderr, message)
+	printVersion()
+	os.Exit(1)
+}
+
+func printVersion() {
 	fmt.Fprintln(os.Stderr, "")
 	version.PrintSubctlVersion(os.Stderr)
 	fmt.Fprintln(os.Stderr, "")
-	os.Exit(1)
 }

pkg/broker/file.go

@@ -43,8 +43,7 @@ func WriteInfoToFile(restConfig *rest.Config, brokerNamespace, ipsecFile string,
 
 	kubeClient, err := kubernetes.NewForConfig(restConfig)
 	if err != nil {
-		// TODO return reporter.Error(err, "error creating Kubernetes client")
-		return errors.Wrap(err, "error creating Kubernetes client")
+		return status.Error(err, "error creating Kubernetes client")
 	}
 
 	data, err := newDataFrom(kubeClient, brokerNamespace, ipsecFile)
@@ -57,8 +56,7 @@ func WriteInfoToFile(restConfig *rest.Config, brokerNamespace, ipsecFile string,
 
 	newFilename, err := backupIfExists(InfoFileName)
 	if err != nil {
-		// TODO return reporter.Error(err, "error backing up the broker file")
-		return errors.Wrap(err, "error backing up the broker file")
+		return status.Error(err, "error backing up the broker file")
 	}
 
 	if newFilename != "" {

pkg/deploy/broker.go

@@ -47,9 +47,6 @@ type BrokerOptions struct {
 
 var ValidComponents = []string{component.ServiceDiscovery, component.Connectivity}
 
-// Ignoring th cyclic complexity of Broker function because it is being refactored in
-// https://github.com/submariner-io/submariner-operator/pull/1717.
-//gocyclo:ignore
 func Broker(options *BrokerOptions, clientProducer client.Producer, status reporter.Interface) error {
 	componentSet := stringset.New(options.BrokerSpec.Components...)