Modify the JSON output of kpod history

The JSON output was being modified to type string, fixed that
to be of the same type as the source
This is better for further processing of the JSON output if needed
Restructured kpod history a bit as well

Signed-off-by: umohnani8 <umohnani@redhat.com>

cmd/kpod/history.go

@@ -1,20 +1,19 @@
 package main
 
 import (
-	"encoding/json"
-	"fmt"
 	"reflect"
 	"strconv"
 	"strings"
 	"time"
 
 	is "github.com/containers/image/storage"
+	"github.com/containers/image/types"
 	"github.com/containers/storage"
 	units "github.com/docker/go-units"
 	"github.com/kubernetes-incubator/cri-o/cmd/kpod/formats"
 	"github.com/kubernetes-incubator/cri-o/libpod/common"
+	"github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
 
@@ -23,27 +22,39 @@ const (
 	idTruncLength        = 13
 )
 
-// historyOutputParams stores info about each layer
+// historyTemplateParams stores info about each layer
-type historyOutputParams struct {
+type historyTemplateParams struct {
-	ID        string `json:"id"`
+	ID        string
-	Created   string `json:"created"`
+	Created   string
-	CreatedBy string `json:"createdby"`
+	CreatedBy string
-	Size      string `json:"size"`
+	Size      string
-	Comment   string `json:"comment"`
+	Comment   string
+}
+
+// historyJSONParams is only used when the JSON format is specified,
+// and is better for data processing from JSON.
+// historyJSONParams will be populated by data from v1.History and types.BlobInfo,
+// the members of the struct are the sama data types as their sources.
+type historyJSONParams struct {
+	ID        string     `json:"id"`
+	Created   *time.Time `json:"created"`
+	CreatedBy string     `json:"createdBy"`
+	Size      int64      `json:"size"`
+	Comment   string     `json:"comment"`
 }
 
 // historyOptions stores cli flag values
 type historyOptions struct {
-	image    string
+	image   string
-	human    bool
+	human   bool
-	truncate bool
+	noTrunc bool
-	quiet    bool
+	quiet   bool
-	format   string
+	format  string
 }
 
 var (
 	historyFlags = []cli.Flag{
-		cli.BoolFlag{
+		cli.BoolTFlag{
 			Name:  "human, H",
 			Usage: "Display sizes and dates in human readable format",
 		},
@@ -83,226 +94,179 @@ func historyCmd(c *cli.Context) error {
 		return err
 	}
 
-	human := true
+	format := genHistoryFormat(c.Bool("quiet"))
-	if c.IsSet("human") {
-		human = c.Bool("human")
-	}
-
-	truncate := true
-	if c.IsSet("no-trunc") {
-		truncate = !c.Bool("no-trunc")
-	}
-	quiet := false
-	if c.IsSet("quiet") {
-		quiet = c.Bool("quiet")
-	}
-	format := ""
 	if c.IsSet("format") {
 		format = c.String("format")
 	}
+
 	args := c.Args()
 	if len(args) == 0 {
-		logrus.Errorf("an image name must be specified")
+		return errors.Errorf("an image name must be specified")
-		return nil
 	}
 	if len(args) > 1 {
-		logrus.Errorf("Kpod history takes at most 1 argument")
+		return errors.Errorf("Kpod history takes at most 1 argument")
-		return nil
 	}
 	imgName := args[0]
 
 	opts := historyOptions{
-		image:    imgName,
+		image:   imgName,
-		human:    human,
+		human:   c.BoolT("human"),
-		truncate: truncate,
+		noTrunc: c.Bool("no-trunc"),
-		quiet:    quiet,
+		quiet:   c.Bool("quiet"),
-		format:   format,
+		format:  format,
 	}
-	err = outputHistory(store, opts)
+	return generateHistoryOutput(store, opts)
-	return err
 }
 
-func genHistoryFormat(quiet, truncate, human bool) (format string) {
+func genHistoryFormat(quiet bool) (format string) {
 	if quiet {
 		return formats.IDString
 	}
+	return "table {{.ID}}\t{{.Created}}\t{{.CreatedBy}}\t{{.Size}}\t{{.Comment}}\t"
+}
 
-	if truncate {
+// historyToGeneric makes an empty array of interfaces for output
-		format = "table {{ .ID | printf \"%-12.12s\" }} {{ .Created | printf \"%-16s\" }} {{ .CreatedBy | " +
+func historyToGeneric(templParams []historyTemplateParams, JSONParams []historyJSONParams) (genericParams []interface{}) {
-			"printf \"%-45.45s\" }} {{ .Size | printf \"%-16s\" }} {{ .Comment | printf \"%s\" }}"
+	if len(templParams) > 0 {
-	} else {
+		for _, v := range templParams {
-		format = "table {{ .ID | printf \"%-64s\" }} {{ .Created | printf \"%-18s\" }} {{ .CreatedBy | " +
+			genericParams = append(genericParams, interface{}(v))
-			"printf \"%-60s\" }} {{ .Size | printf \"%-16s\" }} {{ .Comment | printf \"%s\"}}"
+		}
+		return
+	}
+	for _, v := range JSONParams {
+		genericParams = append(genericParams, interface{}(v))
 	}
 	return
 }
 
-// outputTime displays the time stamp in "2017-06-20T20:24:10Z" format
+// generate the header based on the template provided
-func outputTime(tm *time.Time) string {
+func (h *historyTemplateParams) headerMap() map[string]string {
-	return tm.Format(time.RFC3339)
+	v := reflect.Indirect(reflect.ValueOf(h))
+	values := make(map[string]string)
+	for h := 0; h < v.NumField(); h++ {
+		key := v.Type().Field(h).Name
+		value := key
+		values[key] = strings.ToUpper(splitCamelCase(value))
+	}
+	return values
 }
 
-// outputHumanTime displays the time elapsed since creation
+// getHistory gets the history of an image and information about its layers
-func outputHumanTime(tm *time.Time) string {
+func getHistory(store storage.Store, image string) ([]v1.History, []types.BlobInfo, string, error) {
-	return units.HumanDuration(time.Since(*tm))
+	ref, err := is.Transport.ParseStoreReference(store, image)
-}
-
-// createJSON retrieves the history of the image and returns a JSON object
-func createJSON(store storage.Store, opts historyOptions) ([]byte, error) {
-	var (
-		size        int64
-		img         *storage.Image
-		imageID     string
-		layerAll    []historyOutputParams
-		createdTime string
-		outputSize  string
-	)
-
-	ref, err := is.Transport.ParseStoreReference(store, opts.image)
 	if err != nil {
-		return nil, errors.Errorf("error parsing reference to image %q: %v", opts.image, err)
+		return nil, nil, "", errors.Wrapf(err, "error parsing reference to image %q", image)
 	}
 
-	img, err = is.Transport.GetStoreImage(store, ref)
+	img, err := is.Transport.GetStoreImage(store, ref)
 	if err != nil {
-		return nil, errors.Errorf("no such image %q: %v", opts.image, err)
+		return nil, nil, "", errors.Wrapf(err, "no such image %q", image)
 	}
 
 	systemContext := common.GetSystemContext("")
 
 	src, err := ref.NewImage(systemContext)
 	if err != nil {
-		return nil, errors.Errorf("error instantiating image %q: %v", opts.image, err)
+		return nil, nil, "", errors.Wrapf(err, "error instantiating image %q", image)
 	}
 
 	oci, err := src.OCIConfig()
 	if err != nil {
-		return nil, err
+		return nil, nil, "", err
 	}
 
-	history := oci.History
+	return oci.History, src.LayerInfos(), img.ID, nil
-	layers := src.LayerInfos()
+}
-	count := 1
+
-	// iterating backwards to get newwest to oldest
+// getHistorytemplateOutput gets the modified history information to be printed in human readable format
+func getHistoryTemplateOutput(history []v1.History, layers []types.BlobInfo, imageID string, opts historyOptions) (historyOutput []historyTemplateParams) {
+	var (
+		outputSize  string
+		createdTime string
+		createdBy   string
+		count       = 1
+	)
 	for i := len(history) - 1; i >= 0; i-- {
-		if i == len(history)-1 {
+		if i != len(history)-1 {
-			imageID = img.ID
-		} else {
 			imageID = "<missing>"
 		}
+		if !opts.noTrunc && i == len(history)-1 {
+			imageID = imageID[:idTruncLength]
+		}
 
+		var size int64
 		if !history[i].EmptyLayer {
 			size = layers[len(layers)-count].Size
 			count++
-		} else {
-			size = 0
 		}
 
 		if opts.human {
-			createdTime = outputHumanTime(history[i].Created) + " ago"
+			createdTime = units.HumanDuration(time.Since((*history[i].Created))) + " ago"
 			outputSize = units.HumanSize(float64(size))
 		} else {
-			createdTime = outputTime(history[i].Created)
+			createdTime = (history[i].Created).Format(time.RFC3339)
 			outputSize = strconv.FormatInt(size, 10)
 		}
-		params := historyOutputParams{
+
+		createdBy = strings.Join(strings.Fields(history[i].CreatedBy), " ")
+		if !opts.noTrunc && len(createdBy) > createdByTruncLength {
+			createdBy = createdBy[:createdByTruncLength-3] + "..."
+		}
+
+		params := historyTemplateParams{
 			ID:        imageID,
 			Created:   createdTime,
-			CreatedBy: history[i].CreatedBy,
+			CreatedBy: createdBy,
 			Size:      outputSize,
 			Comment:   history[i].Comment,
 		}
-
-		layerAll = append(layerAll, params)
-	}
-
-	output, err := json.MarshalIndent(layerAll, "", "\t\t")
-	if err != nil {
-		return nil, errors.Errorf("error marshalling to JSON: %v", err)
-	}
-
-	if err = src.Close(); err != nil {
-		return nil, err
-	}
-
-	return output, nil
-}
-
-// historyToGeneric makes an empty array of interfaces for output
-func historyToGeneric(params []historyOutputParams) []interface{} {
-	genericParams := make([]interface{}, len(params))
-	for i, v := range params {
-		genericParams[i] = interface{}(v)
-	}
-	return genericParams
-}
-
-// outputHistory gets the history of the image from the JSON object
-// and pretty prints it to the screen
-func outputHistory(store storage.Store, opts historyOptions) error {
-	var (
-		outputCreatedBy string
-		imageID         string
-		history         []historyOutputParams
-	)
-
-	raw, err := createJSON(store, opts)
-	if err != nil {
-		return errors.Errorf("error creating JSON: %v", err)
-	}
-
-	if err = json.Unmarshal(raw, &history); err != nil {
-		return errors.Errorf("error Unmarshalling JSON: %v", err)
-	}
-
-	historyOutput := []historyOutputParams{}
-
-	historyFormat := opts.format
-	if historyFormat == "" {
-		historyFormat = genHistoryFormat(opts.quiet, opts.truncate, opts.human)
-	}
-
-	for i := 0; i < len(history); i++ {
-		imageID = history[i].ID
-
-		outputCreatedBy = strings.Join(strings.Fields(history[i].CreatedBy), " ")
-		if opts.truncate && len(outputCreatedBy) > createdByTruncLength {
-			outputCreatedBy = outputCreatedBy[:createdByTruncLength-3] + "..."
-		}
-
-		if opts.truncate && i == 0 {
-			imageID = history[i].ID[:idTruncLength]
-		}
-
-		params := historyOutputParams{
-			ID:        imageID,
-			Created:   history[i].Created,
-			CreatedBy: outputCreatedBy,
-			Size:      history[i].Size,
-			Comment:   history[i].Comment,
-		}
 		historyOutput = append(historyOutput, params)
 	}
+	return
+}
+
+// getHistoryJSONOutput returns the history information in its raw form
+func getHistoryJSONOutput(history []v1.History, layers []types.BlobInfo, imageID string) (historyOutput []historyJSONParams) {
+	count := 1
+	for i := len(history) - 1; i >= 0; i-- {
+		var size int64
+		if !history[i].EmptyLayer {
+			size = layers[len(layers)-count].Size
+			count++
+		}
+
+		params := historyJSONParams{
+			ID:        imageID,
+			Created:   history[i].Created,
+			CreatedBy: history[i].CreatedBy,
+			Size:      size,
+			Comment:   history[i].Comment,
+		}
+		historyOutput = append(historyOutput, params)
+	}
+	return
+}
+
+// generateHistoryOutput generates the history based on the format given
+func generateHistoryOutput(store storage.Store, opts historyOptions) error {
+	history, layers, imageID, err := getHistory(store, opts.image)
+	if err != nil {
+		return errors.Wrapf(err, "error getting history of image %q", opts.image)
+	}
+	if len(history) == 0 {
+		return nil
+	}
 
 	var out formats.Writer
+
 	switch opts.format {
 	case formats.JSONString:
-		out = formats.JSONStructArray{Output: historyToGeneric(historyOutput)}
+		historyOutput := getHistoryJSONOutput(history, layers, imageID)
+		out = formats.JSONStructArray{Output: historyToGeneric([]historyTemplateParams{}, historyOutput)}
 	default:
-		out = formats.StdoutTemplateArray{Output: historyToGeneric(historyOutput), Template: historyFormat, Fields: historyOutput[0].headerMap()}
+		historyOutput := getHistoryTemplateOutput(history, layers, imageID, opts)
-
+		out = formats.StdoutTemplateArray{Output: historyToGeneric(historyOutput, []historyJSONParams{}), Template: opts.format, Fields: historyOutput[0].headerMap()}
 	}
-	formats.Writer(out).Out()
-	return nil
-}
 
-func (h *historyOutputParams) headerMap() map[string]string {
+	return formats.Writer(out).Out()
-	v := reflect.Indirect(reflect.ValueOf(h))
-	values := make(map[string]string)
-	for h := 0; h < v.NumField(); h++ {
-		key := v.Type().Field(h).Name
-		value := key
-		values[key] = fmt.Sprintf("%s        ", strings.ToUpper(splitCamelCase(value)))
-	}
-	return values
 }