new(cmd,pkg/driver): properly use a spinner for long operations.

Signed-off-by: Federico Di Pierro <[email protected]>

cmd/artifact/install/install.go

@@ -22,7 +22,6 @@ import (
 	"path/filepath"
 	"runtime"
 
-	"github.com/pterm/pterm"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 
@@ -182,8 +181,6 @@ Examples:
 
 // RunArtifactInstall executes the business logic for the artifact install command.
 func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []string) error {
-	var sp *pterm.SpinnerPrinter
-
 	logger := o.Printer.Logger
 	// Retrieve configuration for installer
 	configuredInstaller, err := config.Installer()
@@ -317,7 +314,7 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
 		logger.Info("Extracting and installing artifact", logger.Args("type", result.Type, "file", result.Filename))
 
 		if !o.Printer.DisableStyling {
-			sp, _ = o.Printer.Spinner.Start("Extracting and installing")
+			o.Printer.Spinner, _ = o.Printer.Spinner.Start("Extracting and installing")
 		}
 
 		result.Filename = filepath.Join(tmpDir, result.Filename)
@@ -337,8 +334,8 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
 			return err
 		}
 
-		if sp != nil {
-			_ = sp.Stop()
+		if o.Printer.Spinner != nil {
+			_ = o.Printer.Spinner.Stop()
 		}
 		logger.Info("Artifact successfully installed", logger.Args("name", ref, "type", result.Type, "digest", result.Digest, "directory", destDir))
 	}

cmd/driver/cleanup/cleanup.go

@@ -16,6 +16,10 @@
 package drivercleanup
 
 import (
+	"bytes"
+	"strings"
+
+	"github.com/pterm/pterm"
 	"github.com/spf13/cobra"
 	"golang.org/x/net/context"
 
@@ -51,5 +55,21 @@ func (o *driverCleanupOptions) RunDriverCleanup(_ context.Context) error {
 	o.Printer.Logger.Info("Running falcoctl driver cleanup", o.Printer.Logger.Args(
 		"driver type", o.Driver.Type,
 		"driver name", o.Driver.Name))
-	return o.Driver.Type.Cleanup(o.Printer, o.Driver.Name)
+	var buf bytes.Buffer
+	if !o.Printer.DisableStyling {
+		o.Printer.Spinner, _ = o.Printer.Spinner.Start("Cleaning up existing drivers")
+	}
+	err := o.Driver.Type.Cleanup(o.Printer.WithWriter(&buf), o.Driver.Name)
+	if o.Printer.Spinner != nil {
+		_ = o.Printer.Spinner.Stop()
+	}
+	if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
+		// Only print formatted text if we are formatting to json
+		out := strings.ReplaceAll(buf.String(), "\n", ";")
+		o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
+	} else {
+		// Print much more readable output as-is
+		o.Printer.DefaultText.Print(buf.String())
+	}
+	return err
 }

cmd/driver/install/install.go

@@ -16,12 +16,15 @@
 package driverinstall
 
 import (
+	"bytes"
 	"crypto/tls"
 	"errors"
 	"fmt"
 	"net/http"
+	"strings"
 	"time"
 
+	"github.com/pterm/pterm"
 	"github.com/spf13/cobra"
 	"golang.org/x/net/context"
 
@@ -140,30 +143,88 @@ func (o *driverInstallOptions) RunDriverInstall(ctx context.Context) (string, er
 	}
 	o.Printer.Logger.Info("found distro", o.Printer.Logger.Args("target", d))
 
-	err = o.Driver.Type.Cleanup(o.Printer, o.Driver.Name)
+	var (
+		dest string
+		buf  bytes.Buffer
+	)
+
+	if !o.Printer.DisableStyling {
+		o.Printer.Spinner, _ = o.Printer.Spinner.Start("Cleaning up existing drivers")
+	}
+	err = o.Driver.Type.Cleanup(o.Printer.WithWriter(&buf), o.Driver.Name)
+	if o.Printer.Spinner != nil {
+		_ = o.Printer.Spinner.Stop()
+	}
+	if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
+		// Only print formatted text if we are formatting to json
+		out := strings.ReplaceAll(buf.String(), "\n", ";")
+		o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
+	} else {
+		// Print much more readable output as-is
+		o.Printer.DefaultText.Print(buf.String())
+	}
 	if err != nil {
 		return "", err
 	}
 
-	setDefaultHTTPClientOpts(o.driverDownloadOptions)
-
-	var dest string
 	if o.Download {
-		dest, err = driverdistro.Download(ctx, d, o.Printer, kr, o.Driver.Name, o.Driver.Type, o.Driver.Version, o.Driver.Repos)
+		setDefaultHTTPClientOpts(o.driverDownloadOptions)
+		if !o.Printer.DisableStyling {
+			o.Printer.Spinner, _ = o.Printer.Spinner.Start("Trying to download the driver")
+		}
+		dest, err = driverdistro.Download(ctx, d, o.Printer.WithWriter(&buf), kr, o.Driver.Name, o.Driver.Type, o.Driver.Version, o.Driver.Repos)
+		if o.Printer.Spinner != nil {
+			_ = o.Printer.Spinner.Stop()
+		}
+		if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
+			// Only print formatted text if we are formatting to json
+			out := strings.ReplaceAll(buf.String(), "\n", ";")
+			o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
+		} else {
+			// Print much more readable output as-is
+			o.Printer.DefaultText.Print(buf.String())
+		}
+		buf.Reset()
 		if err == nil {
+			o.Printer.Logger.Info("Driver downloaded.", o.Printer.Logger.Args("path", dest))
+			return dest, nil
+		}
+		if errors.Is(err, driverdistro.ErrAlreadyPresent) {
+			o.Printer.Logger.Info("Skipping download, driver already present.", o.Printer.Logger.Args("path", dest))
 			return dest, nil
 		}
 		// Print the error but go on
 		// attempting a build if requested
-		o.Printer.Logger.Warn(err.Error())
+		if o.Compile {
+			o.Printer.Logger.Warn(err.Error())
+		}
 	}
 
 	if o.Compile {
-		dest, err = driverdistro.Build(ctx, d, o.Printer, kr, o.Driver.Name, o.Driver.Type, o.Driver.Version)
+		if !o.Printer.DisableStyling {
+			o.Printer.Spinner, _ = o.Printer.Spinner.Start("Trying to build the driver")
+		}
+		dest, err = driverdistro.Build(ctx, d, o.Printer.WithWriter(&buf), kr, o.Driver.Name, o.Driver.Type, o.Driver.Version)
+		if o.Printer.Spinner != nil {
+			_ = o.Printer.Spinner.Stop()
+		}
+		if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
+			// Only print formatted text if we are formatting to json
+			out := strings.ReplaceAll(buf.String(), "\n", ";")
+			o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
+		} else {
+			// Print much more readable output as-is
+			o.Printer.DefaultText.Print(buf.String())
+		}
+		buf.Reset()
 		if err == nil {
+			o.Printer.Logger.Info("Driver built.", o.Printer.Logger.Args("path", dest))
+			return dest, nil
+		}
+		if errors.Is(err, driverdistro.ErrAlreadyPresent) {
+			o.Printer.Logger.Info("Skipping build, driver already present.", o.Printer.Logger.Args("path", dest))
 			return dest, nil
 		}
-		o.Printer.Logger.Warn(err.Error())
 	}
 
 	return o.Driver.Name, fmt.Errorf("failed: %w", err)

main.go

@@ -40,6 +40,9 @@ func main() {
 		if opt.Printer != nil && opt.Printer.ProgressBar != nil && opt.Printer.ProgressBar.IsActive {
 			_, _ = opt.Printer.ProgressBar.Stop()
 		}
+		if opt.Printer != nil && opt.Printer.Spinner != nil && opt.Printer.Spinner.IsActive {
+			_ = opt.Printer.Spinner.Stop()
+		}
 		opt.Printer.Logger.Info("Received signal, terminating...")
 		stop()
 	}()

pkg/driver/distro/distro.go

@@ -49,6 +49,8 @@ var (
 	hostRoot = string(os.PathSeparator)
 	// ErrUnsupported is the error returned when the target distro is not supported.
 	ErrUnsupported = errors.New("failed to determine distro")
+	// ErrAlreadyPresent is the error returned when a driver is already present on filesystem.
+	ErrAlreadyPresent = errors.New("driver already present")
 )
 
 // Distro is the common interface used by distro-specific implementations.
@@ -169,8 +171,7 @@ func Build(ctx context.Context,
 	driverFileName := toFilename(d, &kr, driverName, driverType)
 	destination := toLocalPath(driverVer, driverFileName, kr.Architecture.ToNonDeb())
 	if exist, _ := utils.FileExists(destination); exist {
-		printer.Logger.Info("Skipping build, driver already present.", printer.Logger.Args("path", destination))
-		return destination, nil
+		return destination, ErrAlreadyPresent
 	}
 
 	env, err := d.customizeBuild(ctx, printer, driverType, kr)
@@ -189,11 +190,7 @@ func Build(ctx context.Context,
 	if err != nil {
 		return "", err
 	}
-	err = copyDataToLocalPath(destination, f)
-	if err == nil {
-		printer.Logger.Info("Driver built.", printer.Logger.Args("path", destination))
-	}
-	return destination, err
+	return destination, copyDataToLocalPath(destination, f)
 }
 
 // Download will try to download drivers for a distro trying specified repos.
@@ -211,8 +208,7 @@ func Download(ctx context.Context,
 	// Skip if existent
 	destination := toLocalPath(driverVer, driverFileName, kr.Architecture.ToNonDeb())
 	if exist, _ := utils.FileExists(destination); exist {
-		printer.Logger.Info("Skipping download, driver already present.", printer.Logger.Args("path", destination))
-		return destination, nil
+		return destination, ErrAlreadyPresent
 	}
 
 	// Try to download from any specified repository,
@@ -236,11 +232,7 @@ func Download(ctx context.Context,
 			}
 			continue
 		}
-		err = copyDataToLocalPath(destination, resp.Body)
-		if err == nil {
-			printer.Logger.Info("Driver downloaded.", printer.Logger.Args("path", destination))
-		}
-		return destination, err
+		return destination, copyDataToLocalPath(destination, resp.Body)
 	}
 	return destination, fmt.Errorf("unable to find a prebuilt driver")
 }

pkg/driver/type/bpf.go

@@ -78,8 +78,6 @@ func (b *bpf) Build(ctx context.Context,
 	driverName, driverVersion string,
 	env map[string]string,
 ) (string, error) {
-	printer.Logger.Info("Trying to compile the eBPF probe")
-
 	srcPath := fmt.Sprintf("/usr/src/%s-%s/bpf", driverName, driverVersion)
 
 	makeCmdArgs := fmt.Sprintf(`make -C %q`, filepath.Clean(srcPath))
@@ -88,12 +86,10 @@ func (b *bpf) Build(ctx context.Context,
 	for key, val := range env {
 		makeCmd.Env = append(makeCmd.Env, fmt.Sprintf("%s=%s", key, val))
 	}
-	err := runCmdPipingStdout(printer, makeCmd)
-	outProbe := fmt.Sprintf("%s/probe.o", srcPath)
-	if err == nil {
-		printer.Logger.Info("Probe successfully built.", printer.Logger.Args("file", outProbe))
-	} else {
-		printer.Logger.Info("Failed to build probe.", printer.Logger.Args("err", err))
+	out, err := makeCmd.CombinedOutput()
+	if err != nil {
+		printer.DefaultText.Print(string(out))
 	}
+	outProbe := fmt.Sprintf("%s/probe.o", srcPath)
 	return outProbe, err
 }

pkg/driver/type/kmod.go

@@ -192,8 +192,6 @@ func (k *kmod) Build(ctx context.Context,
 		return "", fmt.Errorf("unsupported on uek hosts")
 	}
 
-	printer.Logger.Info("Trying to compile the kernel module")
-
 	out, err := exec.Command("which", "gcc").Output()
 	if err != nil {
 		return "", err
@@ -224,8 +222,7 @@ func (k *kmod) Build(ctx context.Context,
 			driverName, driverVersion, kr.String())
 
 		// Try the build through dkms
-		dkmsCmd := exec.CommandContext(ctx, "bash", "-c", dkmsCmdArgs) //nolint:gosec // false positive
-		err = runCmdPipingStdout(printer, dkmsCmd)
+		out, err = exec.CommandContext(ctx, "bash", "-c", dkmsCmdArgs).CombinedOutput() //nolint:gosec // false positive
 		if err == nil {
 			koGlob := fmt.Sprintf("/var/lib/dkms/%s/%s/%s/%s/module/%s", driverName, driverVersion, kr.String(), kr.Architecture.ToNonDeb(), driverName)
 			var koFiles []string
@@ -238,6 +235,7 @@ func (k *kmod) Build(ctx context.Context,
 			printer.Logger.Info("Module installed in dkms.", printer.Logger.Args("file", koFile))
 			return koFile, nil
 		}
+		printer.DefaultText.Print(string(out))
 		dkmsLogFile := fmt.Sprintf("/var/lib/dkms/$%s/%s/build/make.log", driverName, driverVersion)
 		logs, err := os.ReadFile(filepath.Clean(dkmsLogFile))
 		if err != nil {

pkg/driver/type/type.go

@@ -17,9 +17,7 @@
 package drivertype
 
 import (
-	"bufio"
 	"fmt"
-	"os/exec"
 
 	"github.com/falcosecurity/driverkit/pkg/kernelrelease"
 	"golang.org/x/net/context"
@@ -59,26 +57,3 @@ func Parse(driverType string) (DriverType, error) {
 	}
 	return nil, fmt.Errorf("wrong driver type specified: %s", driverType)
 }
-
-func runCmdPipingStdout(printer *output.Printer, cmd *exec.Cmd) error {
-	stdout, err := cmd.StdoutPipe()
-	if err != nil {
-		printer.Logger.Warn("Failed to pipe output. Trying without piping.", printer.Logger.Args("err", err))
-		_, err = cmd.Output()
-	} else {
-		defer stdout.Close()
-		err = cmd.Start()
-		if err != nil {
-			printer.Logger.Warn("Failed to execute command.", printer.Logger.Args("err", err))
-		} else {
-			// print the output of the subprocess line by line
-			scanner := bufio.NewScanner(stdout)
-			for scanner.Scan() {
-				m := scanner.Text()
-				printer.DefaultText.Println(m)
-			}
-			err = cmd.Wait()
-		}
-	}
-	return err
-}