summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatthew Heon <matthew.heon@gmail.com>2018-02-09 17:13:07 -0500
committerAtomic Bot <atomic-devel@projectatomic.io>2018-02-12 14:28:07 +0000
commit4f225b47c9be6f9d72997cea83c029275c3530db (patch)
tree5b9092449ca27777657e7baf1a880bb1c313ed5b
parentaa85ae212e4cc23df7a6bafe8992dc76770bac87 (diff)
downloadpodman-4f225b47c9be6f9d72997cea83c029275c3530db.tar.gz
podman-4f225b47c9be6f9d72997cea83c029275c3530db.tar.bz2
podman-4f225b47c9be6f9d72997cea83c029275c3530db.zip
Refactor Pod to use a Config struct
This allows us to JSON it and stuff it in the DB - previously, all pod fields were private, so JSON couldn't encode them. This allows us to keep all pod fields private by having a substruct with public fields. Signed-off-by: Matthew Heon <matthew.heon@gmail.com> Closes: #184 Approved by: baude
-rw-r--r--libpod/boltdb_state.go41
-rw-r--r--libpod/boltdb_state_internal.go2
-rw-r--r--libpod/options.go6
-rw-r--r--libpod/pod.go32
-rw-r--r--libpod/sql_state.go2
-rw-r--r--libpod/sql_state_internal.go7
-rw-r--r--libpod/state_test.go72
-rw-r--r--libpod/test_common.go31
8 files changed, 80 insertions, 113 deletions
diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go
index 96d987c82..2ca0319d0 100644
--- a/libpod/boltdb_state.go
+++ b/libpod/boltdb_state.go
@@ -466,6 +466,7 @@ func (s *BoltState) SaveContainer(ctr *Container) error {
ctrToSave := ctrBucket.Bucket(ctrID)
if ctrToSave == nil {
+ ctr.valid = false
return errors.Wrapf(ErrNoSuchCtr, "container %s does not exist in DB", ctr.ID())
}
@@ -612,6 +613,7 @@ func (s *BoltState) Pod(id string) (*Pod, error) {
podID := []byte(id)
pod := new(Pod)
+ pod.config = new(PodConfig)
db, err := s.getDBCon()
if err != nil {
@@ -645,6 +647,7 @@ func (s *BoltState) LookupPod(idOrName string) (*Pod, error) {
}
pod := new(Pod)
+ pod.config = new(PodConfig)
db, err := s.getDBCon()
if err != nil {
@@ -736,8 +739,8 @@ func (s *BoltState) HasPod(id string) (bool, error) {
return err
}
- podBytes := podBkt.Get(podID)
- if podBytes != nil {
+ podDB := podBkt.Bucket(podID)
+ if podDB != nil {
exists = true
}
@@ -913,6 +916,8 @@ func (s *BoltState) PodContainers(pod *Pod) ([]*Container, error) {
// Iterate through all containers in the pod
err = podCtrs.ForEach(func(id, val []byte) error {
newCtr := new(Container)
+ newCtr.config = new(ContainerConfig)
+ newCtr.state = new(containerState)
ctrs = append(ctrs, newCtr)
return s.getContainerFromDB(id, newCtr, ctrBkt)
@@ -943,7 +948,7 @@ func (s *BoltState) AddPod(pod *Pod) error {
podID := []byte(pod.ID())
podName := []byte(pod.Name())
- podJSON, err := json.Marshal(pod)
+ podJSON, err := json.Marshal(pod.config)
if err != nil {
return errors.Wrapf(err, "error marshalling pod %s JSON", pod.ID())
}
@@ -1143,14 +1148,7 @@ func (s *BoltState) RemovePodContainers(pod *Pod) error {
// Traverse all containers in the pod with a cursor
// for-each has issues with data mutation
- cursor := podCtrsBkt.Cursor()
- id, name := cursor.Last()
- for id != nil {
- // Remove the container from the bucker
- if err := podCtrsBkt.Delete(id); err != nil {
- return errors.Wrapf(err, "error removing container %s from pod %s", string(id), pod.ID())
- }
-
+ err = podCtrsBkt.ForEach(func(id, name []byte) error {
// Get the container so we can check dependencies
ctr := ctrBkt.Bucket(id)
if ctr == nil {
@@ -1165,15 +1163,10 @@ func (s *BoltState) RemovePodContainers(pod *Pod) error {
err = ctrDeps.ForEach(func(depID, name []byte) error {
exists := podCtrsBkt.Get(depID)
if exists == nil {
- // The dependency isn't in the
- // pod
- return errors.Wrapf(ErrCtrExists, "container %s has a dependency outside of pod %s", string(depID), pod.ID())
+ return errors.Wrapf(ErrCtrExists, "container %s has dependency %s outside of pod %s", string(id), string(depID), pod.ID())
}
return nil
})
- if err != nil {
- return err
- }
}
// Dependencies are set, we're clear to remove
@@ -1190,7 +1183,18 @@ func (s *BoltState) RemovePodContainers(pod *Pod) error {
return errors.Wrapf(err, "error deleting container %s name in DB", string(id))
}
- id, name = cursor.Last()
+ return nil
+ })
+ if err != nil {
+ return err
+ }
+
+ // Delete and recreate the bucket to empty it
+ if err := podDB.DeleteBucket(containersBkt); err != nil {
+ return errors.Wrapf(err, "error removing pod %s containers bucket", pod.ID())
+ }
+ if _, err := podDB.CreateBucket(containersBkt); err != nil {
+ return errors.Wrapf(err, "error recreating pod %s containers bucket", pod.ID())
}
return nil
@@ -1290,6 +1294,7 @@ func (s *BoltState) AllPods() ([]*Pod, error) {
}
pod := new(Pod)
+ pod.config = new(PodConfig)
pods = append(pods, pod)
diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go
index 89adac79d..b8b6b558d 100644
--- a/libpod/boltdb_state_internal.go
+++ b/libpod/boltdb_state_internal.go
@@ -216,7 +216,7 @@ func (s *BoltState) getPodFromDB(id []byte, pod *Pod, podBkt *bolt.Bucket) error
return errors.Wrapf(ErrInternal, "pod %s is missing configuration key in DB", string(id))
}
- if err := json.Unmarshal(podBytes, pod); err != nil {
+ if err := json.Unmarshal(podBytes, pod.config); err != nil {
return errors.Wrapf(err, "error unmarshalling pod %s from DB", string(id))
}
diff --git a/libpod/options.go b/libpod/options.go
index 965eb8cc5..f5df3349f 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -649,7 +649,7 @@ func WithPodName(name string) PodCreateOption {
return errors.Wrapf(ErrInvalidArg, "name must match regex [a-zA-Z0-9_-]+")
}
- pod.name = name
+ pod.config.Name = name
return nil
}
@@ -662,9 +662,9 @@ func WithPodLabels(labels map[string]string) PodCreateOption {
return ErrPodFinalized
}
- pod.labels = make(map[string]string)
+ pod.config.Labels = make(map[string]string)
for key, value := range labels {
- pod.labels[key] = value
+ pod.config.Labels[key] = value
}
return nil
diff --git a/libpod/pod.go b/libpod/pod.go
index 09e003608..d1e53f415 100644
--- a/libpod/pod.go
+++ b/libpod/pod.go
@@ -11,29 +11,34 @@ import (
// Pod represents a group of containers that may share namespaces
type Pod struct {
- id string `json:"id"`
- name string `json:"name"`
- labels map[string]string `json:"labels"`
+ config *PodConfig
- valid bool `json:"-"`
- runtime *Runtime `json:"-"`
- lock storage.Locker `json:"-"`
+ valid bool
+ runtime *Runtime
+ lock storage.Locker
+}
+
+// PodConfig represents a pod's static configuration
+type PodConfig struct {
+ ID string `json:"id"`
+ Name string `json:"name"`
+ Labels map[string]string `json:""`
}
// ID retrieves the pod's ID
func (p *Pod) ID() string {
- return p.id
+ return p.config.ID
}
// Name retrieves the pod's name
func (p *Pod) Name() string {
- return p.name
+ return p.config.Name
}
// Labels returns the pod's labels
func (p *Pod) Labels() map[string]string {
labels := make(map[string]string)
- for key, value := range p.labels {
+ for key, value := range p.config.Labels {
labels[key] = value
}
@@ -43,13 +48,14 @@ func (p *Pod) Labels() map[string]string {
// Creates a new, empty pod
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.config = new(PodConfig)
+ pod.config.ID = stringid.GenerateNonCryptoID()
+ pod.config.Name = namesgenerator.GetRandomName(0)
+ pod.config.Labels = make(map[string]string)
pod.runtime = runtime
// Path our lock file will reside at
- lockPath := filepath.Join(lockDir, pod.id)
+ lockPath := filepath.Join(lockDir, pod.config.ID)
// Grab a lockfile at the given path
lock, err := storage.GetLockfile(lockPath)
if err != nil {
diff --git a/libpod/sql_state.go b/libpod/sql_state.go
index d69a3b58e..a1830c468 100644
--- a/libpod/sql_state.go
+++ b/libpod/sql_state.go
@@ -764,7 +764,7 @@ func (s *SQLState) AddPod(pod *Pod) (err error) {
return ErrPodRemoved
}
- labelsJSON, err := json.Marshal(pod.labels)
+ labelsJSON, err := json.Marshal(pod.config.Labels)
if err != nil {
return errors.Wrapf(err, "error marshaling pod %s labels to JSON", pod.ID())
}
diff --git a/libpod/sql_state_internal.go b/libpod/sql_state_internal.go
index 5e18b7cfd..3fb1ac64f 100644
--- a/libpod/sql_state_internal.go
+++ b/libpod/sql_state_internal.go
@@ -709,8 +709,9 @@ func (s *SQLState) podFromScannable(row scannable) (*Pod, error) {
}
pod := new(Pod)
- pod.id = id
- pod.name = name
+ pod.config = new(PodConfig)
+ pod.config.ID = id
+ pod.config.Name = name
pod.runtime = s.runtime
// Decode labels JSON
@@ -718,7 +719,7 @@ func (s *SQLState) podFromScannable(row scannable) (*Pod, error) {
if err := json.Unmarshal([]byte(labelsJSON), &podLabels); err != nil {
return nil, errors.Wrapf(err, "error unmarshaling pod %s labels JSON", id)
}
- pod.labels = podLabels
+ pod.config.Labels = podLabels
// Retrieve pod lock
// Open and set the lockfile
diff --git a/libpod/state_test.go b/libpod/state_test.go
index 63e3bf173..33eb3ae13 100644
--- a/libpod/state_test.go
+++ b/libpod/state_test.go
@@ -848,11 +848,8 @@ func TestGetPodOnePod(t *testing.T) {
statePod, err := state.Pod(testPod.ID())
assert.NoError(t, err)
- // Use assert.EqualValues if the test fails to pretty print diff
- // between actual and expected
- if !testPodsEqual(testPod, statePod) {
- assert.EqualValues(t, testPod, statePod)
- }
+ assert.EqualValues(t, testPod.config, statePod.config)
+ assert.Equal(t, testPod.valid, statePod.valid)
})
}
@@ -873,11 +870,8 @@ func TestGetOnePodFromTwo(t *testing.T) {
statePod, err := state.Pod(testPod1.ID())
assert.NoError(t, err)
- // Use assert.EqualValues if the test fails to pretty print diff
- // between actual and expected
- if !testPodsEqual(testPod1, statePod) {
- assert.EqualValues(t, testPod1, statePod)
- }
+ assert.EqualValues(t, testPod1.config, statePod.config)
+ assert.Equal(t, testPod1.valid, statePod.valid)
})
}
@@ -938,11 +932,8 @@ func TestLookupPodFullID(t *testing.T) {
statePod, err := state.LookupPod(testPod.ID())
assert.NoError(t, err)
- // Use assert.EqualValues if the test fails to pretty print diff
- // between actual and expected
- if !testPodsEqual(testPod, statePod) {
- assert.EqualValues(t, testPod, statePod)
- }
+ assert.EqualValues(t, testPod.config, statePod.config)
+ assert.Equal(t, testPod.valid, statePod.valid)
})
}
@@ -957,11 +948,8 @@ func TestLookupPodUniquePartialID(t *testing.T) {
statePod, err := state.LookupPod(testPod.ID()[0:8])
assert.NoError(t, err)
- // Use assert.EqualValues if the test fails to pretty print diff
- // between actual and expected
- if !testPodsEqual(testPod, statePod) {
- assert.EqualValues(t, testPod, statePod)
- }
+ assert.EqualValues(t, testPod.config, statePod.config)
+ assert.Equal(t, testPod.valid, statePod.valid)
})
}
@@ -995,11 +983,8 @@ func TestLookupPodByName(t *testing.T) {
statePod, err := state.LookupPod(testPod.Name())
assert.NoError(t, err)
- // Use assert.EqualValues if the test fails to pretty print diff
- // between actual and expected
- if !testPodsEqual(testPod, statePod) {
- assert.EqualValues(t, testPod, statePod)
- }
+ assert.EqualValues(t, testPod.config, statePod.config)
+ assert.Equal(t, testPod.valid, statePod.valid)
})
}
@@ -1088,7 +1073,7 @@ func TestHasPodCtrIDFalse(t *testing.T) {
func TestAddPodInvalidPodErrors(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, lockPath string) {
- err := state.AddPod(&Pod{})
+ err := state.AddPod(&Pod{config: &PodConfig{},})
assert.Error(t, err)
})
}
@@ -1105,11 +1090,8 @@ func TestAddPodValidPodSucceeds(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, 1, len(allPods))
- // Use assert.EqualValues if the test fails to pretty print diff
- // between actual and expected
- if !testPodsEqual(testPod, allPods[0]) {
- assert.EqualValues(t, testPod, allPods[0])
- }
+ assert.EqualValues(t, testPod.config, allPods[0].config)
+ assert.Equal(t, testPod.valid, allPods[0].valid)
})
}
@@ -1215,7 +1197,7 @@ func TestAddPodCtrNameConflictFails(t *testing.T) {
func TestRemovePodInvalidPodErrors(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, lockPath string) {
- err := state.RemovePod(&Pod{})
+ err := state.RemovePod(&Pod{config: &PodConfig{},})
assert.Error(t, err)
})
}
@@ -1269,11 +1251,8 @@ func TestRemovePodFromPods(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, 1, len(allPods))
- // Use assert.EqualValues if the test fails to pretty print diff
- // between actual and expected
- if !testPodsEqual(testPod2, allPods[0]) {
- assert.EqualValues(t, testPod2, allPods[0])
- }
+ assert.EqualValues(t, testPod2.config, allPods[0].config)
+ assert.Equal(t, testPod2.valid, allPods[0].valid)
})
}
@@ -1348,11 +1327,8 @@ func TestAllPodsFindsPod(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, 1, len(allPods))
- // Use assert.EqualValues if the test fails to pretty print diff
- // between actual and expected
- if !testPodsEqual(testPod, allPods[0]) {
- assert.EqualValues(t, testPod, allPods[0])
- }
+ assert.EqualValues(t, testPod.config, allPods[0].config)
+ assert.Equal(t, testPod.valid, allPods[0].valid)
})
}
@@ -1396,7 +1372,7 @@ func TestAllPodsMultiplePods(t *testing.T) {
func TestPodHasContainerNoSuchPod(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, lockPath string) {
- _, err := state.PodHasContainer(&Pod{}, strings.Repeat("0", 32))
+ _, err := state.PodHasContainer(&Pod{config: &PodConfig{},}, strings.Repeat("0", 32))
assert.Error(t, err)
})
}
@@ -1472,7 +1448,7 @@ func TestPodHasContainerSucceeds(t *testing.T) {
func TestPodContainersByIDInvalidPod(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, lockPath string) {
- _, err := state.PodContainersByID(&Pod{})
+ _, err := state.PodContainersByID(&Pod{config: &PodConfig{},})
assert.Error(t, err)
})
}
@@ -1574,7 +1550,7 @@ func TestPodContainersByIDMultipleContainers(t *testing.T) {
func TestPodContainersInvalidPod(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, lockPath string) {
- _, err := state.PodContainers(&Pod{})
+ _, err := state.PodContainers(&Pod{config: &PodConfig{},})
assert.Error(t, err)
})
}
@@ -1681,7 +1657,7 @@ func TestPodContainersMultipleContainers(t *testing.T) {
func TestRemovePodContainersInvalidPod(t *testing.T) {
runForAllStates(t, func(t *testing.T, state State, lockPath string) {
- err := state.RemovePodContainers(&Pod{})
+ err := state.RemovePodContainers(&Pod{config: &PodConfig{},})
assert.Error(t, err)
})
}
@@ -1872,7 +1848,7 @@ func TestAddContainerToPodInvalidPod(t *testing.T) {
testCtr, err := getTestCtr1(lockPath)
assert.NoError(t, err)
- err = state.AddContainerToPod(&Pod{}, testCtr)
+ err = state.AddContainerToPod(&Pod{config: &PodConfig{},}, testCtr)
assert.Error(t, err)
})
}
@@ -2203,7 +2179,7 @@ func TestRemoveContainerFromPodBadPodFails(t *testing.T) {
testCtr, err := getTestCtr1(lockPath)
assert.NoError(t, err)
- err = state.RemoveContainerFromPod(&Pod{}, testCtr)
+ err = state.RemoveContainerFromPod(&Pod{config: &PodConfig{},}, testCtr)
assert.Error(t, err)
})
}
diff --git a/libpod/test_common.go b/libpod/test_common.go
index ad3020b7e..0d4199037 100644
--- a/libpod/test_common.go
+++ b/libpod/test_common.go
@@ -77,9 +77,11 @@ func getTestContainer(id, name, locksDir string) (*Container, error) {
// nolint
func getTestPod(id, name, locksDir string) (*Pod, error) {
pod := &Pod{
- id: id,
- name: name,
- labels: map[string]string{"a": "b", "c": "d"},
+ config: &PodConfig{
+ ID: id,
+ Name: name,
+ Labels: map[string]string{"a": "b", "c": "d"},
+ },
valid: true,
}
@@ -133,26 +135,3 @@ func testContainersEqual(a, b *Container) bool {
return reflect.DeepEqual(aStateJSON, bStateJSON)
}
-
-// This tests pod equality
-// We cannot guarantee equality in lockfile objects so we can't simply compare
-// nolint
-func testPodsEqual(a, b *Pod) bool {
- if a == nil && b == nil {
- return true
- } else if a == nil || b == nil {
- return false
- }
-
- if a.id != b.id {
- return false
- }
- if a.name != b.name {
- return false
- }
- if a.valid != b.valid {
- return false
- }
-
- return reflect.DeepEqual(a.labels, b.labels)
-}