Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Commit

Permalink
sandbox: change container slice to a map
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@bergwolf
bergwolf authored and Eric Ernst committed Aug 21, 2018
1 parent 585f9b8 commit fe4f360
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 42 deletions.
5 changes: 3 additions & 2 deletions virtcontainers/filesystem_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down
41 changes: 23 additions & 18 deletions virtcontainers/sandbox.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ type Sandbox struct {

volumes []Volume

containers []*Container
containers map[string]*Container

runPath string
configPath string
Expand Down Expand Up @@ -521,17 +521,19 @@ 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
}

// 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
}
}
Expand Down Expand Up @@ -741,6 +743,7 @@ func newSandbox(sandboxConfig SandboxConfig) (*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{},
Expand Down Expand Up @@ -794,8 +797,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
}
Expand Down Expand Up @@ -846,8 +849,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
}
}
Expand All @@ -867,15 +870,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.
Expand Down Expand Up @@ -954,7 +956,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
}
Expand Down Expand Up @@ -1066,8 +1071,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,
Expand Down
41 changes: 19 additions & 22 deletions virtcontainers/sandbox_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -634,8 +634,8 @@ func TestSandboxSetContainersStateFailingEmptySandboxID(t *testing.T) {
storage: &filesystem{},
}

containers := []*Container{
{
containers := map[string]*Container{
"100": {
id: "100",
sandbox: sandbox,
},
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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()
}
}
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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)
Expand Down

0 comments on commit fe4f360

Please sign in to comment.