Add a status reporter to be used in the subctl lib

Current cli.status implementation does not work well
with the library context of subctl. Hence a new reporter
is added that uses cli.status to report the status

A End() method is modified and old implementation is
renamed to EndWith().

Signed-Off-By: Janki Chhatbar <[email protected]>

.golangci.yml

@@ -19,6 +19,17 @@ linters-settings:
       - fieldalignment
   lll:
     line-length: 140
+  wrapcheck:
+    ignoreSigs:
+      - .Error(
+      - .Errorf(
+      - errors.New(
+      - errors.Unwrap(
+      - .Wrap(
+      - .Wrapf(
+      - .WithMessage(
+      - .WithMessagef(
+      - .WithStack(
   wsl:
     # Separating explicit var declarations by blank lines seems excessive.
     allow-cuddle-declarations: true

.licenserc.json

@@ -22,7 +22,9 @@
 	],
 	"ignore": [
 	    "pkg/client/clientset/",
-	    "pkg/internal/",
+	    "internal/cli",
+		"internal/env",
+		"internal/log",
 	    "vendor/"
 	]
     },

Makefile

@@ -55,7 +55,7 @@ override CLUSTERS_ARGS += $(CLUSTER_SETTINGS_FLAG)
 override DEPLOY_ARGS += $(CLUSTER_SETTINGS_FLAG)
 override E2E_ARGS += $(CLUSTER_SETTINGS_FLAG)
 export DEPLOY_ARGS
-override UNIT_TEST_ARGS += cmd pkg/internal
+override UNIT_TEST_ARGS += cmd internal/cli internal/env internal/log
 override VALIDATE_ARGS += --skip-dirs pkg/client
 
 # Process extra flags from the `using=a,b,c` optional flag

cmd/subctl/deploybroker.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 
 	"github.com/spf13/cobra"
+	"github.com/submariner-io/submariner-operator/internal/cli"
 	"github.com/submariner-io/submariner-operator/internal/constants"
 	"github.com/submariner-io/submariner-operator/internal/exit"
 	"github.com/submariner-io/submariner-operator/pkg/deploy"
@@ -38,8 +39,7 @@ var deployBroker = &cobra.Command{
 	Use:   "deploy-broker",
 	Short: "Deploys the broker",
 	Run: func(cmd *cobra.Command, args []string) {
-		fmt.Println("deployBroker called")
-		err := deploy.Broker(&deployflags, restConfigProducer)
+		err := deploy.Broker(&deployflags, restConfigProducer, cli.NewReporter())
 		exit.OnError("Error deploying Broker", err)
 	},
 }

cmd/subctl/root.go

@@ -1,16 +1,21 @@
 /*
 SPDX-License-Identifier: Apache-2.0
+
 Copyright Contributors to the Submariner project.
+
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
+
 package subctl
 
 import (

pkg/internal/cli/logger.go → internal/cli/logger.go

@@ -25,8 +25,8 @@ import (
 	"sync"
 	"sync/atomic"
 
-	"github.com/submariner-io/submariner-operator/pkg/internal/env"
-	"github.com/submariner-io/submariner-operator/pkg/internal/log"
+	"github.com/submariner-io/submariner-operator/internal/env"
+	"github.com/submariner-io/submariner-operator/internal/log"
 )
 
 // Logger is the kind cli's log.Logger implementation.

pkg/internal/cli/status.go → internal/cli/status.go

@@ -20,9 +20,12 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"unicode"
 
-	"github.com/submariner-io/submariner-operator/pkg/internal/env"
-	"github.com/submariner-io/submariner-operator/pkg/internal/log"
+	"github.com/pkg/errors"
+	"github.com/submariner-io/submariner-operator/internal/env"
+	"github.com/submariner-io/submariner-operator/internal/log"
+	"github.com/submariner-io/submariner-operator/pkg/reporter"
 )
 
 type Result int
@@ -86,11 +89,15 @@ func StatusForLogger(l log.Logger) *Status {
 	return s
 }
 
+func NewReporter() reporter.Interface {
+	return NewStatus()
+}
+
 // Start starts a new phase of the status, if attached to a terminal
 // there will be a loading spinner with this status.
-func (s *Status) Start(status string) {
-	s.End(Success)
-	s.status = status
+func (s *Status) Start(message string, args ...interface{}) {
+	s.End()
+	s.status = fmt.Sprintf(message, args...)
 
 	if s.spinner != nil {
 		s.spinner.SetSuffix(fmt.Sprintf(" %s ", s.status))
@@ -100,9 +107,9 @@ func (s *Status) Start(status string) {
 	}
 }
 
-// End completes the current status, ending any previous spinning and
+// EndWith completes the current status, ending any previous spinning and
 // marking the status as success or failure.
-func (s *Status) End(output Result) {
+func (s *Status) EndWith(output Result) {
 	if s.status == "" {
 		return
 	}
@@ -141,17 +148,17 @@ func (s *Status) End(output Result) {
 
 func (s *Status) EndWithFailure(message string, a ...interface{}) {
 	s.QueueFailureMessage(fmt.Sprintf(message, a...))
-	s.End(Failure)
+	s.EndWith(Failure)
 }
 
 func (s *Status) EndWithSuccess(message string, a ...interface{}) {
 	s.QueueSuccessMessage(fmt.Sprintf(message, a...))
-	s.End(Success)
+	s.EndWith(Success)
 }
 
 func (s *Status) EndWithWarning(message string, a ...interface{}) {
 	s.QueueWarningMessage(fmt.Sprintf(message, a...))
-	s.End(Warning)
+	s.EndWith(Warning)
 }
 
 // QueueSuccessMessage queues up a message, which will be displayed once
@@ -166,7 +173,7 @@ func (s *Status) QueueFailureMessage(message string) {
 	s.failureQueue = append(s.failureQueue, message)
 }
 
-// QueuewarningMessage queues up a message, which will be displayed once
+// QueueWarningMessage queues up a message, which will be displayed once
 // the status ends (using the warning format).
 func (s *Status) QueueWarningMessage(message string) {
 	s.warningQueue = append(s.warningQueue, message)
@@ -199,3 +206,50 @@ func CheckForError(err error) Result {
 
 	return Failure
 }
+
+// Failure queues up a message, which will be displayed once
+// the status ends (using the failure format).
+func (s *Status) Failure(message string, a ...interface{}) {
+	if message != "" {
+		s.failureQueue = append(s.failureQueue, fmt.Sprintf(message, a...))
+	}
+}
+
+// Success queues up a message, which will be displayed once
+// the status ends (using the warning format).
+func (s *Status) Success(message string, a ...interface{}) {
+	if message != "" {
+		s.successQueue = append(s.successQueue, fmt.Sprintf(message, a...))
+	}
+}
+
+// Warning queues up a message, which will be displayed once
+// the status ends (using the warning format).
+func (s *Status) Warning(message string, a ...interface{}) {
+	if message != "" {
+		s.warningQueue = append(s.warningQueue, fmt.Sprintf(message, a...))
+	}
+}
+
+func (s *Status) Error(err error, message string, args ...interface{}) error {
+	err = errors.Wrapf(err, message, args...)
+
+	capitalizeFirst := func(str string) string {
+		for i, v := range str {
+			return string(unicode.ToUpper(v)) + str[i+1:]
+		}
+
+		return ""
+	}
+
+	s.Failure(capitalizeFirst(err.Error()))
+	s.End()
+
+	return err
+}
+
+// End completes the current status, ending any previous spinning and
+// marking the status as success or failure.
+func (s *Status) End() {
+	s.EndWith(s.ResultFromMessages())
+}

pkg/internal/env/term_test.go → internal/env/term_test.go

@@ -21,7 +21,7 @@ import (
 	"io/ioutil"
 	"testing"
 
-	"github.com/submariner-io/submariner-operator/pkg/internal/env"
+	"github.com/submariner-io/submariner-operator/internal/env"
 )
 
 func TestIsTerminal(t *testing.T) {

internal/restconfig/restconfig.go

@@ -154,7 +154,7 @@ func ForBroker(submariner *v1alpha1.Submariner, serviceDisc *v1alpha1.ServiceDis
 	if submariner != nil {
 		// Try to authorize against the submariner Cluster resource as we know the CRD should exist and the credentials
 		// should allow read access.
-		restConfig, _, err := resource.GetAuthorizedRestConfig(submariner.Spec.BrokerK8sApiServer, submariner.Spec.BrokerK8sApiServerToken,
+		restConfig, _, err := resource.GetAuthorizedRestConfigFromData(submariner.Spec.BrokerK8sApiServer, submariner.Spec.BrokerK8sApiServerToken,
 			submariner.Spec.BrokerK8sCA, &rest.TLSClientConfig{}, schema.GroupVersionResource{
 				Group:    subv1.SchemeGroupVersion.Group,
 				Version:  subv1.SchemeGroupVersion.Version,
@@ -167,7 +167,7 @@ func ForBroker(submariner *v1alpha1.Submariner, serviceDisc *v1alpha1.ServiceDis
 	if serviceDisc != nil {
 		// Try to authorize against the ServiceImport resource as we know the CRD should exist and the credentials
 		// should allow read access.
-		restConfig, _, err := resource.GetAuthorizedRestConfig(serviceDisc.Spec.BrokerK8sApiServer, serviceDisc.Spec.BrokerK8sApiServerToken,
+		restConfig, _, err := resource.GetAuthorizedRestConfigFromData(serviceDisc.Spec.BrokerK8sApiServer, serviceDisc.Spec.BrokerK8sApiServerToken,
 			serviceDisc.Spec.BrokerK8sCA, &rest.TLSClientConfig{}, schema.GroupVersionResource{
 				Group:    "multicluster.x-k8s.io",
 				Version:  "v1alpha1",

pkg/deploy/broker.go

@@ -29,7 +29,7 @@ import (
 	"github.com/submariner-io/submariner-operator/internal/restconfig"
 	"github.com/submariner-io/submariner-operator/pkg/broker"
 	"github.com/submariner-io/submariner-operator/pkg/discovery/globalnet"
-	"github.com/submariner-io/submariner-operator/pkg/internal/cli"
+	"github.com/submariner-io/submariner-operator/pkg/reporter"
 	"github.com/submariner-io/submariner-operator/pkg/subctl/components"
 	"github.com/submariner-io/submariner-operator/pkg/subctl/datafile"
 	"github.com/submariner-io/submariner-operator/pkg/subctl/operator/brokercr"
@@ -52,8 +52,10 @@ var ValidComponents = []string{components.ServiceDiscovery, components.Connectiv
 
 const brokerDetailsFilename = "broker-info.subm"
 
-func Broker(options *BrokerOptions, restConfigProducer restconfig.Producer) error {
-	status := cli.NewStatus()
+// 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, restConfigProducer restconfig.Producer, status reporter.Interface) error {
 	componentSet := stringset.New(options.BrokerSpec.Components...)
 
 	if err := isValidComponents(componentSet); err != nil {
@@ -73,35 +75,34 @@ func Broker(options *BrokerOptions, restConfigProducer restconfig.Producer) erro
 		return errors.Wrap(err, "the provided kubeconfig is invalid")
 	}
 
-	err = deploy(options, status, config)
-	if err != nil {
+	if err := deploy(options, status, config); err != nil {
 		return err
 	}
 
-	status.Start(fmt.Sprintf("Creating %s file", brokerDetailsFilename))
+	status.Start("Creating %s file", brokerDetailsFilename)
 
 	// If deploy-broker is retried we will attempt to re-use the existing IPsec PSK secret
 	if options.IpsecSubmFile == "" {
 		if _, err := datafile.NewFromFile(brokerDetailsFilename); err == nil {
 			options.IpsecSubmFile = brokerDetailsFilename
-			status.QueueWarningMessage(fmt.Sprintf("Reusing IPsec PSK from existing %s", brokerDetailsFilename))
+			status.Warning("Reusing IPsec PSK from existing %s", brokerDetailsFilename)
 		} else {
-			status.QueueSuccessMessage(fmt.Sprintf("A new IPsec PSK will be generated for %s", brokerDetailsFilename))
+			status.Success("A new IPsec PSK will be generated for %s", brokerDetailsFilename)
 		}
 	}
 
 	subctlData, err := datafile.NewFromCluster(config, options.BrokerNamespace, options.IpsecSubmFile)
 	if err != nil {
-		return errors.Wrap(err, "error retrieving preparing the subm data file")
+		return status.Error(err, "error retrieving preparing the subm data file")
 	}
 
 	newFilename, err := datafile.BackupIfExists(brokerDetailsFilename)
 	if err != nil {
-		return errors.Wrap(err, "error backing up the brokerfile")
+		return status.Error(err, "error backing up the brokerfile")
 	}
 
 	if newFilename != "" {
-		status.QueueSuccessMessage(fmt.Sprintf("Backed up previous %s to %s", brokerDetailsFilename, newFilename))
+		status.Success("Backed up previous %s to %s", brokerDetailsFilename, newFilename)
 	}
 
 	subctlData.ServiceDiscovery = componentSet.Contains(components.ServiceDiscovery)
@@ -123,49 +124,50 @@ func Broker(options *BrokerOptions, restConfigProducer restconfig.Producer) erro
 	}
 
 	err = subctlData.WriteToFile(brokerDetailsFilename)
-	status.End(cli.CheckForError(err))
+	if err != nil {
+		return status.Error(err, "error writing the broker information")
+	}
+
+	status.End()
 
-	return errors.Wrap(err, "error writing the broker information")
+	return nil
 }
 
-func deploy(options *BrokerOptions, status *cli.Status, config *rest.Config) error {
+func deploy(options *BrokerOptions, status reporter.Interface, config *rest.Config) error {
 	status.Start("Setting up broker RBAC")
 
 	err := broker.Ensure(config, options.BrokerSpec.Components, false, options.BrokerNamespace)
-	status.End(cli.CheckForError(err))
-
 	if err != nil {
-		return errors.Wrap(err, "error setting up broker RBAC")
+		return status.Error(err, "error setting up broker RBAC")
 	}
 
+	status.End()
+
 	status.Start("Deploying the Submariner operator")
 
 	operatorImage, err := image.ForOperator(options.ImageVersion, options.Repository, nil)
 	if err != nil {
-		return errors.Wrap(err, "error getting Operator image")
+		return status.Error(err, "error getting Operator image")
 	}
 
 	err = submarinerop.Ensure(status, config, constants.OperatorNamespace, operatorImage, options.OperatorDebug)
-	status.End(cli.CheckForError(err))
-
 	if err != nil {
-		return errors.Wrap(err, "error deploying the operator")
+		return status.Error(err, "error deploying Submariner operator")
 	}
 
+	status.End()
+
 	status.Start("Deploying the broker")
 
 	err = brokercr.Ensure(config, options.BrokerNamespace, options.BrokerSpec)
-	if err == nil {
-		status.QueueSuccessMessage("The broker has been deployed")
-		status.End(cli.Success)
-
-		return nil
+	if err != nil {
+		return status.Error(err, "Broker deployment failed")
 	}
 
-	status.QueueFailureMessage("Broker deployment failed")
-	status.End(cli.Failure)
+	status.Success("The broker has been deployed")
+	status.End()
 
-	return errors.Wrap(err, "error deploying the broker")
+	return nil
 }
 
 func isValidComponents(componentSet stringset.Interface) error {