From d0816dcc09d3a4249347651888239dad0ee65f53 Mon Sep 17 00:00:00 2001
From: Peng Tao <bergwolf@gmail.com>
Date: Mon, 23 Jul 2018 08:32:11 +0800
Subject: [PATCH] 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 | 5 ++--
virtcontainers/sandbox.go | 41 +++++++++++++++++--------------
virtcontainers/sandbox_test.go | 41 ++++++++++++++-----------------
3 files changed, 45 insertions(+), 42 deletions(-)
diff --git a/virtcontainers/filesystem_test.go b/virtcontainers/filesystem_test.go
index 7341b61ca5..7687acb917 100644
--- a/virtcontainers/filesystem_test.go
+++ b/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{
- {id: ""},
+ containers := map[string]*Container{
+ testContainerID: {},
}
sandbox := &Sandbox{
diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go
index 316dbd72cc..ca7908e3ed 100644
--- a/virtcontainers/sandbox.go
+++ b/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 {
- if c.id == containerID {
+ for id, c := range s.containers {
+ 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 {
- err = s.storage.storeContainerResource(s.id, container.id, configFileType, *(container.config))
+ for id, container := range s.containers {
+ 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 {
- if containerID == c.id {
+ for id, c := range s.containers {
+ if containerID == id {
return c, nil
}
}
@@ -870,15 +873,14 @@ func (s *Sandbox) removeContainer(containerID string) error {
return errNeedContainerID
}
- for idx, c := range s.containers {
- if containerID == c.id {
- s.containers = append(s.containers[:idx], s.containers[idx+1:]...)
- return nil
- }
+ if _, ok := s.containers[containerID]; !ok {
+ return fmt.Errorf("Could not remove the container %q from the sandbox %q containers list",
+ containerID, s.id)
}
- 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.
@@ -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 {
- if c.id == containerID {
+ for id, c := range s.containers {
+ if id == containerID {
return ContainerStatus{
ID: c.id,
State: c.state,
diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go
index d2f3517428..cc6049d1c2 100644
--- a/virtcontainers/sandbox_test.go
+++ b/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}
- containers = append(containers, &c)
+ c := &Container{id: id}
+ containers[id] = c
}
sandbox := Sandbox{
@@ -826,8 +826,8 @@ func TestGetAllContainers(t *testing.T) {
list := sandbox.GetAllContainers()
- for i, c := range list {
- if c.ID() != containerIDs[i] {
+ for _, c := range list {
+ 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{
- {
- id: testContainerID,
- },
+ containers: map[string]*Container{
+ 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{
- {
- id: testContainerID,
- },
+ containers: map[string]*Container{
+ testContainerID: {id: testContainerID},
},
}
err := sandbox.removeContainer(testContainerID)