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)