From 94daaf58d9957e2421c3274966f947bdc059641d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20M=C3=ADchal?= Date: Tue, 14 Jul 2020 20:17:40 +0200 Subject: [PATCH] cmd/list: Decode toolbox image/container JSONs to typed structs Every time Podman changes their JSON API Toolbox breaks horribly. That is caused by the combination of decoding the JSON purely by hand and by the complete lack of type assertions to make the process stable. I previously didn't know that unmarshalling with Go works on the 'do the best job it can' logic (thank you Owen!). This makes the need to check for subtle changes in the names of fields go away and type checking a bit more bareable (marking the questioned field as interface{} and then immediatelly type asserting). If even now an existing field does not hold an expected value the field will remain blank. To take this a bit further, I created two types (toolboxImage and toolboxContainer) that are used by both functions for returning the list of images and containers. Instead of using getters for properties that need some checks or adjustments this uses a custom unmarshaling function (Owen's idea; thanks!). --- src/cmd/list.go | 178 ++++++++++++++++++++++++++++++++++-------------- src/cmd/run.go | 2 +- 2 files changed, 129 insertions(+), 51 deletions(-) diff --git a/src/cmd/list.go b/src/cmd/list.go index 7188701b7..816fe2202 100644 --- a/src/cmd/list.go +++ b/src/cmd/list.go @@ -17,17 +17,34 @@ package cmd import ( + "encoding/json" "errors" "fmt" "os" "text/tabwriter" + "time" "github.com/containers/toolbox/pkg/podman" "github.com/containers/toolbox/pkg/utils" + "github.com/docker/go-units" "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) +type toolboxImage struct { + ID string + Name string + Created string +} + +type toolboxContainer struct { + ID string + Name string + Status string + Created string + Image string +} + var ( listFlags struct { onlyContainers bool @@ -82,8 +99,8 @@ func list(cmd *cobra.Command, args []string) error { lsImages = false } - var images []map[string]interface{} - var containers []map[string]interface{} + var images []toolboxImage + var containers []toolboxContainer var err error if lsImages { @@ -104,7 +121,7 @@ func list(cmd *cobra.Command, args []string) error { return nil } -func listContainers() ([]map[string]interface{}, error) { +func listContainers() ([]toolboxContainer, error) { logrus.Debug("Fetching containers with label=com.redhat.component=fedora-toolbox") args := []string{"--all", "--filter", "label=com.redhat.component=fedora-toolbox"} containers_old, err := podman.GetContainers(args...) @@ -128,7 +145,24 @@ func listContainers() ([]map[string]interface{}, error) { containers = utils.SortJSON(containers, "Names", false) } - return containers, nil + var toolboxContainers []toolboxContainer + for _, container := range containers { + var c toolboxContainer + containerJSON, err := json.Marshal(container) + if err != nil { + logrus.Errorf("failed to marshal container: %v", err) + continue + } + + err = c.UnmarshalJSON(containerJSON) + if err != nil { + logrus.Errorf("failed to unmarshal container: %v", err) + continue + } + toolboxContainers = append(toolboxContainers, c) + } + + return toolboxContainers, nil } func listHelp(cmd *cobra.Command, args []string) { @@ -152,7 +186,7 @@ func listHelp(cmd *cobra.Command, args []string) { } } -func listImages() ([]map[string]interface{}, error) { +func listImages() ([]toolboxImage, error) { logrus.Debug("Fetching images with label=com.redhat.component=fedora-toolbox") args := []string{"--filter", "label=com.redhat.component=fedora-toolbox"} images_old, err := podman.GetImages(args...) @@ -179,34 +213,36 @@ func listImages() ([]map[string]interface{}, error) { images = utils.SortJSON(images, "names", true) } - return images, nil + var toolboxImages []toolboxImage + for _, image := range images { + var i toolboxImage + imageJSON, err := json.Marshal(image) + if err != nil { + logrus.Errorf("failed to marshal toolbox image: %v", err) + continue + } + + err = i.UnmarshalJSON(imageJSON) + if err != nil { + logrus.Errorf("failed to unmarshal toolbox image: %v", err) + continue + } + toolboxImages = append(toolboxImages, i) + } + + return toolboxImages, nil } -func listOutput(images, containers []map[string]interface{}) { +func listOutput(images []toolboxImage, containers []toolboxContainer) { if len(images) != 0 { writer := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) fmt.Fprintf(writer, "%s\t%s\t%s\n", "IMAGE ID", "IMAGE NAME", "CREATED") - var idKey, nameKey, createdKey string - if podman.CheckVersion("2.0.0") { - idKey = "Id" - nameKey = "Names" - createdKey = "Created" - } else if podman.CheckVersion("1.8.3") { - idKey = "ID" - nameKey = "Names" - createdKey = "Created" - } else { - idKey = "id" - nameKey = "names" - createdKey = "created" - } - for _, image := range images { - id := utils.ShortID(image[idKey].(string)) - name := image[nameKey].([]interface{})[0].(string) - created := image[createdKey].(string) - fmt.Fprintf(writer, "%s\t%s\t%s\n", id, name, created) + fmt.Fprintf(writer, "%s\t%s\t%s\n", + utils.ShortID(image.ID), + image.Name, + image.Created) } writer.Flush() @@ -226,35 +262,77 @@ func listOutput(images, containers []map[string]interface{}) { "STATUS", "IMAGE NAME") - var idKey, createdKey, statusKey string - if podman.CheckVersion("2.0.0") { - idKey = "Id" - createdKey = "CreatedAt" - statusKey = "State" - } else { - idKey = "ID" - createdKey = "Created" - statusKey = "Status" + for _, container := range containers { + fmt.Fprintf(writer, "%s\t%s\t%s\t%s\t%s\n", + utils.ShortID(container.ID), + container.Name, + container.Created, + container.Status, + container.Image) } - for _, container := range containers { - id := utils.ShortID(container[idKey].(string)) + writer.Flush() + } +} - var nameString string - switch name := container["Names"].(type) { - case string: - nameString = name - case []interface{}: - nameString = name[0].(string) - } +func (i *toolboxImage) UnmarshalJSON(data []byte) error { + var raw struct { + ID string + Names []string + Created interface{} + } - created := container[createdKey].(string) - status := container[statusKey].(string) - imageName := container["Image"].(string) + if err := json.Unmarshal(data, &raw); err != nil { + return err + } - fmt.Fprintf(writer, "%s\t%s\t%s\t%s\t%s\n", id, nameString, created, status, imageName) - } + i.ID = raw.ID + i.Name = raw.Names[0] + switch value := raw.Created.(type) { + case string: + i.Created = value + case float64: + i.Created = units.HumanDuration(time.Since(time.Unix(int64(value), 0))) + " ago" + } - writer.Flush() + return nil +} + +func (c *toolboxContainer) UnmarshalJSON(data []byte) error { + var raw struct { + ID string + Name string + Names []string + Status string + State string + Created interface{} + Image string } + + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + c.ID = raw.ID + if raw.Name != "" { + c.Name = raw.Name + } else { + c.Name = raw.Names[0] + } + + if raw.Status != "" { + c.Status = raw.Status + } else { + c.Status = raw.State + } + + switch value := raw.Created.(type) { + case string: + c.Created = value + case float64: + c.Created = units.HumanDuration(time.Since(time.Unix(int64(value), 0))) + " ago" + } + c.Image = raw.Image + + return nil } diff --git a/src/cmd/run.go b/src/cmd/run.go index 7a1364665..7b9305117 100644 --- a/src/cmd/run.go +++ b/src/cmd/run.go @@ -194,7 +194,7 @@ func runCommand(container string, } else if containersCount == 1 && defaultContainer { fmt.Fprintf(os.Stderr, "Error: container %s not found\n", container) - container = containers[0]["Names"].(string) + container = containers[0].Name fmt.Fprintf(os.Stderr, "Entering container %s instead.\n", container) fmt.Fprintf(os.Stderr, "Use the 'create' command to create a different toolbox.\n") fmt.Fprintf(os.Stderr, "Run '%s --help' for usage.\n", executableBase)