Fixes crc-org#3546 crc-org#3550 always show progress during uncompres…

…sion of the bundle As part of `crc setup` we show the progress of the download and uncompression. However, the download progress is always shown, while the uncompression is suppressed when this is not a standard terninal. For consistency we remove the suppression and add additional complexity to allow this to be forced.
gbraad-redhat · Mar 20, 2023 · fb84ee0 · fb84ee0
1 parent ba0b846
commit fb84ee0
Show file tree

Hide file tree

Showing 10 changed files with 51 additions and 23 deletions.
diff --git a/cmd/crc/cmd/setup.go b/cmd/crc/cmd/setup.go
@@ -11,19 +11,22 @@ import (
 	"github.com/crc-org/crc/pkg/crc/input"
 	"github.com/crc-org/crc/pkg/crc/preflight"
 	"github.com/crc-org/crc/pkg/crc/validation"
+	crcTerminal "github.com/crc-org/crc/pkg/os/terminal"
 
 	"github.com/spf13/cobra"
 	"k8s.io/client-go/util/exec"
 )
 
 var (
-	checkOnly bool
+	checkOnly             bool
+	forceShowProgressbars bool
 )
 
 func init() {
 	setupCmd.Flags().Bool(crcConfig.ExperimentalFeatures, false, "Allow the use of experimental features")
 	setupCmd.Flags().StringP(crcConfig.Bundle, "b", constants.GetDefaultBundlePath(crcConfig.GetPreset(config)), "Bundle to use for instance")
 	setupCmd.Flags().BoolVar(&checkOnly, "check-only", false, "Only run the preflight checks, don't try to fix any misconfiguration")
+	setupCmd.Flags().BoolVar(&forceShowProgressbars, "show-progressbars", false, "Always show the progress bars for download and extraction")
 	addOutputFormatFlag(setupCmd)
 	rootCmd.AddCommand(setupCmd)
 }
@@ -63,6 +66,8 @@ func runSetup(arguments []string) error {
 		}
 	}
 
+	// set global variable to force terminal output
+	crcTerminal.ForceShowOutput = forceShowProgressbars
 	err := preflight.SetupHost(config, checkOnly)
 	if err != nil && checkOnly {
 		err = exec.CodeExitError{

diff --git a/pkg/crc/cache/cache.go b/pkg/crc/cache/cache.go
@@ -120,7 +120,7 @@ func (c *Cache) cacheExecutable() error {
 	// Check the file is tarball or not
 	if isTarball(assetTmpFile) {
 		// Extract the tarball and put it the cache directory.
-		extractedFiles, err = extract.UncompressWithFilter(assetTmpFile, tmpDir, false,
+		extractedFiles, err = extract.UncompressWithFilter(assetTmpFile, tmpDir,
 			func(filename string) bool { return filepath.Base(filename) == c.GetExecutableName() })
 		if err != nil {
 			return errors.Wrapf(err, "Cannot uncompress '%s'", assetTmpFile)

diff --git a/pkg/crc/cluster/pullsecret.go b/pkg/crc/cluster/pullsecret.go
@@ -15,7 +15,7 @@ import (
 	"github.com/crc-org/crc/pkg/crc/logging"
 	"github.com/crc-org/crc/pkg/crc/preset"
 	"github.com/crc-org/crc/pkg/crc/validation"
-	crcos "github.com/crc-org/crc/pkg/os"
+	crcTerminal "github.com/crc-org/crc/pkg/os/terminal"
 
 	"github.com/AlecAivazis/survey/v2"
 	"github.com/zalando/go-keyring"
@@ -171,7 +171,7 @@ You can copy it from the Pull Secret section of %s.
 // promptUserForSecret can be used for any kind of secret like image pull
 // secret or for password.
 func promptUserForSecret() (string, error) {
-	if !crcos.RunningInTerminal() {
+	if !crcTerminal.IsRunningInTerminal() {
 		return "", errors.New("cannot ask for secret, crc not launched by a terminal")
 	}
 

diff --git a/pkg/crc/input/input.go b/pkg/crc/input/input.go
@@ -4,14 +4,14 @@ import (
 	"fmt"
 	"strings"
 
-	crcos "github.com/crc-org/crc/pkg/os"
+	crcTerminal "github.com/crc-org/crc/pkg/os/terminal"
 )
 
 func PromptUserForYesOrNo(message string, force bool) bool {
 	if force {
 		return true
 	}
-	if !crcos.RunningInTerminal() {
+	if !crcTerminal.IsRunningInTerminal() {
 		return false
 	}
 	var response string

diff --git a/pkg/crc/segment/segment.go b/pkg/crc/segment/segment.go
@@ -22,6 +22,7 @@ import (
 	"github.com/crc-org/crc/pkg/crc/telemetry"
 	"github.com/crc-org/crc/pkg/crc/version"
 	crcos "github.com/crc-org/crc/pkg/os"
+	crcTerminal "github.com/crc-org/crc/pkg/os/terminal"
 	"github.com/pborman/uuid"
 	"github.com/segmentio/analytics-go/v3"
 	"github.com/spf13/cast"
@@ -197,7 +198,7 @@ func properties(ctx context.Context, err error, duration time.Duration) analytic
 	properties = properties.
 		Set("success", err == nil).
 		Set("duration", duration.Milliseconds()).
-		Set("tty", crcos.RunningInTerminal()).
+		Set("tty", crcTerminal.IsRunningInTerminal()).
 		Set("remote", crcos.RunningUsingSSH())
 	if err != nil {
 		properties = properties.Set("error", telemetry.SetError(err)).

diff --git a/pkg/download/download.go b/pkg/download/download.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/crc-org/crc/pkg/crc/logging"
 	"github.com/crc-org/crc/pkg/crc/network"
+	"github.com/crc-org/crc/pkg/os/terminal"
 
 	"github.com/cavaliergopher/grab/v3"
 	"github.com/cheggaaa/pb/v3"
@@ -28,15 +29,20 @@ func doRequest(client *grab.Client, req *grab.Request) (string, error) {
 
 	t := time.NewTicker(500 * time.Millisecond)
 	defer t.Stop()
-	bar := pb.Start64(resp.Size())
-	bar.Set(pb.Bytes, true)
-	defer bar.Finish()
+	var bar *pb.ProgressBar
+	if terminal.IsShowTerminalOutput() {
+		bar = pb.Start64(resp.Size())
+		bar.Set(pb.Bytes, true)
+		defer bar.Finish()
+	}
 
 loop:
 	for {
 		select {
 		case <-t.C:
-			bar.SetCurrent(resp.BytesComplete())
+			if terminal.IsShowTerminalOutput() {
+				bar.SetCurrent(resp.BytesComplete())
+			}
 		case <-resp.Done:
 			break loop
 		}

diff --git a/pkg/extract/extract.go b/pkg/extract/extract.go
@@ -13,28 +13,29 @@ import (
 	"github.com/cheggaaa/pb/v3"
 	"github.com/crc-org/crc/pkg/crc/logging"
 	crcos "github.com/crc-org/crc/pkg/os"
+	"github.com/crc-org/crc/pkg/os/terminal"
+
 	"github.com/h2non/filetype"
 	"github.com/klauspost/compress/zstd"
 	"github.com/pkg/errors"
 	"github.com/xi2/xz"
-	terminal "golang.org/x/term"
 )
 
 const minSizeForProgressBar = 100_000_000
 
-func UncompressWithFilter(tarball, targetDir string, showProgress bool, fileFilter func(string) bool) ([]string, error) {
-	return uncompress(tarball, targetDir, fileFilter, showProgress && terminal.IsTerminal(int(os.Stdout.Fd())))
+func UncompressWithFilter(tarball, targetDir string, fileFilter func(string) bool) ([]string, error) {
+	return uncompress(tarball, targetDir, fileFilter, false) // never show detailed output
 }
 
 func Uncompress(tarball, targetDir string, showProgress bool) ([]string, error) {
-	return uncompress(tarball, targetDir, nil, showProgress && terminal.IsTerminal(int(os.Stdout.Fd())))
+	return uncompress(tarball, targetDir, nil, terminal.IsShowTerminalOutput())
 }
 
 func uncompress(tarball, targetDir string, fileFilter func(string) bool, showProgress bool) ([]string, error) {
 	logging.Debugf("Uncompressing %s to %s", tarball, targetDir)
 
 	if strings.HasSuffix(tarball, ".zip") {
-		return unzip(tarball, targetDir, fileFilter, showProgress)
+		return unzip(tarball, targetDir, fileFilter, terminal.IsShowTerminalOutput())
 	}
 
 	file, err := os.Open(filepath.Clean(tarball))

diff --git a/pkg/extract/extract_test.go b/pkg/extract/extract_test.go
@@ -132,7 +132,7 @@ func testUncompress(t *testing.T, archiveName string, fileFilter func(string) bo
 	var fileList []string
 	var err error
 	if fileFilter != nil {
-		fileList, err = UncompressWithFilter(archiveName, destDir, false, fileFilter)
+		fileList, err = UncompressWithFilter(archiveName, destDir, fileFilter)
 	} else {
 		fileList, err = Uncompress(archiveName, destDir, false)
 	}

diff --git a/pkg/os/terminal/terminal.go b/pkg/os/terminal/terminal.go
@@ -0,0 +1,21 @@
+package terminal
+
+import (
+	"os"
+
+	xterminal "golang.org/x/term"
+)
+
+var (
+	// Global variable to force output regardless if terninal
+	ForceShowOutput = false
+)
+
+func IsShowTerminalOutput() bool {
+	// if this is a terminal or set to force output
+	return IsRunningInTerminal() || ForceShowOutput
+}
+
+func IsRunningInTerminal() bool {
+	return xterminal.IsTerminal(int(os.Stdout.Fd()))
+}
diff --git a/pkg/os/util.go b/pkg/os/util.go
@@ -9,8 +9,6 @@ import (
 	"strings"
 
 	"github.com/crc-org/crc/pkg/crc/logging"
-
-	terminal "golang.org/x/term"
 )
 
 // ReplaceOrAddEnv changes the value of an environment variable if it exists otherwise add the new variable
@@ -112,10 +110,6 @@ func RemoveFileIfExists(path string) error {
 	return nil
 }
 
-func RunningInTerminal() bool {
-	return terminal.IsTerminal(int(os.Stdin.Fd()))
-}
-
 func RunningUsingSSH() bool {
 	return os.Getenv("SSH_TTY") != ""
 }