Merge pull request #12487 from jwhonce/issues/10974-2

Refactor podman system to report.Formatter

6 files changed, 116 insertions, 103 deletions

diff --git a/cmd/podman/system/connection/list.go b/cmd/podman/system/connection/list.go
index 2710142a8..fbae1e4eb 100644
--- a/cmd/podman/system/connection/list.go
+++ b/cmd/podman/system/connection/list.go
@@ -53,13 +53,6 @@ func list(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
-	hdrs := []map[string]string{{
-		"Identity": "Identity",
-		"Name":     "Name",
-		"URI":      "URI",
-		"Default":  "Default",
-	}}
-
 	rows := make([]namedDestination, 0)
 	for k, v := range cfg.Engine.ServiceDestinations {
 		def := false
@@ -82,35 +75,37 @@ func list(cmd *cobra.Command, _ []string) error {
 		return rows[i].Name < rows[j].Name
 	})
 
-	var format string
-	switch {
-	case report.IsJSON(cmd.Flag("format").Value.String()):
+	rpt := report.New(os.Stdout, cmd.Name())
+	defer rpt.Flush()
+
+	if report.IsJSON(cmd.Flag("format").Value.String()) {
 		buf, err := registry.JSONLibrary().MarshalIndent(rows, "", "    ")
 		if err == nil {
 			fmt.Println(string(buf))
 		}
 		return err
-	case cmd.Flags().Changed("format"):
-		format = report.NormalizeFormat(cmd.Flag("format").Value.String())
-	default:
-		format = "{{.Name}}\t{{.URI}}\t{{.Identity}}\t{{.Default}}\n"
 	}
-	format = report.EnforceRange(format)
 
-	tmpl, err := report.NewTemplate("list").Parse(format)
-	if err != nil {
-		return err
+	if cmd.Flag("format").Changed {
+		rpt, err = rpt.Parse(report.OriginUser, cmd.Flag("format").Value.String())
+	} else {
+		rpt, err = rpt.Parse(report.OriginPodman,
+			"{{range .}}{{.Name}}\t{{.URI}}\t{{.Identity}}\t{{.Default}}\n{{end -}}")
 	}
-
-	w, err := report.NewWriterDefault(os.Stdout)
 	if err != nil {
 		return err
 	}
-	defer w.Flush()
 
-	isTable := report.HasTable(cmd.Flag("format").Value.String())
-	if !cmd.Flag("format").Changed || isTable {
-		_ = tmpl.Execute(w, hdrs)
+	if rpt.RenderHeaders {
+		err = rpt.Execute([]map[string]string{{
+			"Default":  "Default",
+			"Identity": "Identity",
+			"Name":     "Name",
+			"URI":      "URI",
+		}})
+		if err != nil {
+			return err
+		}
 	}
-	return tmpl.Execute(w, rows)
+	return rpt.Execute(rows)
 }
diff --git a/cmd/podman/system/df.go b/cmd/podman/system/df.go
index dfde3bc07..b65a1ca55 100644
--- a/cmd/podman/system/df.go
+++ b/cmd/podman/system/df.go
@@ -54,23 +54,19 @@ func df(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	w, err := report.NewWriterDefault(os.Stdout)
-	if err != nil {
-		return err
-	}
-
 	if dfOptions.Verbose {
-		return printVerbose(w, cmd, reports)
+		return printVerbose(cmd, reports)
 	}
-	return printSummary(w, cmd, reports)
+	return printSummary(cmd, reports)
 }
 
-func printSummary(w *report.Writer, cmd *cobra.Command, reports *entities.SystemDfReport) error {
+func printSummary(cmd *cobra.Command, reports *entities.SystemDfReport) error {
 	var (
 		dfSummaries       []*dfSummary
 		active            int
 		size, reclaimable int64
 	)
+
 	for _, i := range reports.Images {
 		if i.Containers > 0 {
 			active++
@@ -136,17 +132,28 @@ func printSummary(w *report.Writer, cmd *cobra.Command, reports *entities.System
 		"Size":        "SIZE",
 		"Reclaimable": "RECLAIMABLE",
 	})
-	row := "{{.Type}}\t{{.Total}}\t{{.Active}}\t{{.Size}}\t{{.Reclaimable}}\n"
+
+	rpt := report.New(os.Stdout, cmd.Name())
+	defer rpt.Flush()
+
+	var err error
 	if cmd.Flags().Changed("format") {
-		row = report.NormalizeFormat(dfOptions.Format)
+		rpt, err = rpt.Parse(report.OriginUser, dfOptions.Format)
+	} else {
+		row := "{{range . }}{{.Type}}\t{{.Total}}\t{{.Active}}\t{{.Size}}\t{{.Reclaimable}}\n{{end -}}"
+		rpt, err = rpt.Parse(report.OriginPodman, row)
+	}
+	if err != nil {
+		return err
 	}
-	return writeTemplate(w, cmd, hdrs, row, dfSummaries)
+	return writeTemplate(rpt, hdrs, dfSummaries)
 }
 
-func printVerbose(w *report.Writer, cmd *cobra.Command, reports *entities.SystemDfReport) error {
-	defer w.Flush()
+func printVerbose(cmd *cobra.Command, reports *entities.SystemDfReport) error { // nolint:interfacer
+	rpt := report.New(os.Stdout, cmd.Name())
+	defer rpt.Flush()
 
-	fmt.Fprint(w, "Images space usage:\n\n")
+	fmt.Fprint(rpt.Writer(), "Images space usage:\n\n")
 	// convert to dfImage for output
 	dfImages := make([]*dfImage, 0, len(reports.Images))
 	for _, d := range reports.Images {
@@ -157,12 +164,16 @@ func printVerbose(w *report.Writer, cmd *cobra.Command, reports *entities.System
 		"SharedSize": "SHARED SIZE",
 		"UniqueSize": "UNIQUE SIZE",
 	})
-	imageRow := "{{.Repository}}\t{{.Tag}}\t{{.ImageID}}\t{{.Created}}\t{{.Size}}\t{{.SharedSize}}\t{{.UniqueSize}}\t{{.Containers}}\n"
-	if err := writeTemplate(w, cmd, hdrs, imageRow, dfImages); err != nil {
+	imageRow := "{{range .}}{{.Repository}}\t{{.Tag}}\t{{.ImageID}}\t{{.Created}}\t{{.Size}}\t{{.SharedSize}}\t{{.UniqueSize}}\t{{.Containers}}\n{{end -}}"
+	rpt, err := rpt.Parse(report.OriginPodman, imageRow)
+	if err != nil {
+		return err
+	}
+	if err := writeTemplate(rpt, hdrs, dfImages); err != nil {
 		return nil
 	}
 
-	fmt.Fprint(w, "\nContainers space usage:\n\n")
+	fmt.Fprint(rpt.Writer(), "\nContainers space usage:\n\n")
 	// convert to dfContainers for output
 	dfContainers := make([]*dfContainer, 0, len(reports.Containers))
 	for _, d := range reports.Containers {
@@ -173,12 +184,16 @@ func printVerbose(w *report.Writer, cmd *cobra.Command, reports *entities.System
 		"LocalVolumes": "LOCAL VOLUMES",
 		"RWSize":       "SIZE",
 	})
-	containerRow := "{{.ContainerID}}\t{{.Image}}\t{{.Command}}\t{{.LocalVolumes}}\t{{.RWSize}}\t{{.Created}}\t{{.Status}}\t{{.Names}}\n"
-	if err := writeTemplate(w, cmd, hdrs, containerRow, dfContainers); err != nil {
+	containerRow := "{{range .}}{{.ContainerID}}\t{{.Image}}\t{{.Command}}\t{{.LocalVolumes}}\t{{.RWSize}}\t{{.Created}}\t{{.Status}}\t{{.Names}}\n{{end -}}"
+	rpt, err = rpt.Parse(report.OriginPodman, containerRow)
+	if err != nil {
+		return err
+	}
+	if err := writeTemplate(rpt, hdrs, dfContainers); err != nil {
 		return nil
 	}
 
-	fmt.Fprint(w, "\nLocal Volumes space usage:\n\n")
+	fmt.Fprint(rpt.Writer(), "\nLocal Volumes space usage:\n\n")
 	dfVolumes := make([]*dfVolume, 0, len(reports.Volumes))
 	// convert to dfVolume for output
 	for _, d := range reports.Volumes {
@@ -187,25 +202,21 @@ func printVerbose(w *report.Writer, cmd *cobra.Command, reports *entities.System
 	hdrs = report.Headers(entities.SystemDfVolumeReport{}, map[string]string{
 		"VolumeName": "VOLUME NAME",
 	})
-	volumeRow := "{{.VolumeName}}\t{{.Links}}\t{{.Size}}\n"
-	return writeTemplate(w, cmd, hdrs, volumeRow, dfVolumes)
-}
-
-func writeTemplate(w *report.Writer, cmd *cobra.Command, hdrs []map[string]string, format string, output interface{}) error {
-	defer w.Flush()
-
-	format = report.EnforceRange(format)
-	tmpl, err := report.NewTemplate("df").Parse(format)
+	volumeRow := "{{range .}}{{.VolumeName}}\t{{.Links}}\t{{.Size}}\n{{end -}}"
+	rpt, err = rpt.Parse(report.OriginPodman, volumeRow)
 	if err != nil {
 		return err
 	}
+	return writeTemplate(rpt, hdrs, dfVolumes)
+}
 
-	if !cmd.Flags().Changed("format") {
-		if err := tmpl.Execute(w, hdrs); err != nil {
+func writeTemplate(rpt *report.Formatter, hdrs []map[string]string, output interface{}) error {
+	if rpt.RenderHeaders {
+		if err := rpt.Execute(hdrs); err != nil {
 			return err
 		}
 	}
-	return tmpl.Execute(w, output)
+	return rpt.Execute(output)
 }
 
 type dfImage struct {
diff --git a/cmd/podman/system/events.go b/cmd/podman/system/events.go
index e698e6652..881b9b8f4 100644
--- a/cmd/podman/system/events.go
+++ b/cmd/podman/system/events.go
@@ -77,7 +77,7 @@ func eventsCmd(cmd *cobra.Command, _ []string) error {
 	errChannel := make(chan error)
 
 	var (
-		tmpl   *report.Template
+		rpt    *report.Formatter
 		doJSON bool
 	)
 
@@ -85,7 +85,7 @@ func eventsCmd(cmd *cobra.Command, _ []string) error {
 		doJSON = report.IsJSON(eventFormat)
 		if !doJSON {
 			var err error
-			tmpl, err = report.NewTemplate("events").Parse(eventFormat)
+			rpt, err = report.New(os.Stdout, cmd.Name()).Parse(report.OriginUser, eventFormat)
 			if err != nil {
 				return err
 			}
@@ -108,10 +108,10 @@ func eventsCmd(cmd *cobra.Command, _ []string) error {
 			}
 			fmt.Println(jsonStr)
 		case cmd.Flags().Changed("format"):
-			if err := tmpl.Execute(os.Stdout, event); err != nil {
+			if err := rpt.Execute(event); err != nil {
 				return err
 			}
-			fmt.Println("")
+			os.Stdout.WriteString("\n")
 		default:
 			fmt.Println(event.ToHumanReadable(!noTrunc))
 		}
diff --git a/cmd/podman/system/info.go b/cmd/podman/system/info.go
index c3f543e6a..c323ee276 100644
--- a/cmd/podman/system/info.go
+++ b/cmd/podman/system/info.go
@@ -3,6 +3,7 @@ package system
 import (
 	"fmt"
 	"os"
+	"text/template"
 
 	"github.com/containers/common/pkg/completion"
 	"github.com/containers/common/pkg/report"
@@ -84,7 +85,10 @@ func info(cmd *cobra.Command, args []string) error {
 		}
 		fmt.Println(string(b))
 	case cmd.Flags().Changed("format"):
-		tmpl, err := report.NewTemplate("info").Parse(inFormat)
+		// Cannot use report.New() as it enforces {{range .}} for OriginUser templates
+		tmpl := template.New(cmd.Name()).Funcs(template.FuncMap(report.DefaultFuncs))
+		inFormat = report.NormalizeFormat(inFormat)
+		tmpl, err := tmpl.Parse(inFormat)
 		if err != nil {
 			return err
 		}
diff --git a/cmd/podman/system/version.go b/cmd/podman/system/version.go
index 87b806503..521bb8774 100644
--- a/cmd/podman/system/version.go
+++ b/cmd/podman/system/version.go
@@ -2,16 +2,15 @@ package system
 
 import (
 	"fmt"
-	"io"
 	"os"
 	"strings"
+	"text/template"
 
 	"github.com/containers/common/pkg/completion"
 	"github.com/containers/common/pkg/report"
 	"github.com/containers/podman/v3/cmd/podman/common"
 	"github.com/containers/podman/v3/cmd/podman/registry"
 	"github.com/containers/podman/v3/cmd/podman/validate"
-	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/pkg/domain/entities"
 	"github.com/spf13/cobra"
 )
@@ -53,54 +52,60 @@ func version(cmd *cobra.Command, args []string) error {
 		return nil
 	}
 
-	w, err := report.NewWriterDefault(os.Stdout)
-	if err != nil {
-		return err
-	}
-	defer w.Flush()
+	if cmd.Flag("format").Changed {
+		// Cannot use report.New() as it enforces {{range .}} for OriginUser templates
+		tmpl := template.New(cmd.Name()).Funcs(template.FuncMap(report.DefaultFuncs))
 
-	if cmd.Flags().Changed("format") {
-		row := report.NormalizeFormat(versionFormat)
-		tmpl, err := report.NewTemplate("version 2.0.0").Parse(row)
+		versionFormat = report.NormalizeFormat(versionFormat)
+		tmpl, err := tmpl.Parse(versionFormat)
 		if err != nil {
 			return err
 		}
-		if err := tmpl.Execute(w, versions); err != nil {
+		if err := tmpl.Execute(os.Stdout, versions); err != nil {
 			// On Failure, assume user is using older version of podman version --format and check client
-			row = strings.ReplaceAll(row, ".Server.", ".")
-			tmpl, err := report.NewTemplate("version 1.0.0").Parse(row)
+			versionFormat = strings.ReplaceAll(versionFormat, ".Server.", ".")
+			tmpl, err := tmpl.Parse(versionFormat)
 			if err != nil {
 				return err
 			}
-			if err := tmpl.Execute(w, versions.Client); err != nil {
+			if err := tmpl.Execute(os.Stdout, versions.Client); err != nil {
 				return err
 			}
 		}
 		return nil
 	}
 
-	if versions.Server != nil {
-		if _, err := fmt.Fprintf(w, "Client:\n"); err != nil {
-			return err
-		}
-		formatVersion(w, versions.Client)
-		if _, err := fmt.Fprintf(w, "\nServer:\n"); err != nil {
-			return err
-		}
-		formatVersion(w, versions.Server)
-	} else {
-		formatVersion(w, versions.Client)
+	rpt := report.New(os.Stdout, cmd.Name())
+	defer rpt.Flush()
+	rpt, err = rpt.Parse(report.OriginPodman, versionTemplate)
+	if err != nil {
+		return err
 	}
-	return nil
+	return rpt.Execute(versions)
 }
 
-func formatVersion(w io.Writer, version *define.Version) {
-	fmt.Fprintf(w, "Version:\t%s\n", version.Version)
-	fmt.Fprintf(w, "API Version:\t%s\n", version.APIVersion)
-	fmt.Fprintf(w, "Go Version:\t%s\n", version.GoVersion)
-	if version.GitCommit != "" {
-		fmt.Fprintf(w, "Git Commit:\t%s\n", version.GitCommit)
-	}
-	fmt.Fprintf(w, "Built:\t%s\n", version.BuiltTime)
-	fmt.Fprintf(w, "OS/Arch:\t%s\n", version.OsArch)
-}
+const versionTemplate = `{{with .Client -}}
+Client:\tPodman Engine
+Version:\t{{.Version}}
+API Version:\t{{.APIVersion}}
+Go Version:\t{{.GoVersion}}
+{{if .GitCommit -}}
+  Git Commit:\t{{.GitCommit}}
+{{- end}}
+Built:\t{{.BuiltTime}}
+OS/Arch:\t{{.OsArch}}
+{{- end}}
+
+{{- if .Server }}{{with .Server}}
+
+Server:\tPodman Engine
+Version:\t{{.Version}}
+API Version:\t{{.APIVersion}}
+Go Version:\t{{.GoVersion}}
+{{if .GitCommit -}}
+  Git Commit:\t{{.GitCommit}}
+{{- end}}
+Built:\t{{.BuiltTime}}
+OS/Arch:\t{{.OsArch}}
+{{- end}}{{- end}}
+`
diff --git a/test/system/001-basic.bats b/test/system/001-basic.bats
index 03f07d602..23489c1b5 100644
--- a/test/system/001-basic.bats
+++ b/test/system/001-basic.bats
@@ -15,12 +15,10 @@ function setup() {
 @test "podman version emits reasonable output" {
     run_podman version
 
-    # First line of podman-remote is "Client:<blank>".
+    # First line of podman version is "Client: *Podman Engine".
     # Just delete it (i.e. remove the first entry from the 'lines' array)
-    if is_remote; then
-        if expr "${lines[0]}" : "Client:" >/dev/null; then
-            lines=("${lines[@]:1}")
-        fi
+    if expr "${lines[0]}" : "Client: *" >/dev/null; then
+        lines=("${lines[@]:1}")
     fi
 
     is "${lines[0]}" "Version:[ ]\+[1-9][0-9.]\+" "Version line 1"