Skip to content

Commit

Permalink
Add a status reporter to be used in the subctl lib
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
Jaanki authored Dec 22, 2021
1 parent afdfa70 commit 4d3bcca
Show file tree
Hide file tree
Showing 37 changed files with 236 additions and 124 deletions.
11 changes: 11 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion .licenserc.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
],
"ignore": [
"pkg/client/clientset/",
"pkg/internal/",
"internal/cli",
"internal/env",
"internal/log",
"vendor/"
]
},
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions cmd/subctl/deploybroker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
},
}
Expand Down
5 changes: 5 additions & 0 deletions cmd/subctl/root.go
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down
4 changes: 2 additions & 2 deletions pkg/internal/cli/logger.go → internal/cli/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
File renamed without changes.
76 changes: 65 additions & 11 deletions pkg/internal/cli/status.go → internal/cli/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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())
}
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
File renamed without changes.
60 changes: 31 additions & 29 deletions pkg/deploy/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions pkg/discovery/globalnet/globalnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import (
"net"

"github.com/pkg/errors"
"github.com/submariner-io/submariner-operator/internal/cli"
"github.com/submariner-io/submariner-operator/pkg/broker"
"github.com/submariner-io/submariner-operator/pkg/internal/cli"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -322,7 +322,7 @@ func ValidateGlobalnetConfiguration(globalnetInfo *Info, netconfig Config) (stri
}
}

status.End(cli.Success)
status.EndWith(cli.Success)

return globalnetCIDR, nil
}
Expand Down Expand Up @@ -413,7 +413,7 @@ func AssignGlobalnetIPs(globalnetInfo *Info, netconfig Config) (string, error) {
}
}

status.End(cli.Success)
status.EndWith(cli.Success)

return globalnetCIDR, nil
}
Expand Down
Loading

0 comments on commit 4d3bcca

Please sign in to comment.