From 20df2196f2158d8656d1b38580d816567843a5e0 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 12 Jan 2018 12:41:10 -0500 Subject: Add ability for states to track container dependencies Also prevent containers with dependencies from being removed from in memory states. SQLite already enforced this via FOREIGN KEY constraints. Signed-off-by: Matthew Heon Closes: #220 Approved by: rhatdan --- libpod/in_memory_state.go | 83 +++++++++++++++++++++++++++++++++ libpod/sql_state.go | 45 ++++++++++++++++++ libpod/sql_state_test.go | 114 ++++++++++++++++++++++++++++++++++++++++++++++ libpod/state.go | 7 +++ 4 files changed, 249 insertions(+) (limited to 'libpod') diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go index 4e4cbb664..b4e224d77 100644 --- a/libpod/in_memory_state.go +++ b/libpod/in_memory_state.go @@ -1,6 +1,8 @@ package libpod import ( + "strings" + "github.com/docker/docker/pkg/truncindex" "github.com/pkg/errors" "github.com/projectatomic/libpod/pkg/registrar" @@ -10,6 +12,7 @@ import ( type InMemoryState struct { pods map[string]*Pod containers map[string]*Container + ctrDepends map[string][]string podNameIndex *registrar.Registrar podIDIndex *truncindex.TruncIndex ctrNameIndex *registrar.Registrar @@ -137,6 +140,15 @@ func (s *InMemoryState) AddContainer(ctr *Container) error { s.containers[ctr.ID()] = ctr + // Add containers this container depends on + s.addCtrToDependsMap(ctr.ID(), ctr.config.IPCNsCtr) + s.addCtrToDependsMap(ctr.ID(), ctr.config.MountNsCtr) + s.addCtrToDependsMap(ctr.ID(), ctr.config.NetNsCtr) + s.addCtrToDependsMap(ctr.ID(), ctr.config.PIDNsCtr) + s.addCtrToDependsMap(ctr.ID(), ctr.config.UserNsCtr) + s.addCtrToDependsMap(ctr.ID(), ctr.config.UTSNsCtr) + s.addCtrToDependsMap(ctr.ID(), ctr.config.CgroupNsCtr) + return nil } @@ -146,6 +158,13 @@ func (s *InMemoryState) RemoveContainer(ctr *Container) error { // Almost no validity checks are performed, to ensure we can kick // misbehaving containers out of the state + // Ensure we don't remove a container which other containers depend on + deps, ok := s.ctrDepends[ctr.ID()] + if ok && len(deps) != 0 { + depsStr := strings.Join(deps, ", ") + return errors.Wrapf(ErrCtrExists, "the following containers depend on container %s: %s", ctr.ID(), depsStr) + } + if _, ok := s.containers[ctr.ID()]; !ok { return errors.Wrapf(ErrNoSuchCtr, "no container exists in state with ID %s", ctr.ID()) } @@ -156,6 +175,16 @@ func (s *InMemoryState) RemoveContainer(ctr *Container) error { delete(s.containers, ctr.ID()) s.ctrNameIndex.Release(ctr.Name()) + delete(s.ctrDepends, ctr.ID()) + + s.removeCtrFromDependsMap(ctr.ID(), ctr.config.IPCNsCtr) + s.removeCtrFromDependsMap(ctr.ID(), ctr.config.MountNsCtr) + s.removeCtrFromDependsMap(ctr.ID(), ctr.config.NetNsCtr) + s.removeCtrFromDependsMap(ctr.ID(), ctr.config.PIDNsCtr) + s.removeCtrFromDependsMap(ctr.ID(), ctr.config.UserNsCtr) + s.removeCtrFromDependsMap(ctr.ID(), ctr.config.UTSNsCtr) + s.removeCtrFromDependsMap(ctr.ID(), ctr.config.CgroupNsCtr) + return nil } @@ -174,6 +203,20 @@ func (s *InMemoryState) SaveContainer(ctr *Container) error { return nil } +// ContainerInUse checks if the given container is being used by other containers +func (s *InMemoryState) ContainerInUse(ctr *Container) ([]string, error) { + if !ctr.valid { + return nil, ErrCtrRemoved + } + + arr, ok := s.ctrDepends[ctr.ID()] + if !ok { + return []string{}, nil + } + + return arr, nil +} + // AllContainers retrieves all containers from the state func (s *InMemoryState) AllContainers() ([]*Container, error) { ctrs := make([]*Container, 0, len(s.containers)) @@ -298,3 +341,43 @@ func (s *InMemoryState) AllPods() ([]*Pod, error) { return pods, nil } + +// Internal Functions + +// Add a container to the dependency mappings +func (s *InMemoryState) addCtrToDependsMap(ctrID, dependsID string) { + if dependsID != "" { + arr, ok := s.ctrDepends[dependsID] + if !ok { + // Do not have a mapping for that container yet + s.ctrDepends[dependsID] = []string{ctrID} + } else { + // Have a mapping for the container + arr = append(arr, ctrID) + s.ctrDepends[dependsID] = arr + } + } +} + +// Remove a container from dependency mappings +func (s *InMemoryState) removeCtrFromDependsMap(ctrID, dependsID string) { + if dependsID != "" { + arr, ok := s.ctrDepends[dependsID] + if !ok { + // Internal state seems inconsistent + // But the dependency is definitely gone + // So just return + return + } + + newArr := make([]string, len(arr), 0) + + for _, id := range arr { + if id != ctrID { + newArr = append(newArr, id) + } + } + + s.ctrDepends[dependsID] = newArr + } +} diff --git a/libpod/sql_state.go b/libpod/sql_state.go index 51ec25510..4abce8576 100644 --- a/libpod/sql_state.go +++ b/libpod/sql_state.go @@ -661,6 +661,51 @@ func (s *SQLState) SaveContainer(ctr *Container) error { return nil } +// ContainerInUse checks if other containers depend on the given container +// It returns the IDs of containers which depend on the given container +func (s *SQLState) ContainerInUse(ctr *Container) ([]string, error) { + const inUseQuery = `SELECT Id FROM containers WHERE + IPCNsCtr=? OR + MountNsCtr=? OR + NetNsCtr=? OR + PIDNsCtr=? OR + UserNsCtr=? OR + UTSNsCtr=? OR + CgroupNsCtr=?;` + + if !s.valid { + return nil, ErrDBClosed + } + + if !ctr.valid { + return nil, ErrCtrRemoved + } + + id := ctr.ID() + + rows, err := s.db.Query(inUseQuery, id, id, id, id, id, id, id) + if err != nil { + return nil, errors.Wrapf(err, "error querying database for containers that depend on container %s", id) + } + defer rows.Close() + + ids := []string{} + + for rows.Next() { + var ctrID string + if err := rows.Scan(&ctrID); err != nil { + return nil, errors.Wrapf(err, "error scanning container IDs from db rows for container %s", id) + } + + ids = append(ids, ctrID) + } + if err := rows.Err(); err != nil { + return nil, errors.Wrapf(err, "error retrieving rows for container %s", id) + } + + return ids, nil +} + // RemoveContainer removes the container from the state func (s *SQLState) RemoveContainer(ctr *Container) error { const ( diff --git a/libpod/sql_state_test.go b/libpod/sql_state_test.go index 19d5da63e..0c08467f0 100644 --- a/libpod/sql_state_test.go +++ b/libpod/sql_state_test.go @@ -587,3 +587,117 @@ func TestGetAllContainersTwoContainers(t *testing.T) { assert.EqualValues(t, testCtr1, ctrs[1]) } } + +func TestContainerInUseInvalidContainer(t *testing.T) { + state, path, _, err := getEmptyState() + assert.NoError(t, err) + defer os.RemoveAll(path) + defer state.Close() + + _, err = state.ContainerInUse(&Container{}) + assert.Error(t, err) +} + +func TestContainerInUseOneContainer(t *testing.T) { + state, path, lockPath, err := getEmptyState() + assert.NoError(t, err) + defer os.RemoveAll(path) + defer state.Close() + + testCtr1, err := getTestContainer("11111111111111111111111111111111", "test1", lockPath) + assert.NoError(t, err) + testCtr2, err := getTestContainer("22222222222222222222222222222222", "test2", lockPath) + assert.NoError(t, err) + + testCtr2.config.UserNsCtr = testCtr1.config.ID + + err = state.AddContainer(testCtr1) + assert.NoError(t, err) + + err = state.AddContainer(testCtr2) + assert.NoError(t, err) + + ids, err := state.ContainerInUse(testCtr1) + assert.NoError(t, err) + assert.Equal(t, 1, len(ids)) + assert.Equal(t, testCtr2.config.ID, ids[0]) +} + +func TestContainerInUseTwoContainers(t *testing.T) { + state, path, lockPath, err := getEmptyState() + assert.NoError(t, err) + defer os.RemoveAll(path) + defer state.Close() + + testCtr1, err := getTestContainer("11111111111111111111111111111111", "test1", lockPath) + assert.NoError(t, err) + testCtr2, err := getTestContainer("22222222222222222222222222222222", "test2", lockPath) + assert.NoError(t, err) + testCtr3, err := getTestContainer("33333333333333333333333333333333", "test3", lockPath) + assert.NoError(t, err) + + testCtr2.config.UserNsCtr = testCtr1.config.ID + testCtr3.config.IPCNsCtr = testCtr1.config.ID + + err = state.AddContainer(testCtr1) + assert.NoError(t, err) + + err = state.AddContainer(testCtr2) + assert.NoError(t, err) + + err = state.AddContainer(testCtr3) + assert.NoError(t, err) + + ids, err := state.ContainerInUse(testCtr1) + assert.NoError(t, err) + assert.Equal(t, 2, len(ids)) +} + +func TestCannotRemoveContainerWithDependency(t *testing.T) { + state, path, lockPath, err := getEmptyState() + assert.NoError(t, err) + defer os.RemoveAll(path) + defer state.Close() + + testCtr1, err := getTestContainer("11111111111111111111111111111111", "test1", lockPath) + assert.NoError(t, err) + testCtr2, err := getTestContainer("22222222222222222222222222222222", "test2", lockPath) + assert.NoError(t, err) + + testCtr2.config.UserNsCtr = testCtr1.config.ID + + err = state.AddContainer(testCtr1) + assert.NoError(t, err) + + err = state.AddContainer(testCtr2) + assert.NoError(t, err) + + err = state.RemoveContainer(testCtr1) + assert.Error(t, err) +} + +func TestCanRemoveContainerAfterDependencyRemoved(t *testing.T) { + state, path, lockPath, err := getEmptyState() + assert.NoError(t, err) + defer os.RemoveAll(path) + defer state.Close() + + testCtr1, err := getTestContainer("11111111111111111111111111111111", "test1", lockPath) + assert.NoError(t, err) + testCtr2, err := getTestContainer("22222222222222222222222222222222", "test2", lockPath) + assert.NoError(t, err) + + testCtr2.config.UserNsCtr = testCtr1.config.ID + + err = state.AddContainer(testCtr1) + assert.NoError(t, err) + + err = state.AddContainer(testCtr2) + assert.NoError(t, err) + + err = state.RemoveContainer(testCtr2) + assert.NoError(t, err) + + err = state.RemoveContainer(testCtr1) + assert.NoError(t, err) +} diff --git a/libpod/state.go b/libpod/state.go index 4a79b8d2d..63865a541 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -28,6 +28,13 @@ type State interface { UpdateContainer(ctr *Container) error // SaveContainer saves a container's current state to the backing store SaveContainer(ctr *Container) error + // ContainerInUse checks if other containers depend upon a given + // container + // It returns a slice of the IDs of containers which depend on the given + // container. If the slice is empty, no container depend on the given + // container. + // A container cannot be removed if other containers depend on it + ContainerInUse(ctr *Container) ([]string, error) // Retrieves all containers presently in state AllContainers() ([]*Container, error) -- cgit v1.2.3-54-g00ecf