sandbox: change container slice to a map

ContainerID is supposed to be unique within a sandbox. It is better to use
a map to describe containers of a sandbox.

Fixes: #502

Signed-off-by: Peng Tao <bergwolf@gmail.com>

virtcontainers/filesystem_test.go

@@ -34,6 +34,7 @@ func TestFilesystemCreateAllResourcesSuccessful(t *testing.T) {
 		storage:    fs,
 		config:     sandboxConfig,
 		devManager: manager.NewDeviceManager(manager.VirtioBlock),
+		containers: map[string]*Container{},
 	}
 
 	if err := sandbox.newContainers(); err != nil {
@@ -110,8 +111,8 @@ func TestFilesystemCreateAllResourcesFailingSandboxIDEmpty(t *testing.T) {
 func TestFilesystemCreateAllResourcesFailingContainerIDEmpty(t *testing.T) {
 	fs := &filesystem{}
 
-	containers := []*Container{
+	containers := map[string]*Container{
-		{id: ""},
+		testContainerID: {},
 	}
 
 	sandbox := &Sandbox{

virtcontainers/sandbox.go

@@ -453,7 +453,7 @@ type Sandbox struct {
 
 	volumes []Volume
 
-	containers []*Container
+	containers map[string]*Container
 
 	runPath    string
 	configPath string
@@ -522,8 +522,10 @@ func (s *Sandbox) GetAnnotations() map[string]string {
 func (s *Sandbox) GetAllContainers() []VCContainer {
 	ifa := make([]VCContainer, len(s.containers))
 
-	for i, v := range s.containers {
+	i := 0
+	for _, v := range s.containers {
 		ifa[i] = v
+		i++
 	}
 
 	return ifa
@@ -531,8 +533,8 @@ func (s *Sandbox) GetAllContainers() []VCContainer {
 
 // GetContainer returns the container named by the containerID.
 func (s *Sandbox) GetContainer(containerID string) VCContainer {
-	for _, c := range s.containers {
+	for id, c := range s.containers {
-		if c.id == containerID {
+		if id == containerID {
 			return c
 		}
 	}
@@ -743,6 +745,7 @@ func newSandbox(sandboxConfig SandboxConfig, factory Factory) (*Sandbox, error)
 		config:          &sandboxConfig,
 		devManager:      deviceManager.NewDeviceManager(sandboxConfig.HypervisorConfig.BlockDeviceDriver),
 		volumes:         sandboxConfig.Volumes,
+		containers:      map[string]*Container{},
 		runPath:         filepath.Join(runStoragePath, sandboxConfig.ID),
 		configPath:      filepath.Join(configStoragePath, sandboxConfig.ID),
 		state:           State{},
@@ -796,8 +799,8 @@ func (s *Sandbox) storeSandbox() error {
 		return err
 	}
 
-	for _, container := range s.containers {
+	for id, container := range s.containers {
-		err = s.storage.storeContainerResource(s.id, container.id, configFileType, *(container.config))
+		err = s.storage.storeContainerResource(s.id, id, configFileType, *(container.config))
 		if err != nil {
 			return err
 		}
@@ -849,8 +852,8 @@ func (s *Sandbox) findContainer(containerID string) (*Container, error) {
 		return nil, errNeedContainerID
 	}
 
-	for _, c := range s.containers {
+	for id, c := range s.containers {
-		if containerID == c.id {
+		if containerID == id {
 			return c, nil
 		}
 	}
@@ -870,17 +873,16 @@ func (s *Sandbox) removeContainer(containerID string) error {
 		return errNeedContainerID
 	}
 
-	for idx, c := range s.containers {
+	if _, ok := s.containers[containerID]; !ok {
-		if containerID == c.id {
-			s.containers = append(s.containers[:idx], s.containers[idx+1:]...)
-			return nil
-		}
-	}
-
 		return fmt.Errorf("Could not remove the container %q from the sandbox %q containers list",
 			containerID, s.id)
 	}
 
+	delete(s.containers, containerID)
+
+	return nil
+}
+
 // Delete deletes an already created sandbox.
 // The VM in which the sandbox is running will be shut down.
 func (s *Sandbox) Delete() error {
@@ -978,7 +980,10 @@ func (s *Sandbox) startVM() error {
 }
 
 func (s *Sandbox) addContainer(c *Container) error {
-	s.containers = append(s.containers, c)
+	if _, ok := s.containers[c.id]; ok {
+		return fmt.Errorf("Duplicated container: %s", c.id)
+	}
+	s.containers[c.id] = c
 
 	return nil
 }
@@ -1090,8 +1095,8 @@ func (s *Sandbox) StatusContainer(containerID string) (ContainerStatus, error) {
 		return ContainerStatus{}, errNeedContainerID
 	}
 
-	for _, c := range s.containers {
+	for id, c := range s.containers {
-		if c.id == containerID {
+		if id == containerID {
 			return ContainerStatus{
 				ID:          c.id,
 				State:       c.state,

virtcontainers/sandbox_test.go

@@ -634,8 +634,8 @@ func TestSandboxSetContainersStateFailingEmptySandboxID(t *testing.T) {
 		storage: &filesystem{},
 	}
 
-	containers := []*Container{
+	containers := map[string]*Container{
-		{
+		"100": {
 			id:      "100",
 			sandbox: sandbox,
 		},
@@ -787,11 +787,11 @@ func TestSandboxDeleteContainersStateFailingEmptySandboxID(t *testing.T) {
 
 func TestGetContainer(t *testing.T) {
 	containerIDs := []string{"abc", "123", "xyz", "rgb"}
-	containers := []*Container{}
+	containers := map[string]*Container{}
 
 	for _, id := range containerIDs {
 		c := Container{id: id}
-		containers = append(containers, &c)
+		containers[id] = &c
 	}
 
 	sandbox := Sandbox{
@@ -813,11 +813,11 @@ func TestGetContainer(t *testing.T) {
 
 func TestGetAllContainers(t *testing.T) {
 	containerIDs := []string{"abc", "123", "xyz", "rgb"}
-	containers := []*Container{}
+	containers := map[string]*Container{}
 
 	for _, id := range containerIDs {
-		c := Container{id: id}
+		c := &Container{id: id}
-		containers = append(containers, &c)
+		containers[id] = c
 	}
 
 	sandbox := Sandbox{
@@ -826,8 +826,8 @@ func TestGetAllContainers(t *testing.T) {
 
 	list := sandbox.GetAllContainers()
 
-	for i, c := range list {
+	for _, c := range list {
-		if c.ID() != containerIDs[i] {
+		if containers[c.ID()] == nil {
 			t.Fatal()
 		}
 	}
@@ -1168,19 +1168,20 @@ func TestSandboxAttachDevicesVFIO(t *testing.T) {
 		},
 	}
 
-	containers := []*Container{c}
+	containers := map[string]*Container{}
+	containers[c.id] = c
 
 	sandbox := Sandbox{
 		containers: containers,
 		hypervisor: &mockHypervisor{},
 	}
 
-	containers[0].sandbox = &sandbox
+	containers[c.id].sandbox = &sandbox
 
-	err = containers[0].attachDevices()
+	err = containers[c.id].attachDevices()
 	assert.Nil(t, err, "Error while attaching devices %s", err)
 
-	err = containers[0].detachDevices()
+	err = containers[c.id].detachDevices()
 	assert.Nil(t, err, "Error while detaching devices %s", err)
 }
 
@@ -1256,17 +1257,15 @@ func TestFindContainerNoContainerMatchFailure(t *testing.T) {
 
 func TestFindContainerSuccess(t *testing.T) {
 	sandbox := &Sandbox{
-		containers: []*Container{
+		containers: map[string]*Container{
-			{
+			testContainerID: {id: testContainerID},
-				id: testContainerID,
-			},
 		},
 	}
 	c, err := sandbox.findContainer(testContainerID)
 	assert.NotNil(t, c, "Container pointer should not be nil")
 	assert.Nil(t, err, "Should not have returned an error: %v", err)
 
-	assert.True(t, c == sandbox.containers[0], "Container pointers should point to the same address")
+	assert.True(t, c == sandbox.containers[testContainerID], "Container pointers should point to the same address")
 }
 
 func TestRemoveContainerSandboxNilFailure(t *testing.T) {
@@ -1285,10 +1284,8 @@ func TestRemoveContainerNoContainerMatchFailure(t *testing.T) {
 
 func TestRemoveContainerSuccess(t *testing.T) {
 	sandbox := &Sandbox{
-		containers: []*Container{
+		containers: map[string]*Container{
-			{
+			testContainerID: {id: testContainerID},
-				id: testContainerID,
-			},
 		},
 	}
 	err := sandbox.removeContainer(testContainerID)