From 85b943eb47bc7abe53b9f9e3d953566ed0f65e6c Mon Sep 17 00:00:00 2001
From: Lantao Liu <lantaol@google.com>
Date: Tue, 5 Dec 2017 18:17:58 +0000
Subject: [PATCH] Use one big info struct before we change info to an array.

Signed-off-by: Lantao Liu <lantaol@google.com>
---
 pkg/server/container_status.go      | 58 ++++++++++++++++++-----------
 pkg/server/container_status_test.go |  3 +-
 2 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/pkg/server/container_status.go b/pkg/server/container_status.go
index f913533c3..f5e96152b 100644
--- a/pkg/server/container_status.go
+++ b/pkg/server/container_status.go
@@ -20,6 +20,8 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"github.com/golang/glog"
+	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"golang.org/x/net/context"
 	"k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"
 
@@ -54,7 +56,10 @@ func (c *criContainerdService) ContainerStatus(ctx context.Context, r *runtime.C
 		imageRef = image.RepoDigests[0]
 	}
 	status := toCRIContainerStatus(container, spec, imageRef)
-	info := toCRIContainerInfo(ctx, container, r.GetVerbose())
+	info, err := toCRIContainerInfo(ctx, container, r.GetVerbose())
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate container info: %v", err)
+	}
 
 	return &runtime.ContainerStatusResponse{
 		Status: status,
@@ -94,44 +99,53 @@ func toCRIContainerStatus(container containerstore.Container, spec *runtime.Imag
 	}
 }
 
+type containerInfo struct {
+	SandboxID   string                   `json:"sandboxID"`
+	Pid         uint32                   `json:"pid"`
+	Removing    bool                     `json:"removing"`
+	SnapshotKey string                   `json:"snapshotKey"`
+	Snapshotter string                   `json:"snapshotter"`
+	Config      *runtime.ContainerConfig `json:"config"`
+	RuntimeSpec *runtimespec.Spec        `json:"runtimeSpec"`
+}
+
 // toCRIContainerInfo converts internal container object information to CRI container status response info map.
-func toCRIContainerInfo(ctx context.Context, container containerstore.Container, verbose bool) map[string]string {
+func toCRIContainerInfo(ctx context.Context, container containerstore.Container, verbose bool) (map[string]string, error) {
 	if !verbose {
-		return nil
+		return nil, nil
 	}
 
 	info := make(map[string]string)
 	meta := container.Metadata
 	status := container.Status.Get()
 
-	// TODO (mikebrow): discuss predefining constants for some or all of these key names in CRI for these info map values
-	info["pid"] = marshallToString(status.Pid)
-	info["containerConfig"] = marshallToString(meta.Config)
-	info["removingState"] = marshallToString(status.Removing)
+	// TODO(random-liu): Change CRI status info to use array instead of map.
+	ci := &containerInfo{
+		SandboxID: container.SandboxID,
+		Pid:       status.Pid,
+		Removing:  status.Removing,
+		Config:    meta.Config,
+	}
 
-	oldSpec, err := container.Container.Spec(ctx)
+	spec, err := container.Container.Spec(ctx)
 	if err == nil {
-		info["runtimeSpec"] = marshallToString(oldSpec)
+		ci.RuntimeSpec = spec
 	} else {
-		info["runtimeSpec"] = err.Error()
+		glog.Errorf("Failed to get container %q spec: %v", container.ID, err)
 	}
 
 	ctrInfo, err := container.Container.Info(ctx)
 	if err == nil {
-		info["snapshotKey"] = marshallToString(ctrInfo.SnapshotKey)
-		info["snapshotter"] = marshallToString(ctrInfo.Snapshotter)
+		ci.SnapshotKey = ctrInfo.SnapshotKey
+		ci.Snapshotter = ctrInfo.Snapshotter
 	} else {
-		info["snapshotKey"] = err.Error()
-		info["snapshotter"] = err.Error()
+		glog.Errorf("Failed to get container %q info: %v", container.ID, err)
 	}
 
-	return info
-}
-
-func marshallToString(v interface{}) string {
-	m, err := json.Marshal(v)
-	if err == nil {
-		return string(m)
+	infoBytes, err := json.Marshal(ci)
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal info %v: %v", ci, err)
 	}
-	return err.Error()
+	info["info"] = string(infoBytes)
+	return info, nil
 }
diff --git a/pkg/server/container_status_test.go b/pkg/server/container_status_test.go
index 1e1560ffd..d1cce81ff 100644
--- a/pkg/server/container_status_test.go
+++ b/pkg/server/container_status_test.go
@@ -153,9 +153,10 @@ func TestToCRIContainerInfo(t *testing.T) {
 	)
 	assert.NoError(t, err)
 
-	info := toCRIContainerInfo(context.Background(),
+	info, err := toCRIContainerInfo(context.Background(),
 		container,
 		false)
+	assert.NoError(t, err)
 	assert.Nil(t, info)
 }