Repository: code-ready/crc
Pull request #1429 (2020-07-31 12:37:09)

Add json output for crc version
Continuation of #1227

openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1429#" title="Author self-approved">guillaumerose</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=code-ready%2Fcrc).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/code-ready/crc/blob/master/OWNERS)~~ [guillaumerose]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
openshift-ci-robot: @guillaumerose: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/e2e-crc | 6ae8ca217c8e4f786be03f880cc9f25b0a5137c8 | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/code-ready_crc/1429/pull-ci-code-ready-crc-master-e2e-crc/1289178289218260992) | `/test e2e-crc`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=code-ready&repo=crc&pr=1429). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:guillaumerose).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
anjannath: Commit message in https://github.com/code-ready/crc/pull/1429/commits/6ae8ca217c8e4f786be03f880cc9f25b0a5137c8 has typos
guillaumerose: thanks fixed!
openshift-ci-robot: @guillaumerose: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/e2e-crc | 8b99b1a3f4180c9c7a2f0324ac9cb8151de3031a | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/code-ready_crc/1429/pull-ci-code-ready-crc-master-e2e-crc/1290586493219246080) | `/test e2e-crc`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=code-ready&repo=crc&pr=1429). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:guillaumerose).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1429#pullrequestreview-461753151" title="Approved">cfergeau</a>*, *<a href="https://github.com/code-ready/crc/pull/1429#" title="Author self-approved">guillaumerose</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=code-ready%2Fcrc).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/code-ready/crc/blob/master/OWNERS)~~ [cfergeau,guillaumerose]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
openshift-ci-robot: New changes are detected. LGTM label has been removed.

Patch:
diff --git a/cmd/crc/cmd/json.go b/cmd/crc/cmd/json.go
new file mode 100644
index 000000000..e988d901c
--- /dev/null
+++ b/cmd/crc/cmd/json.go
@@ -0,0 +1,36 @@
+package cmd
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+
+	"github.com/spf13/cobra"
+)
+
+const jsonFormat = "json"
+
+var (
+	outputFormat string
+)
+
+func addOutputFormatFlag(cmd *cobra.Command) {
+	cmd.Flags().StringVarP(&outputFormat, "output", "o", "", "Output format. One of: json")
+}
+
+type prettyPrintable interface {
+	prettyPrintTo(writer io.Writer) error
+}
+
+func render(obj prettyPrintable, writer io.Writer, outputFormat string) error {
+	switch outputFormat {
+	case jsonFormat:
+		encoder := json.NewEncoder(writer)
+		encoder.SetIndent("", "  ")
+		return encoder.Encode(obj)
+	case "":
+		return obj.prettyPrintTo(writer)
+	default:
+		return fmt.Errorf("invalid format: %s", outputFormat)
+	}
+}
diff --git a/cmd/crc/cmd/root.go b/cmd/crc/cmd/root.go
index 3c05bcd05..b7433b413 100644
--- a/cmd/crc/cmd/root.go
+++ b/cmd/crc/cmd/root.go
@@ -57,7 +57,8 @@ func runPrerun() {
 	// Setting up logrus
 	logging.InitLogrus(logging.LogLevel, constants.LogFilePath)
 	setProxyDefaults()
-	for _, str := range GetVersionStrings() {
+
+	for _, str := range defaultVersion().lines() {
 		logging.Debugf(str)
 	}
 }
diff --git a/cmd/crc/cmd/start.go b/cmd/crc/cmd/start.go
index 64f3d51fa..1db24f7a0 100644
--- a/cmd/crc/cmd/start.go
+++ b/cmd/crc/cmd/start.go
@@ -15,7 +15,7 @@ import (
 	"github.com/code-ready/crc/pkg/crc/output"
 	"github.com/code-ready/crc/pkg/crc/preflight"
 	"github.com/code-ready/crc/pkg/crc/validation"
-	"github.com/code-ready/crc/pkg/crc/version"
+	crcversion "github.com/code-ready/crc/pkg/crc/version"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -141,7 +141,7 @@ func checkIfNewVersionAvailable(noUpdateCheck bool) {
 	if noUpdateCheck {
 		return
 	}
-	isNewVersionAvailable, newVersion, err := version.NewVersionAvailable()
+	isNewVersionAvailable, newVersion, err := crcversion.NewVersionAvailable()
 	if err != nil {
 		logging.Debugf("Unable to find out if a new version is available: %v", err)
 		return
diff --git a/cmd/crc/cmd/status.go b/cmd/crc/cmd/status.go
index f259d9534..5dadfd01d 100644
--- a/cmd/crc/cmd/status.go
+++ b/cmd/crc/cmd/status.go
@@ -1,7 +1,6 @@
 package cmd
 
 import (
-	"encoding/json"
 	"fmt"
 	"io"
 	"os"
@@ -15,14 +14,8 @@ import (
 	"github.com/spf13/cobra"
 )
 
-const jsonFormat = "json"
-
-var (
-	outputFormat string
-)
-
 func init() {
-	statusCmd.Flags().StringVarP(&outputFormat, "output", "o", "", "Output format. One of: json")
+	addOutputFormatFlag(statusCmd)
 	rootCmd.AddCommand(statusCmd)
 }
 
@@ -37,7 +30,7 @@ var statusCmd = &cobra.Command{
 	},
 }
 
-type Status struct {
+type status struct {
 	CrcStatus        string `json:"crcStatus"`
 	OpenShiftStatus  string `json:"openshiftStatus"`
 	OpenShiftVersion string `json:"openshiftVersion"`
@@ -68,7 +61,7 @@ func runStatus(writer io.Writer, client client, cacheDir, outputFormat string) e
 	if err != nil {
 		return fmt.Errorf("Error finding size of cache: %s", err.Error())
 	}
-	status := Status{
+	status := &status{
 		CrcStatus:        clusterStatus.CrcStatus,
 		OpenShiftStatus:  clusterStatus.OpenshiftStatus,
 		OpenShiftVersion: clusterStatus.OpenshiftVersion,
@@ -77,29 +70,23 @@ func runStatus(writer io.Writer, client client, cacheDir, outputFormat string) e
 		CacheUsage:       size,
 		CacheDir:         cacheDir,
 	}
-
-	if outputFormat == jsonFormat {
-		encoder := json.NewEncoder(writer)
-		encoder.SetIndent("", "  ")
-		return encoder.Encode(status)
-	}
-	return printStatus(writer, status)
+	return render(status, writer, outputFormat)
 }
 
-func printStatus(writer io.Writer, status Status) error {
+func (s *status) prettyPrintTo(writer io.Writer) error {
 	w := tabwriter.NewWriter(writer, 0, 0, 1, ' ', 0)
 
 	lines := []struct {
 		left, right string
 	}{
-		{"CRC VM", status.CrcStatus},
-		{"OpenShift", openshiftStatus(status)},
+		{"CRC VM", s.CrcStatus},
+		{"OpenShift", openshiftStatus(s)},
 		{"Disk Usage", fmt.Sprintf(
 			"%s of %s (Inside the CRC VM)",
-			units.HumanSize(float64(status.DiskUsage)),
-			units.HumanSize(float64(status.DiskSize)))},
-		{"Cache Usage", units.HumanSize(float64(status.CacheUsage))},
-		{"Cache Directory", status.CacheDir},
+			units.HumanSize(float64(s.DiskUsage)),
+			units.HumanSize(float64(s.DiskSize)))},
+		{"Cache Usage", units.HumanSize(float64(s.CacheUsage))},
+		{"Cache Directory", s.CacheDir},
 	}
 	for _, line := range lines {
 		if err := printLine(w, line.left, line.right); err != nil {
@@ -109,7 +96,7 @@ func printStatus(writer io.Writer, status Status) error {
 	return w.Flush()
 }
 
-func openshiftStatus(status Status) string {
+func openshiftStatus(status *status) string {
 	if status.OpenShiftVersion != "" {
 		return fmt.Sprintf("%s (v%s)", status.OpenShiftStatus, status.OpenShiftVersion)
 	}
diff --git a/cmd/crc/cmd/version.go b/cmd/crc/cmd/version.go
index 2e8e59897..8f26caf60 100644
--- a/cmd/crc/cmd/version.go
+++ b/cmd/crc/cmd/version.go
@@ -2,14 +2,17 @@ package cmd
 
 import (
 	"fmt"
-	"strings"
+	"io"
+	"os"
 
 	"github.com/code-ready/crc/pkg/crc/constants"
-	"github.com/code-ready/crc/pkg/crc/version"
+	"github.com/code-ready/crc/pkg/crc/exit"
+	crcversion "github.com/code-ready/crc/pkg/crc/version"
 	"github.com/spf13/cobra"
 )
 
 func init() {
+	addOutputFormatFlag(versionCmd)
 	rootCmd.AddCommand(versionCmd)
 }
 
@@ -18,21 +21,48 @@ var versionCmd = &cobra.Command{
 	Short: "Print version information",
 	Long:  "Print version information",
 	Run: func(cmd *cobra.Command, args []string) {
-		runPrintVersion(args)
+		if err := runPrintVersion(os.Stdout, defaultVersion(), outputFormat); err != nil {
+			exit.WithMessage(1, err.Error())
+		}
 	},
 }
 
-func GetVersionStrings() []string {
+func runPrintVersion(writer io.Writer, version *version, outputFormat string) error {
+	return render(version, writer, outputFormat)
+}
+
+type version struct {
+	Version          string `json:"version"`
+	Commit           string `json:"commit"`
+	OpenshiftVersion string `json:"openshiftVersion"`
+	Embedded         bool   `json:"embedded"`
+}
+
+func defaultVersion() *version {
+	return &version{
+		Version:          crcversion.GetCRCVersion(),
+		Commit:           crcversion.GetCommitSha(),
+		OpenshiftVersion: crcversion.GetBundleVersion(),
+		Embedded:         constants.BundleEmbedded(),
+	}
+}
+
+func (v *version) prettyPrintTo(writer io.Writer) error {
+	for _, line := range v.lines() {
+		if _, err := fmt.Fprint(writer, line); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (v *version) lines() []string {
 	var embedded string
-	if !constants.BundleEmbedded() {
+	if !v.Embedded {
 		embedded = "not "
 	}
 	return []string{
-		fmt.Sprintf("CodeReady Containers version: %s+%s", version.GetCRCVersion(), version.GetCommitSha()),
-		fmt.Sprintf("OpenShift version: %s (%sembedded in binary)", version.GetBundleVersion(), embedded),
+		fmt.Sprintf("CodeReady Containers version: %s+%s\n", v.Version, v.Commit),
+		fmt.Sprintf("OpenShift version: %s (%sembedded in binary)\n", v.OpenshiftVersion, embedded),
 	}
 }
-
-func runPrintVersion(arguments []string) {
-	fmt.Println(strings.Join(GetVersionStrings(), "\n"))
-}
diff --git a/pkg/crc/exit/atexit.go b/pkg/crc/exit/atexit.go
index aaf83ba99..0c657ec34 100644
--- a/pkg/crc/exit/atexit.go
+++ b/pkg/crc/exit/atexit.go
@@ -4,27 +4,10 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/code-ready/crc/pkg/crc/logging"
 	"github.com/code-ready/crc/pkg/crc/output"
 )
 
-const ExitHandlerPanicMessage = "At least one exit handler vetoed to exit program execution"
-
-type exitHandlerFunc func(int) bool
-
-// exitHandlers keeps track of the list of registered exit handlers. Handlers are applied in the order defined in this list.
-var exitHandlers = []exitHandlerFunc{}
-
-// WithoutMessage runs all registered exit handlers and then exits the program with the specified exit code using os.Exit.
-func WithoutMessage(code int) {
-	veto := runHandlers(code)
-	if veto {
-		panic(ExitHandlerPanicMessage)
-	}
-	os.Exit(code)
-}
-
-// WithMessage runs all registered exit handlers, prints the specified message and then exits the program with the specified exit code.
+// WithMessage prints the specified message and then exits the program with the specified exit code.
 // If the exit code is 0, the message is prints to stdout, otherwise to stderr.
 func WithMessage(code int, text string, args ...interface{}) {
 	if code == 0 {
@@ -32,38 +15,5 @@ func WithMessage(code int, text string, args ...interface{}) {
 	} else {
 		_, _ = output.Fout(os.Stderr, fmt.Sprintf(text, args...))
 	}
-	WithoutMessage(code)
-}
-
-// Register registers an exit handler function which is run when WithoutMessage is called
-func RegisterExitHandler(exitHandler exitHandlerFunc) {
-	exitHandlers = append(exitHandlers, exitHandler)
-}
-
-// ClearExitHandler clears all registered exit handlers
-func ClearExitHandler() {
-	exitHandlers = []exitHandlerFunc{}
-}
-
-// runHandlers runs all registered exit handlers, passing on the intended exit code.
-// Handlers can veto to exit the program. If at least one exit handlers casts a veto, the program does panic instead of exiting
-// allowing for a potential recovering.
-func runHandlers(code int) bool {
-	var veto bool
-	for _, handler := range exitHandlers {
-		veto = veto || runHandler(handler, code)
-	}
-	return veto
-}
-
-// runHandler runs the single specified exit handler, returning whether this handler vetos the exit or not.
-func runHandler(exitHandler exitHandlerFunc, code int) bool {
-	defer func() {
-		err := recover()
-		if err != nil {
-			logging.Errorf("Error running exit handler: %v", err)
-		}
-	}()
-
-	return exitHandler(code)
+	os.Exit(code)
 }


Test patch:
diff --git a/cmd/crc/cmd/version_test.go b/cmd/crc/cmd/version_test.go
new file mode 100644
index 000000000..dd2e0c34e
--- /dev/null
+++ b/cmd/crc/cmd/version_test.go
@@ -0,0 +1,40 @@
+package cmd
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPlainVersion(t *testing.T) {
+	out := new(bytes.Buffer)
+	assert.NoError(t, runPrintVersion(out, &version{
+		Version:          "1.13",
+		Commit:           "aabbcc",
+		OpenshiftVersion: "4.5.4",
+		Embedded:         false,
+	}, ""))
+	assert.Equal(t, `CodeReady Containers version: 1.13+aabbcc
+OpenShift version: 4.5.4 (not embedded in binary)
+`, out.String())
+}
+
+func TestJsonVersion(t *testing.T) {
+	out := new(bytes.Buffer)
+	assert.NoError(t, runPrintVersion(out, &version{
+		Version:          "1.13",
+		Commit:           "aabbcc",
+		OpenshiftVersion: "4.5.4",
+		Embedded:         false,
+	}, "json"))
+
+	expected := `{
+  "version": "1.13",
+  "commit": "aabbcc",
+  "openshiftVersion": "4.5.4",
+  "embedded": false
+}
+`
+	assert.Equal(t, expected, out.String())
+}