Prevent ctrs not in pods from depending on pod ctrs

Containers in pods cannot depend on containers outside of the
same pod. Make the reverse true as well - containers not in pods
cannot depend on containers in pods. This greatly simplifies our
dependency handling, as we can guarantee that removing a pod will
not encounter dependency issues.

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

Closes: #558
Approved by: rhatdan
This commit is contained in:
Matthew Heon
2018-03-27 15:00:16 -04:00
committed by Atomic Bot
parent 196c3ab3a5
commit 5b6f59e36c
3 changed files with 46 additions and 38 deletions

View File

@ -407,11 +407,16 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error {
return errors.Wrapf(ErrNoSuchCtr, "container %s depends on container %s, but it does not exist in the DB", ctr.ID(), dependsCtr)
}
// If we're part of a pod, make sure the dependency is part of the same pod
depCtrPod := depCtrBkt.Get(podIDKey)
if pod != nil {
depCtrPod := depCtrBkt.Get(podIDKey)
// If we're part of a pod, make sure the dependency is part of the same pod
if depCtrPod == nil {
return errors.Wrapf(ErrInvalidArg, "container %s depends on container%s which is not in pod %s", ctr.ID(), dependsCtr, pod.ID())
return errors.Wrapf(ErrInvalidArg, "container %s depends on container %s which is not in pod %s", ctr.ID(), dependsCtr, pod.ID())
}
} else {
// If we're not part of a pod, we cannot depend on containets in a pod
if depCtrPod != nil {
return errors.Wrapf(ErrInvalidArg, "container %s depends on container %s which is in a pod - containers not in pods cannot depend on containers in pods", ctr.ID(), dependsCtr)
}
}

View File

@ -126,9 +126,12 @@ func (s *InMemoryState) AddContainer(ctr *Container) error {
// But in-memory state is intended purely for testing and not production
// use, so this should be fine.
depCtrs := ctr.Dependencies()
for _, depCtr := range depCtrs {
if _, ok := s.containers[depCtr]; !ok {
return errors.Wrapf(ErrNoSuchCtr, "cannot depend on nonexistent container %s", depCtr)
for _, depID := range depCtrs {
depCtr, ok := s.containers[depID]
if !ok {
return errors.Wrapf(ErrNoSuchCtr, "cannot depend on nonexistent container %s", depID)
} else if depCtr.config.Pod != "" {
return errors.Wrapf(ErrInvalidArg, "cannot depend on container in a pod if not part of same pod")
}
}

View File

@ -273,6 +273,38 @@ func TestAddCtrInPodFails(t *testing.T) {
})
}
func TestAddCtrDepInPodFails(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, lockPath string) {
testPod, err := getTestPod1(lockPath)
assert.NoError(t, err)
testCtr1, err := getTestCtr2(lockPath)
assert.NoError(t, err)
testCtr1.config.Pod = testPod.ID()
testCtr2, err := getTestCtrN("3", lockPath)
assert.NoError(t, err)
testCtr2.config.UserNsCtr = testCtr1.ID()
err = state.AddPod(testPod)
assert.NoError(t, err)
err = state.AddContainerToPod(testPod, testCtr1)
assert.NoError(t, err)
err = state.AddContainer(testCtr2)
assert.Error(t, err)
ctrs, err := state.AllContainers()
assert.NoError(t, err)
assert.Equal(t, 1, len(ctrs))
testContainersEqual(t, testCtr1, ctrs[0])
})
}
func TestGetNonexistentContainerFails(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, lockPath string) {
_, err := state.Container("does not exist")
@ -1749,38 +1781,6 @@ func TestRemovePodContainerDependencyInPod(t *testing.T) {
})
}
func TestRemovePodContainerDependencyNotInPod(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, lockPath string) {
testPod, err := getTestPod1(lockPath)
assert.NoError(t, err)
testCtr1, err := getTestCtr2(lockPath)
assert.NoError(t, err)
testCtr1.config.Pod = testPod.ID()
testCtr2, err := getTestCtrN("3", lockPath)
assert.NoError(t, err)
testCtr2.config.IPCNsCtr = testCtr1.ID()
err = state.AddPod(testPod)
assert.NoError(t, err)
err = state.AddContainerToPod(testPod, testCtr1)
assert.NoError(t, err)
err = state.AddContainer(testCtr2)
assert.NoError(t, err)
err = state.RemovePodContainers(testPod)
t.Logf("Err %v", err)
assert.Error(t, err)
ctrs, err := state.PodContainersByID(testPod)
assert.NoError(t, err)
assert.Equal(t, 1, len(ctrs))
})
}
func TestAddContainerToPodInvalidPod(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, lockPath string) {
testCtr, err := getTestCtr1(lockPath)