feat: improve feedback to user (#63)

* feat: improve feedback to user

this pr adds more visible feedback to the users while the installation
is taking place. this introduces no functional change, we only added
some spinning while installing add-ons and improved the messages being
written to the screen.

* chore: fixed unit test

Author: ricardomaraschini

cmd/helmvm/bundle.go

@@ -53,7 +53,7 @@ var bundleCommand = &cli.Command{
 			return fmt.Errorf("unable to open bundle file: %w", err)
 		}
 		defer dst.Close()
-		loading := pb.Start(nil)
+		loading := pb.Start()
 		loading.Infof("Downloading base images bundle.")
 		src, err := goods.DownloadImagesBundle(defaults.K0sVersion)
 		if err != nil {
@@ -75,7 +75,7 @@ var bundleCommand = &cli.Command{
 		}
 		images = append(images, embed...)
 		for _, img := range images {
-			loading = pb.Start(nil)
+			loading = pb.Start()
 			loading.Infof(fmt.Sprintf("Pulling image %s", img))
 			if err := pullImage(c.Context, img); err != nil {
 				loading.Close()

cmd/helmvm/install.go

@@ -32,12 +32,15 @@ import (
 // runPostApply is meant to run things that can't be run automatically with
 // k0sctl. Iterates over all hosts and calls runPostApply on each.
 func runPostApply(ctx context.Context) error {
-	logrus.Infof("Running post-apply script on nodes")
-	loading := pb.Start(nil)
+	mask := func(raw string) string {
+		logrus.StandardLogger().Writer().Write([]byte(raw))
+		return fmt.Sprintf("Creating systemd unit for %s", defaults.BinaryName())
+	}
+	loading := pb.Start(pb.WithMask(mask))
 	orig := log.Log
 	rig.SetLogger(loading)
 	defer func() {
-		loading.Closef("Post apply process finished")
+		loading.Close()
 		log.Log = orig
 	}()
 	cfg, err := config.ReadConfigFile(defaults.PathToConfig("k0sctl.yaml"))
@@ -253,15 +256,29 @@ func ensureK0sctlConfig(c *cli.Context, nodes []infra.Node, useprompt bool) erro
 // runK0sctlApply runs `k0sctl apply` refering to the k0sctl.yaml file found on
 // the configuration directory. Returns when the command is finished.
 func runK0sctlApply(ctx context.Context) error {
+	message := "Applying cluster configuration"
+	mask := func(raw string) string {
+		logrus.StandardLogger().Writer().Write([]byte(raw))
+		if !strings.Contains(raw, "Running phase:") {
+			return message
+		}
+		slices := strings.SplitN(raw, ":", 2)
+		message = strings.ReplaceAll(slices[1], `"`, "")
+		message = strings.TrimSpace(message)
+		message = strings.ReplaceAll(message, "k0s", defaults.BinaryName())
+		message = strings.ReplaceAll(message, "Upload", "Copy")
+		message = fmt.Sprintf("Phase: %s", message)
+		return message
+	}
 	bin := defaults.PathToHelmVMBinary("k0sctl")
-	loading := pb.Start(nil)
+	loading := pb.Start(pb.WithMask(mask))
 	defer func() {
-		loading.Close()
+		loading.Closef("Finished applying cluster configuration")
 	}()
 	cfgpath := defaults.PathToConfig("k0sctl.yaml")
 	kctl := exec.Command(bin, "apply", "-c", cfgpath, "--disable-telemetry")
-	kctl.Stderr = logrus.StandardLogger().Writer()
-	kctl.Stdout = logrus.StandardLogger().Writer()
+	kctl.Stderr = loading
+	kctl.Stdout = loading
 	return kctl.Run()
 }
 

pkg/addons/adminconsole/adminconsole.go

@@ -15,6 +15,7 @@ import (
 	"helm.sh/helm/v3/pkg/release"
 
 	"github.com/replicatedhq/helmvm/pkg/addons/adminconsole/charts"
+	pb "github.com/replicatedhq/helmvm/pkg/progressbar"
 	"github.com/replicatedhq/helmvm/pkg/prompts"
 )
 
@@ -94,6 +95,9 @@ func (a *AdminConsole) Apply(ctx context.Context) error {
 		if err != nil {
 			return fmt.Errorf("unable to ask for password: %w", err)
 		}
+		loading := pb.Start()
+		loading.Infof("Applying AdminConsole addon")
+		defer loading.Close()
 		helmValues["password"] = pass
 		act := action.NewInstall(a.config)
 		act.Namespace = a.namespace
@@ -111,6 +115,9 @@ func (a *AdminConsole) Apply(ctx context.Context) error {
 		return fmt.Errorf("unable to downgrade from %s to %s", installedVersion, version)
 	}
 
+	loading := pb.Start()
+	loading.Infof("Applying AdminConsole addon")
+	defer loading.Close()
 	a.logger("Updating Admin Console from %s to %s", installedVersion, version)
 	act := action.NewUpgrade(a.config)
 	act.Namespace = a.namespace

pkg/addons/applier.go

@@ -56,12 +56,10 @@ func (a *Applier) Apply(ctx context.Context) error {
 	if err != nil {
 		return fmt.Errorf("unable to load addons: %w", err)
 	}
-	for name, addon := range addons {
-		logrus.Infof("Apply addon %s.", name)
+	for _, addon := range addons {
 		if err := addon.Apply(ctx); err != nil {
 			return err
 		}
-		logrus.Infof("Addon %s applied.", name)
 	}
 	return nil
 }
@@ -147,9 +145,9 @@ func (a *Applier) kubeClient() (client.Client, error) {
 // waitForKubernetes waits until we manage to make a successful connection to the
 // Kubernetes API server.
 func (a *Applier) waitForKubernetes(ctx context.Context) error {
-	loading := pb.Start(nil)
+	loading := pb.Start()
 	defer func() {
-		loading.Closef("Kubernetes API server is ready.")
+		loading.Closef("Kubernetes API server is ready")
 	}()
 	kcli, err := a.kubeClient()
 	if err != nil {
@@ -158,7 +156,7 @@ func (a *Applier) waitForKubernetes(ctx context.Context) error {
 	ticker := time.NewTicker(3 * time.Second)
 	defer ticker.Stop()
 	counter := 1
-	loading.Infof("1/n Waiting for Kubernetes API server to be ready.")
+	loading.Infof("1/n Waiting for Kubernetes API server to be ready")
 	for {
 		select {
 		case <-ticker.C:

pkg/addons/custom/custom.go

@@ -18,6 +18,7 @@ import (
 	"sigs.k8s.io/yaml"
 
 	"github.com/replicatedhq/helmvm/pkg/hembed"
+	pb "github.com/replicatedhq/helmvm/pkg/progressbar"
 )
 
 type Custom struct {
@@ -102,6 +103,9 @@ func (c *Custom) chartHasBeenDisabled(chart *chart.Chart) bool {
 }
 
 func (c *Custom) applyChart(ctx context.Context, chart *chart.Chart, values map[string]interface{}) error {
+	loading := pb.Start()
+	loading.Infof("Applying %s addon", chart.Name())
+	defer loading.Close()
 	installed, err := c.installedRelease(chart.Name())
 	if err != nil {
 		return fmt.Errorf("unable to check if release %s is installed: %w", chart.Name(), err)

pkg/addons/openebs/openebs.go

@@ -15,6 +15,7 @@ import (
 	"helm.sh/helm/v3/pkg/release"
 
 	"github.com/replicatedhq/helmvm/pkg/addons/openebs/charts"
+	pb "github.com/replicatedhq/helmvm/pkg/progressbar"
 )
 
 const (
@@ -56,6 +57,9 @@ func (o *OpenEBS) HostPreflights() (*v1beta2.HostPreflightSpec, error) {
 }
 
 func (o *OpenEBS) Apply(ctx context.Context) error {
+	loading := pb.Start()
+	loading.Infof("Applying OpenEBS addon")
+	defer loading.Close()
 	version, err := o.latest()
 	if err != nil {
 		return fmt.Errorf("unable to get latest version: %w", err)

pkg/config/host.go

@@ -1,9 +1,12 @@
 package config
 
 import (
+	"fmt"
+
 	"github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster"
 	"github.com/k0sproject/rig"
 	"github.com/k0sproject/rig/log"
+	"github.com/sirupsen/logrus"
 
 	"github.com/replicatedhq/helmvm/pkg/infra"
 	pb "github.com/replicatedhq/helmvm/pkg/progressbar"
@@ -42,7 +45,11 @@ func (h *hostcfg) render() *cluster.Host {
 
 // testConnection attempts to connect to the host via SSH.
 func (h *hostcfg) testConnection() error {
-	loading := pb.Start(nil)
+	mask := func(raw string) string {
+		logrus.StandardLogger().Writer().Write([]byte(raw))
+		return fmt.Sprintf("Validating host %s", h.Address)
+	}
+	loading := pb.Start(pb.WithMask(mask))
 	orig := log.Log
 	defer func() {
 		loading.Close()

pkg/infra/infra.go

@@ -28,7 +28,7 @@ type Node struct {
 
 // ApplyFn is the function that actually applies the infrastructure using terraform
 // library. This exists so we can make tests that don't actually run terraform.
-type ApplyFn func(context.Context, string, pb.MessageWriter) (map[string]tfexec.OutputMeta, error)
+type ApplyFn func(context.Context, string, *pb.MessageWriter) (map[string]tfexec.OutputMeta, error)
 
 // Infra is a struct that holds functions to apply and create infrastructure.
 type Infra struct {
@@ -44,7 +44,7 @@ func New() *Infra {
 // Apply uses "terraform apply" to apply the infrastructe defined in the
 // directory passed as argument.
 func (inf *Infra) Apply(ctx context.Context, dir string, useprompt bool) ([]Node, error) {
-	loading := pb.Start(nil)
+	loading := pb.Start()
 	loading.Infof("Applying infrastructure from %s", dir)
 	applyfn := inf.runApply
 	if inf.apply != nil {
@@ -56,7 +56,6 @@ func (inf *Infra) Apply(ctx context.Context, dir string, useprompt bool) ([]Node
 		return nil, fmt.Errorf("unable to apply infrastructure: %w", err)
 	}
 	loading.Close()
-	fmt.Println("Infrastructure applied successfully")
 	nodes, err := inf.ReadNodes(outputs)
 	if err != nil {
 		return nil, fmt.Errorf("unable to process terraform output: %w", err)
@@ -102,7 +101,7 @@ func (inf *Infra) ReadNodes(outputs map[string]tfexec.OutputMeta) ([]Node, error
 
 // runApply actually runs the terraform apply. This expects the terraform output to contain a
 // property 'instance_ips' which is a list of the created or updated node ip addresses.
-func (inf *Infra) runApply(ctx context.Context, dir string, log pb.MessageWriter) (map[string]tfexec.OutputMeta, error) {
+func (inf *Infra) runApply(ctx context.Context, dir string, log *pb.MessageWriter) (map[string]tfexec.OutputMeta, error) {
 	log.Infof("Reading terraform infrastructure from %s", dir)
 	exe := defaults.PathToHelmVMBinary("terraform")
 	tf, err := tfexec.NewTerraform(dir, exe)

pkg/infra/infra_test.go

@@ -47,31 +47,31 @@ var validTerraformOutput = []byte(`
 
 func TestApply(t *testing.T) {
 	infra := New()
-	infra.apply = func(context.Context, string, pb.MessageWriter) (map[string]tfexec.OutputMeta, error) {
+	infra.apply = func(context.Context, string, *pb.MessageWriter) (map[string]tfexec.OutputMeta, error) {
 		return nil, fmt.Errorf("test error")
 	}
 	_, err := infra.Apply(context.Background(), "", false)
 	assert.ErrorContains(t, err, "test error", "error should contain 'test error'")
 
 	buf := bytes.NewBuffer(nil)
 	infra.printf = writeTo(buf)
-	infra.apply = func(context.Context, string, pb.MessageWriter) (map[string]tfexec.OutputMeta, error) {
+	infra.apply = func(context.Context, string, *pb.MessageWriter) (map[string]tfexec.OutputMeta, error) {
 		return map[string]tfexec.OutputMeta{"nodes": {Value: []byte(`[]`)}}, nil
 	}
 	_, err = infra.Apply(context.Background(), "", false)
 	assert.ErrorContains(t, err, "no nodes found in terraform output")
 
 	buf = bytes.NewBuffer(nil)
 	infra.printf = writeTo(buf)
-	infra.apply = func(context.Context, string, pb.MessageWriter) (map[string]tfexec.OutputMeta, error) {
+	infra.apply = func(context.Context, string, *pb.MessageWriter) (map[string]tfexec.OutputMeta, error) {
 		return map[string]tfexec.OutputMeta{"nodes": {Value: []byte(`{this is not a valid/json{`)}}, nil
 	}
 	_, err = infra.Apply(context.Background(), "", false)
 	assert.ErrorContains(t, err, "unable to unmarshal terraform output")
 
 	buf = bytes.NewBuffer(nil)
 	infra.printf = writeTo(buf)
-	infra.apply = func(context.Context, string, pb.MessageWriter) (map[string]tfexec.OutputMeta, error) {
+	infra.apply = func(context.Context, string, *pb.MessageWriter) (map[string]tfexec.OutputMeta, error) {
 		return map[string]tfexec.OutputMeta{"nodes": {Value: validTerraformOutput}}, nil
 	}
 	nodes, err := infra.Apply(context.Background(), "", false)

pkg/preflights/preflights.go

@@ -115,7 +115,7 @@ func startProgressbar(addr string) (func(), error) {
 	logfile := logrus.New()
 	logfile.SetLevel(logrus.DebugLevel)
 	logfile.SetOutput(fp)
-	loading := pb.Start(nil)
+	loading := pb.Start()
 	loading.Infof("Running host preflight on %s", addr)
 	orig := log.Log
 	rig.SetLogger(logfile)