bugfix: we should get a container's execids from inspect

Signed-off-by: Michael Wan <zirenwan@gmail.com>

apis/server/container_bridge.go

@@ -120,6 +120,7 @@ func (s *Server) getContainer(ctx context.Context, rw http.ResponseWriter, req *
 		Driver:          c.Driver,
 		MountLabel:      c.MountLabel,
 		ProcessLabel:    c.ProcessLabel,
+		ExecIds:         c.ExecIds,
 	}
 
 	return EncodeResponse(rw, http.StatusOK, container)

daemon/mgr/container.go

@@ -540,6 +540,24 @@ func (mgr *ContainerManager) Get(ctx context.Context, name string) (*Container,
 	if err != nil {
 		return nil, err
 	}
+	cID := c.Key()
+
+	// get all execids belongs to this container
+	fn := func(v interface{}) bool {
+		execConfig, ok := v.(*ContainerExecConfig)
+		if !ok || execConfig.ContainerID != cID {
+			return false
+		}
+
+		return true
+	}
+
+	var execIDs []string
+	execProcesses := mgr.ExecProcesses.Values(fn)
+	for k := range execProcesses {
+		execIDs = append(execIDs, k)
+	}
+	c.ExecIds = execIDs
 
 	return c, nil
 }
@@ -1914,7 +1932,7 @@ func (mgr *ContainerManager) setBaseFS(ctx context.Context, c *Container) {
 // execProcessGC cleans unused exec processes config every 5 minutes.
 func (mgr *ContainerManager) execProcessGC() {
 	for range time.Tick(time.Duration(GCExecProcessTick) * time.Minute) {
-		execProcesses := mgr.ExecProcesses.Values()
+		execProcesses := mgr.ExecProcesses.Values(nil)
 		cleaned := 0
 
 		for id, v := range execProcesses {

daemon/mgr/container_types.go

@@ -192,7 +192,7 @@ type Container struct {
 	Driver string `json:"Driver,omitempty"`
 
 	// exec ids
-	ExecIds string `json:"ExecIDs,omitempty"`
+	ExecIds []string `json:"ExecIDs,omitempty"`
 
 	// Snapshotter, GraphDriver is same, keep both
 	// just for compatibility

pkg/collect/safe_map.go

@@ -8,6 +8,9 @@ type SafeMap struct {
 	inner map[string]interface{}
 }
 
+// ValueFilter defines a function to filter values in map
+type ValueFilter func(interface{}) bool
+
 // NewSafeMap generates a instance of SafeMap type.
 func NewSafeMap() *SafeMap {
 	return &SafeMap{
@@ -26,13 +29,15 @@ func (m *SafeMap) Get(k string) *Value {
 }
 
 // Values returns all key-values stored in map
-func (m *SafeMap) Values() map[string]interface{} {
+func (m *SafeMap) Values(filter ValueFilter) map[string]interface{} {
 	m.RLock()
 	defer m.RUnlock()
 
 	nmap := make(map[string]interface{})
 	for k, v := range m.inner {
-		nmap[k] = v
+		if filter == nil || filter(v) {
+			nmap[k] = v
+		}
 	}
 
 	return nmap

pkg/collect/safe_map_test.go

@@ -3,9 +3,62 @@ package collect
 import (
 	"testing"
 
+	"github.com/alibaba/pouch/pkg/utils"
+
 	"github.com/stretchr/testify/assert"
 )
 
+// TestSafeMapValues is to valid the Values function
+func TestSafeMapValues(t *testing.T) {
+	safeMap := NewSafeMap()
+	assert.Equal(t, len(safeMap.inner), 0)
+
+	safeMap.Put("key", "value")
+	assert.Equal(t, len(safeMap.inner), 1)
+
+	safeMap.Put("key1", "value1")
+	assert.Equal(t, len(safeMap.inner), 2)
+
+	// first not specify filter
+	values := safeMap.Values(nil)
+	assert.Equal(t, len(values), 2)
+	for k, v := range values {
+		if !utils.StringInSlice([]string{"key", "key1"}, k) {
+			t.Errorf("got unexpected key: %v", k)
+		}
+
+		if k == "key" && v != "value" {
+			t.Errorf("expected 'value', but got %v", v)
+		}
+
+		if k == "key1" && v != "value1" {
+			t.Errorf("expected 'value1', but got %v", v)
+		}
+	}
+
+	// test filter function
+	fn := func(obj interface{}) bool {
+		v, ok := obj.(string)
+		if !ok || v != "value" {
+			return false
+		}
+
+		return true
+	}
+
+	values = safeMap.Values(fn)
+	assert.Equal(t, len(values), 1)
+	v, ok := values["key"]
+	if !ok {
+		t.Errorf("expected 'key' in map, but got: %v", values)
+	}
+
+	stringVar, ok := v.(string)
+	if !ok || stringVar != "value" {
+		t.Errorf("expected map's value is 'value', but got: %v", stringVar)
+	}
+}
+
 func TestSafeMapPutAndGet(t *testing.T) {
 	safeMap := NewSafeMap()
 	assert.Equal(t, len(safeMap.inner), 0)
@@ -67,7 +120,7 @@ func TestSafeMapDirectNew(t *testing.T) {
 	sm.Remove("k")
 
 	// test Values not panic
-	values := sm.Values()
+	values := sm.Values(nil)
 	assert.Equal(values, map[string]interface{}{})
 }
 

test/cli_inspect_test.go

@@ -243,3 +243,35 @@ func (suite *PouchInspectSuite) TestContainerInspectHostRootPath(c *check.C) {
 
 	c.Assert(strings.TrimSpace(hostRootPathOutput), check.Equals, containers[0].GraphDriver.Data["MergedDir"])
 }
+
+// TestContainerInspectExecIds is to valid if we can normally inspect ExecIds
+func (suite *PouchInspectSuite) TestContainerInspectExecIds(c *check.C) {
+	name := "TestContainerInspectExecIds"
+	command.PouchRun("run", "-d",
+		"--name", name,
+		busyboxImage, "top",
+	).Assert(c, icmd.Success)
+	defer DelContainerForceMultyTime(c, name)
+
+	// first check `ExecIds` when the container created
+	output := command.PouchRun("inspect", "-f", "{{.ExecIds}}", name).Stdout()
+	c.Assert(string(output), check.Equals, "[]\n")
+
+	// then check `ExecIds` when execute a exec command
+	command.PouchRun("exec", name, "echo", "hi").Assert(c, icmd.Success)
+	output = command.PouchRun("inspect", name).Stdout()
+	containers := make([]types.ContainerJSON, 1)
+	err := json.Unmarshal([]byte(output), &containers)
+	if err != nil || len(containers) != 1 {
+		c.Fatal("fail to format container json")
+	}
+
+	execIDs := containers[0].ExecIds
+	if len(execIDs) != 1 {
+		c.Errorf("expected one exec id, but got: %v", execIDs)
+	}
+
+	output = command.PouchRun("inspect", "-f", "{{.ExecIds}}", name).Stdout()
+	expected := fmt.Sprintf("[%v]\n", execIDs[0])
+	c.Assert(string(output), check.Equals, expected)
+}