diff options
author | Matthew Heon <matthew.heon@gmail.com> | 2018-02-09 17:13:07 -0500 |
---|---|---|
committer | Atomic Bot <atomic-devel@projectatomic.io> | 2018-02-12 14:28:07 +0000 |
commit | 4f225b47c9be6f9d72997cea83c029275c3530db (patch) | |
tree | 5b9092449ca27777657e7baf1a880bb1c313ed5b | |
parent | aa85ae212e4cc23df7a6bafe8992dc76770bac87 (diff) | |
download | podman-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.go | 41 | ||||
-rw-r--r-- | libpod/boltdb_state_internal.go | 2 | ||||
-rw-r--r-- | libpod/options.go | 6 | ||||
-rw-r--r-- | libpod/pod.go | 32 | ||||
-rw-r--r-- | libpod/sql_state.go | 2 | ||||
-rw-r--r-- | libpod/sql_state_internal.go | 7 | ||||
-rw-r--r-- | libpod/state_test.go | 72 | ||||
-rw-r--r-- | libpod/test_common.go | 31 |
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) -} |