summaryrefslogtreecommitdiff
path: root/libpod
diff options
context:
space:
mode:
authorMatthew Heon <matthew.heon@gmail.com>2018-02-07 15:05:17 -0500
committerAtomic Bot <atomic-devel@projectatomic.io>2018-02-09 15:01:34 +0000
commitcb28a1d284c8e9ec3539a11e472590686ccbd5f8 (patch)
tree917381beb09c040142213d6874ee49b5e08c7167 /libpod
parent4bc9a6d63370b4bd9cfb29e52f4f0dbb0070f856 (diff)
downloadpodman-cb28a1d284c8e9ec3539a11e472590686ccbd5f8.tar.gz
podman-cb28a1d284c8e9ec3539a11e472590686ccbd5f8.tar.bz2
podman-cb28a1d284c8e9ec3539a11e472590686ccbd5f8.zip
Fix bugs identified by unit tests
Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: #268 Approved by: rhatdan
Diffstat (limited to 'libpod')
-rw-r--r--libpod/in_memory_state.go69
-rw-r--r--libpod/pod.go1
-rw-r--r--libpod/sql_state.go31
-rw-r--r--libpod/sql_state_internal.go86
4 files changed, 163 insertions, 24 deletions
diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go
index c057f6804..4a2d5a311 100644
--- a/libpod/in_memory_state.go
+++ b/libpod/in_memory_state.go
@@ -123,6 +123,17 @@ func (s *InMemoryState) AddContainer(ctr *Container) error {
return errors.Wrapf(ErrInvalidArg, "cannot add a container that is in a pod with AddContainer, use AddContainerToPod")
}
+ // There are potential race conditions with this
+ // 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 {
+ _, ok = s.containers[depCtr]
+ if !ok {
+ return errors.Wrapf(ErrNoSuchCtr, "cannot depend on nonexistent container %s", depCtr)
+ }
+ }
+
if err := s.nameIndex.Reserve(ctr.Name(), ctr.ID()); err != nil {
return errors.Wrapf(err, "error registering container name %s", ctr.Name())
}
@@ -135,7 +146,6 @@ func (s *InMemoryState) AddContainer(ctr *Container) error {
s.containers[ctr.ID()] = ctr
// Add containers this container depends on
- depCtrs := ctr.Dependencies()
for _, depCtr := range depCtrs {
s.addCtrToDependsMap(ctr.ID(), depCtr)
}
@@ -157,6 +167,7 @@ func (s *InMemoryState) RemoveContainer(ctr *Container) error {
}
if _, ok := s.containers[ctr.ID()]; !ok {
+ ctr.valid = false
return errors.Wrapf(ErrNoSuchCtr, "no container exists in state with ID %s", ctr.ID())
}
@@ -301,6 +312,10 @@ func (s *InMemoryState) PodHasContainer(pod *Pod, ctrID string) (bool, error) {
return false, errors.Wrapf(ErrPodRemoved, "pod %s is not valid")
}
+ if ctrID == "" {
+ return false, ErrEmptyID
+ }
+
podCtrs, ok := s.podContainers[pod.ID()]
if !ok {
pod.valid = false
@@ -398,10 +413,12 @@ func (s *InMemoryState) RemovePod(pod *Pod) error {
// pods out of the state
if _, ok := s.pods[pod.ID()]; !ok {
+ pod.valid = false
return errors.Wrapf(ErrNoSuchPod, "no pod exists in state with ID %s", pod.ID())
}
podCtrs, ok := s.podContainers[pod.ID()]
if !ok {
+ pod.valid = false
return errors.Wrapf(ErrNoSuchPod, "no pod exists in state with ID %s", pod.ID())
}
if len(podCtrs) != 0 {
@@ -430,20 +447,19 @@ func (s *InMemoryState) RemovePodContainers(pod *Pod) error {
// Get pod containers
podCtrs, ok := s.podContainers[pod.ID()]
if !ok {
+ pod.valid = false
return errors.Wrapf(ErrNoSuchPod, "no pod exists in state with ID %s", pod.ID())
}
// Go through container dependencies. Check to see if any are outside the pod.
for ctr := range podCtrs {
ctrDeps, ok := s.ctrDepends[ctr]
- if !ok {
- return errors.Wrapf(ErrInternal, "cannot find dependencies for container %s", ctr)
- }
-
- for _, dep := range ctrDeps {
- _, ok := podCtrs[dep]
- if !ok {
- return errors.Wrapf(ErrCtrExists, "container %s has dependency %s outside of pod %s", ctr, dep, pod.ID())
+ if ok {
+ for _, dep := range ctrDeps {
+ _, ok := podCtrs[dep]
+ if !ok {
+ return errors.Wrapf(ErrCtrExists, "container %s has dependency %s outside of pod %s", ctr, dep, pod.ID())
+ }
}
}
}
@@ -490,6 +506,17 @@ func (s *InMemoryState) AddContainerToPod(pod *Pod, ctr *Container) error {
return errors.Wrapf(ErrCtrExists, "container with ID %s already exists in pod %s", ctr.ID(), pod.ID())
}
+ // There are potential race conditions with this
+ // 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 {
+ _, ok = s.containers[depCtr]
+ if !ok {
+ return errors.Wrapf(ErrNoSuchCtr, "cannot depend on nonexistent container %s", depCtr)
+ }
+ }
+
// Add container to state
_, ok = s.containers[ctr.ID()]
if ok {
@@ -510,6 +537,11 @@ func (s *InMemoryState) AddContainerToPod(pod *Pod, ctr *Container) error {
// Add container to pod containers
podCtrs[ctr.ID()] = ctr
+ // Add containers this container depends on
+ for _, depCtr := range depCtrs {
+ s.addCtrToDependsMap(ctr.ID(), depCtr)
+ }
+
return nil
}
@@ -523,6 +555,13 @@ func (s *InMemoryState) RemoveContainerFromPod(pod *Pod, ctr *Container) error {
return errors.Wrapf(ErrCtrRemoved, "container %s is not valid and cannot be removed from the pod", ctr.ID())
}
+ // 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)
+ }
+
// Retrieve pod containers
podCtrs, ok := s.podContainers[pod.ID()]
if !ok {
@@ -530,6 +569,12 @@ func (s *InMemoryState) RemoveContainerFromPod(pod *Pod, ctr *Container) error {
return errors.Wrapf(ErrPodRemoved, "pod %s has been removed", pod.ID())
}
+ // Does the container exist?
+ if _, ok := s.containers[ctr.ID()]; !ok {
+ ctr.valid = false
+ return errors.Wrapf(ErrNoSuchCtr, "container %s does not exist in state", ctr.ID())
+ }
+
// Is the container in the pod?
if _, ok := podCtrs[ctr.ID()]; !ok {
return errors.Wrapf(ErrNoSuchCtr, "container with ID %s not found in pod %s", ctr.ID(), pod.ID())
@@ -549,6 +594,12 @@ func (s *InMemoryState) RemoveContainerFromPod(pod *Pod, ctr *Container) error {
// Remove the container from the pod
delete(podCtrs, ctr.ID())
+ // Remove us from container dependencies
+ depCtrs := ctr.Dependencies()
+ for _, depCtr := range depCtrs {
+ s.removeCtrFromDependsMap(ctr.ID(), depCtr)
+ }
+
return nil
}
diff --git a/libpod/pod.go b/libpod/pod.go
index 4f5521ccb..ecef8554e 100644
--- a/libpod/pod.go
+++ b/libpod/pod.go
@@ -45,6 +45,7 @@ func newPod(lockDir string, runtime *Runtime) (*Pod, error) {
pod := new(Pod)
pod.id = stringid.GenerateNonCryptoID()
pod.name = namesgenerator.GetRandomName(0)
+ pod.labels = make(map[string]string)
pod.runtime = runtime
// Path our lock file will reside at
diff --git a/libpod/sql_state.go b/libpod/sql_state.go
index a5809fe58..ff04d6d63 100644
--- a/libpod/sql_state.go
+++ b/libpod/sql_state.go
@@ -14,7 +14,7 @@ import (
// DBSchema is the current DB schema version
// Increments every time a change is made to the database's tables
-const DBSchema = 9
+const DBSchema = 10
// SQLState is a state implementation backed by a persistent SQLite3 database
type SQLState struct {
@@ -252,7 +252,7 @@ func (s *SQLState) AddContainer(ctr *Container) (err error) {
return errors.Wrapf(ErrPodExists, "cannot add container that belongs to a pod, use AddContainerToPod instead")
}
- return s.addContainer(ctr)
+ return s.addContainer(ctr, nil)
}
// UpdateContainer updates a container's state from the database
@@ -381,7 +381,7 @@ func (s *SQLState) UpdateContainer(ctr *Container) error {
}
// SaveContainer updates a container's state in the database
-func (s *SQLState) SaveContainer(ctr *Container) error {
+func (s *SQLState) SaveContainer(ctr *Container) (err error) {
const update = `UPDATE containerState SET
State=?,
ConfigPath=?,
@@ -508,7 +508,7 @@ func (s *SQLState) RemoveContainer(ctr *Container) error {
return errors.Wrapf(ErrPodExists, "container %s belongs to a pod, use RemoveContainerFromPod", ctr.ID())
}
- return s.removeContainer(ctr)
+ return s.removeContainer(ctr, nil)
}
// AllContainers retrieves all the containers presently in the state
@@ -801,7 +801,7 @@ func (s *SQLState) AddPod(pod *Pod) (err error) {
// RemovePod removes a pod from the state
// Only empty pods can be removed
-func (s *SQLState) RemovePod(pod *Pod) error {
+func (s *SQLState) RemovePod(pod *Pod) (err error) {
const (
removePod = "DELETE FROM pods WHERE ID=?;"
removeRegistry = "DELETE FROM registry WHERE Id=?;"
@@ -832,6 +832,7 @@ func (s *SQLState) RemovePod(pod *Pod) error {
if err != nil {
return errors.Wrapf(err, "error retrieving number of rows in transaction removing pod %s", pod.ID())
} else if rows == 0 {
+ pod.valid = false
return ErrNoSuchPod
}
@@ -851,11 +852,11 @@ func (s *SQLState) RemovePod(pod *Pod) error {
// This can avoid issues with dependencies within the pod
// The operation will fail if any container in the pod has a dependency from
// outside the pod
-func (s *SQLState) RemovePodContainers(pod *Pod) error {
+func (s *SQLState) RemovePodContainers(pod *Pod) (err error) {
const (
getPodCtrs = "SELECT Id FROM containers WHERE pod=?;"
removeCtr = "DELETE FROM containers WHERE pod=?;"
- removeCtrState = "DELETE FROM containerState WHERE ID=ANY(SELECT Id FROM containers WHERE pod=?);"
+ removeCtrState = "DELETE FROM containerState WHERE ID IN (SELECT Id FROM containers WHERE pod=?);"
)
if !s.valid {
@@ -880,6 +881,16 @@ func (s *SQLState) RemovePodContainers(pod *Pod) error {
}
}()
+ // Check if the pod exists
+ exists, err := podExistsTx(pod.ID(), tx)
+ if err != nil {
+ return err
+ }
+ if !exists {
+ pod.valid = false
+ return ErrNoSuchPod
+ }
+
// First get all containers in the pod
rows, err := tx.Query(getPodCtrs, pod.ID())
if err != nil {
@@ -920,7 +931,7 @@ func (s *SQLState) RemovePodContainers(pod *Pod) error {
}
if err := tx.Commit(); err != nil {
- return errors.Wrapf(err, "error committing transaction to add pod %s", pod.ID())
+ return errors.Wrapf(err, "error committing transaction remove pod %s containers", pod.ID())
}
committed = true
@@ -963,7 +974,7 @@ func (s *SQLState) AddContainerToPod(pod *Pod, ctr *Container) error {
return errors.Wrapf(ErrInvalidArg, "container's pod ID does not match given pod's ID")
}
- return s.addContainer(ctr)
+ return s.addContainer(ctr, pod)
}
// RemoveContainerFromPod removes a container from the given pod
@@ -972,7 +983,7 @@ func (s *SQLState) RemoveContainerFromPod(pod *Pod, ctr *Container) error {
return errors.Wrapf(ErrInvalidArg, "container %s is not in pod %s", ctr.ID(), pod.ID())
}
- return s.removeContainer(ctr)
+ return s.removeContainer(ctr, pod)
}
// AllPods retrieves all pods presently in the state
diff --git a/libpod/sql_state_internal.go b/libpod/sql_state_internal.go
index f2b0d603c..5a1b68615 100644
--- a/libpod/sql_state_internal.go
+++ b/libpod/sql_state_internal.go
@@ -37,6 +37,7 @@ const (
FROM containers
INNER JOIN
containerState ON containers.Id = containerState.Id `
+ ExistsQuery = "SELECT 1 FROM pods WHERE Id=?;"
)
// Checks that the DB configuration matches the runtime's configuration
@@ -326,9 +327,7 @@ func prepareDB(db *sql.DB) (err error) {
// Check if given pod exists
// Internal-only version of hasPod
func (s *SQLState) podExists(id string) (bool, error) {
- const query = "SELECT 1 FROM pods WHERE Id=?;"
-
- row := s.db.QueryRow(query, id)
+ row := s.db.QueryRow(ExistsQuery, id)
var check int
err := row.Scan(&check)
@@ -343,7 +342,26 @@ func (s *SQLState) podExists(id string) (bool, error) {
}
return true, nil
+}
+
+// Check if given pod exists within a transaction
+// Transaction-based version of podExists
+func podExistsTx(id string, tx *sql.Tx) (bool, error) {
+ row := tx.QueryRow(ExistsQuery, id)
+
+ var check int
+ err := row.Scan(&check)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ return false, nil
+ }
+ return false, errors.Wrapf(err, "error questing database for existence of pod %s", id)
+ } else if check != 1 {
+ return false, errors.Wrapf(ErrInternal, "check digit for podExists query incorrect")
+ }
+
+ return true, nil
}
// Get filename for OCI spec on disk
@@ -717,7 +735,7 @@ func (s *SQLState) podFromScannable(row scannable) (*Pod, error) {
}
// Internal function for adding containers
-func (s *SQLState) addContainer(ctr *Container) (err error) {
+func (s *SQLState) addContainer(ctr *Container, pod *Pod) (err error) {
const (
addCtr = `INSERT INTO containers VALUES (
?, ?, ?, ?, ?,
@@ -800,6 +818,19 @@ func (s *SQLState) addContainer(ctr *Container) (err error) {
}
}()
+ // First check if pod exists, if one is given
+ if pod != nil {
+ exists, err := podExistsTx(pod.ID(), tx)
+ if err != nil {
+ return err
+ }
+
+ if !exists {
+ pod.valid = false
+ return errors.Wrapf(ErrNoSuchPod, "pod %s does not exist in state, cannot add container to it", pod.ID())
+ }
+ }
+
// Add container to registry
if _, err := tx.Exec(addRegistry, ctr.ID(), ctr.Name()); err != nil {
return errors.Wrapf(err, "error adding container %s to name/ID registry", ctr.ID())
@@ -905,11 +936,13 @@ func (s *SQLState) addContainer(ctr *Container) (err error) {
}
// Internal functions for removing containers
-func (s *SQLState) removeContainer(ctr *Container) error {
+func (s *SQLState) removeContainer(ctr *Container, pod *Pod) (err error) {
const (
removeCtr = "DELETE FROM containers WHERE Id=?;"
removeState = "DELETE FROM containerState WHERE Id=?;"
removeRegistry = "DELETE FROM registry WHERE Id=?;"
+ existsInPod = "SELECT 1 FROM containers WHERE Id=? AND Pod=?;"
+ ctrExists = "SELECT 1 FROM containers WHERE Id=?;"
)
if !s.valid {
@@ -930,6 +963,48 @@ func (s *SQLState) removeContainer(ctr *Container) error {
}
}()
+ if pod != nil {
+ // Check to see if the pod exists
+ exists, err := podExistsTx(pod.ID(), tx)
+ if err != nil {
+ return err
+ }
+
+ if !exists {
+ pod.valid = false
+ return errors.Wrapf(ErrNoSuchPod, "pod %s does not exist in state, cannot add container to it", pod.ID())
+ }
+
+ var check int
+
+ // Check to see if the container exists
+ row := tx.QueryRow(ctrExists, ctr.ID())
+ err = row.Scan(&check)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ ctr.valid = false
+ return errors.Wrapf(ErrNoSuchCtr, "container %s does not exist", ctr.ID())
+ }
+
+ return errors.Wrapf(err, "error querying database for container %s existence", ctr.ID())
+ } else if check != 1 {
+ return errors.Wrapf(ErrInternal, "check digit for ctr exists query incorrect")
+ }
+
+ // Check to see if the container is in the pod
+ row = tx.QueryRow(existsInPod, ctr.ID(), pod.ID())
+ err = row.Scan(&check)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ return errors.Wrapf(ErrNoSuchCtr, "container %s is not in pod %s", ctr.ID(), pod.ID())
+ }
+
+ return errors.Wrapf(err, "error querying database for container %s existence", ctr.ID())
+ } else if check != 1 {
+ return errors.Wrapf(ErrInternal, "check digit for ctr exists in pod query incorrect")
+ }
+ }
+
// Check rows acted on for the first transaction, verify we actually removed something
result, err := tx.Exec(removeCtr, ctr.ID())
if err != nil {
@@ -939,6 +1014,7 @@ func (s *SQLState) removeContainer(ctr *Container) error {
if err != nil {
return errors.Wrapf(err, "error retrieving number of rows in transaction removing container %s", ctr.ID())
} else if rows == 0 {
+ ctr.valid = false
return ErrNoSuchCtr
}