From 6132c4d5482a62358ee9166e2655cc876f3b4575 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 25 Jan 2023 09:21:14 +0100 Subject: [PATCH 1/2] ps: do not sync container Do not sync containers with the runtime and the database when listing containers. It turns out to be extremely expensive and unnecessary. The sync was needed since listing all containers from the database did not populate their state. Doing that, however, is much faster since we already have a connection to the database. This change makes listing 200 containers 2 times faster than before. [NO NEW TESTS NEEDED] Signed-off-by: Valentin Rothberg --- libpod/boltdb_state.go | 36 ++------------------------------- libpod/boltdb_state_internal.go | 32 +++++++++++++++++++++++++++++ libpod/container_api.go | 4 ---- 3 files changed, 34 insertions(+), 38 deletions(-) diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index de9e88c737..d5976f540c 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -815,8 +815,6 @@ func (s *BoltState) UpdateContainer(ctr *Container) error { return fmt.Errorf("container %s is in namespace %q, does not match our namespace %q: %w", ctr.ID(), ctr.config.Namespace, s.namespace, define.ErrNSMismatch) } - newState := new(ContainerState) - ctrID := []byte(ctr.ID()) db, err := s.getDBCon() @@ -825,43 +823,13 @@ func (s *BoltState) UpdateContainer(ctr *Container) error { } defer s.deferredCloseDBCon(db) - err = db.View(func(tx *bolt.Tx) error { + return db.View(func(tx *bolt.Tx) error { ctrBucket, err := getCtrBucket(tx) if err != nil { return err } - - ctrToUpdate := ctrBucket.Bucket(ctrID) - if ctrToUpdate == nil { - ctr.valid = false - return fmt.Errorf("container %s does not exist in database: %w", ctr.ID(), define.ErrNoSuchCtr) - } - - newStateBytes := ctrToUpdate.Get(stateKey) - if newStateBytes == nil { - return fmt.Errorf("container %s does not have a state key in DB: %w", ctr.ID(), define.ErrInternal) - } - - if err := json.Unmarshal(newStateBytes, newState); err != nil { - return fmt.Errorf("unmarshalling container %s state: %w", ctr.ID(), err) - } - - // backwards compat, previously we used a extra bucket for the netns so try to get it from there - netNSBytes := ctrToUpdate.Get(netNSKey) - if netNSBytes != nil && newState.NetNS == "" { - newState.NetNS = string(netNSBytes) - } - - return nil + return s.getContainerStateDB(ctrID, ctr, ctrBucket) }) - if err != nil { - return err - } - - // New state compiled successfully, swap it into the current state - ctr.state = newState - - return nil } // SaveContainer saves a container's current state in the database diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index 48aa0acc49..25f40835bc 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -428,11 +428,43 @@ func (s *BoltState) getContainerConfigFromDB(id []byte, config *ContainerConfig, return nil } +func (s *BoltState) getContainerStateDB(id []byte, ctr *Container, ctrsBkt *bolt.Bucket) error { + newState := new(ContainerState) + ctrToUpdate := ctrsBkt.Bucket(id) + if ctrToUpdate == nil { + ctr.valid = false + return fmt.Errorf("container %s does not exist in database: %w", ctr.ID(), define.ErrNoSuchCtr) + } + + newStateBytes := ctrToUpdate.Get(stateKey) + if newStateBytes == nil { + return fmt.Errorf("container %s does not have a state key in DB: %w", ctr.ID(), define.ErrInternal) + } + + if err := json.Unmarshal(newStateBytes, newState); err != nil { + return fmt.Errorf("unmarshalling container %s state: %w", ctr.ID(), err) + } + + // backwards compat, previously we used a extra bucket for the netns so try to get it from there + netNSBytes := ctrToUpdate.Get(netNSKey) + if netNSBytes != nil && newState.NetNS == "" { + newState.NetNS = string(netNSBytes) + } + + // New state compiled successfully, swap it into the current state + ctr.state = newState + return nil +} + func (s *BoltState) getContainerFromDB(id []byte, ctr *Container, ctrsBkt *bolt.Bucket) error { if err := s.getContainerConfigFromDB(id, ctr.config, ctrsBkt); err != nil { return err } + if err := s.getContainerStateDB(id, ctr, ctrsBkt); err != nil { + return err + } + // Get the lock lock, err := s.runtime.lockManager.RetrieveLock(ctr.config.LockID) if err != nil { diff --git a/libpod/container_api.go b/libpod/container_api.go index a2aac96e7a..ab7985629c 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -829,10 +829,6 @@ func (c *Container) Batch(batchFunc func(*Container) error) error { c.lock.Lock() defer c.lock.Unlock() - if err := c.syncContainer(); err != nil { - return err - } - newCtr := new(Container) newCtr.config = c.config newCtr.state = c.state From c74bdae3515000d816933389b8c8202564fb115e Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 26 Jan 2023 10:20:38 +0100 Subject: [PATCH 2/2] DB: make loading container states optional Loading container states speed things up when listing all containers but it comes with a price tag for many other call paths. Hence, make loading the state conditional to allow for keeping `podman ps` fast without other commands regressing in performance. [NO NEW TESTS NEEDED] Signed-off-by: Valentin Rothberg --- libpod/boltdb_state.go | 11 ++-- libpod/boltdb_state_internal.go | 8 ++- libpod/networking_linux.go | 2 +- libpod/runtime.go | 4 +- libpod/runtime_ctr.go | 13 ++-- libpod/runtime_img.go | 2 +- libpod/runtime_migrate.go | 2 +- libpod/runtime_renumber.go | 2 +- libpod/state.go | 3 +- libpod/state_test.go | 92 +++++++++++++-------------- pkg/api/handlers/compat/containers.go | 2 +- pkg/domain/infra/abi/containers.go | 4 +- pkg/ps/ps.go | 9 ++- 13 files changed, 83 insertions(+), 71 deletions(-) diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index d5976f540c..a31f34bebb 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -598,7 +598,7 @@ func (s *BoltState) Container(id string) (*Container, error) { return err } - return s.getContainerFromDB(ctrID, ctr, ctrBucket) + return s.getContainerFromDB(ctrID, ctr, ctrBucket, false) }) if err != nil { return nil, err @@ -703,7 +703,7 @@ func (s *BoltState) LookupContainer(idOrName string) (*Container, error) { return err } - return s.getContainerFromDB(id, ctr, ctrBucket) + return s.getContainerFromDB(id, ctr, ctrBucket, false) }) if err != nil { return nil, err @@ -950,7 +950,8 @@ func (s *BoltState) ContainerInUse(ctr *Container) ([]string, error) { } // AllContainers retrieves all the containers in the database -func (s *BoltState) AllContainers() ([]*Container, error) { +// If `loadState` is set, the containers' state will be loaded as well. +func (s *BoltState) AllContainers(loadState bool) ([]*Container, error) { if !s.valid { return nil, define.ErrDBClosed } @@ -987,7 +988,7 @@ func (s *BoltState) AllContainers() ([]*Container, error) { ctr.config = new(ContainerConfig) ctr.state = new(ContainerState) - if err := s.getContainerFromDB(id, ctr, ctrBucket); err != nil { + if err := s.getContainerFromDB(id, ctr, ctrBucket, loadState); err != nil { // If the error is a namespace mismatch, we can // ignore it safely. // We just won't include the container in the @@ -2463,7 +2464,7 @@ func (s *BoltState) PodContainers(pod *Pod) ([]*Container, error) { newCtr.state = new(ContainerState) ctrs = append(ctrs, newCtr) - return s.getContainerFromDB(id, newCtr, ctrBkt) + return s.getContainerFromDB(id, newCtr, ctrBkt, false) }) if err != nil { return err diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index 25f40835bc..044ea2f45e 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -456,13 +456,15 @@ func (s *BoltState) getContainerStateDB(id []byte, ctr *Container, ctrsBkt *bolt return nil } -func (s *BoltState) getContainerFromDB(id []byte, ctr *Container, ctrsBkt *bolt.Bucket) error { +func (s *BoltState) getContainerFromDB(id []byte, ctr *Container, ctrsBkt *bolt.Bucket, loadState bool) error { if err := s.getContainerConfigFromDB(id, ctr.config, ctrsBkt); err != nil { return err } - if err := s.getContainerStateDB(id, ctr, ctrsBkt); err != nil { - return err + if loadState { + if err := s.getContainerStateDB(id, ctr, ctrsBkt); err != nil { + return err + } } // Get the lock diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index c1381aa046..31c66246bc 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -283,7 +283,7 @@ func (r *RootlessNetNS) Cleanup(runtime *Runtime) error { // only if the netns is empty we know that we do not need cleanup return c.state.NetNS != "" } - ctrs, err := runtime.GetContainers(activeNetns) + ctrs, err := runtime.GetContainers(false, activeNetns) if err != nil { return err } diff --git a/libpod/runtime.go b/libpod/runtime.go index df5bf0ef49..ece290f797 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -777,7 +777,7 @@ func (r *Runtime) Shutdown(force bool) error { // Shutdown all containers if --force is given if force { - ctrs, err := r.state.AllContainers() + ctrs, err := r.state.AllContainers(false) if err != nil { logrus.Errorf("Retrieving containers from database: %v", err) } else { @@ -833,7 +833,7 @@ func (r *Runtime) refresh(alivePath string) error { // Next refresh the state of all containers to recreate dirs and // namespaces, and all the pods to recreate cgroups. // Containers, pods, and volumes must also reacquire their locks. - ctrs, err := r.state.AllContainers() + ctrs, err := r.state.AllContainers(false) if err != nil { return fmt.Errorf("retrieving all containers from state: %w", err) } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 15efdf07be..33e31c63e3 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -1116,16 +1116,17 @@ func (r *Runtime) LookupContainerID(idOrName string) (string, error) { return r.state.LookupContainerID(idOrName) } -// GetContainers retrieves all containers from the state +// GetContainers retrieves all containers from the state. +// If `loadState` is set, the containers' state will be loaded as well. // Filters can be provided which will determine what containers are included in // the output. Multiple filters are handled by ANDing their output, so only // containers matching all filters are returned -func (r *Runtime) GetContainers(filters ...ContainerFilter) ([]*Container, error) { +func (r *Runtime) GetContainers(loadState bool, filters ...ContainerFilter) ([]*Container, error) { if !r.valid { return nil, define.ErrRuntimeStopped } - ctrs, err := r.GetAllContainers() + ctrs, err := r.state.AllContainers(loadState) if err != nil { return nil, err } @@ -1148,7 +1149,7 @@ func (r *Runtime) GetContainers(filters ...ContainerFilter) ([]*Container, error // GetAllContainers is a helper function for GetContainers func (r *Runtime) GetAllContainers() ([]*Container, error) { - return r.state.AllContainers() + return r.state.AllContainers(false) } // GetRunningContainers is a helper function for GetContainers @@ -1157,7 +1158,7 @@ func (r *Runtime) GetRunningContainers() ([]*Container, error) { state, _ := c.State() return state == define.ContainerStateRunning } - return r.GetContainers(running) + return r.GetContainers(false, running) } // GetContainersByList is a helper function for GetContainers @@ -1231,7 +1232,7 @@ func (r *Runtime) PruneContainers(filterFuncs []ContainerFilter) ([]*reports.Pru return false } filterFuncs = append(filterFuncs, containerStateFilter) - delContainers, err := r.GetContainers(filterFuncs...) + delContainers, err := r.GetContainers(false, filterFuncs...) if err != nil { return nil, err } diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go index d8e88ca503..fc1fc62f3d 100644 --- a/libpod/runtime_img.go +++ b/libpod/runtime_img.go @@ -28,7 +28,7 @@ func (r *Runtime) RemoveContainersForImageCallback(ctx context.Context) libimage if !r.valid { return define.ErrRuntimeStopped } - ctrs, err := r.state.AllContainers() + ctrs, err := r.state.AllContainers(false) if err != nil { return err } diff --git a/libpod/runtime_migrate.go b/libpod/runtime_migrate.go index df1a1f1cbc..f279eaf9d1 100644 --- a/libpod/runtime_migrate.go +++ b/libpod/runtime_migrate.go @@ -49,7 +49,7 @@ func (r *Runtime) migrate() error { return err } - allCtrs, err := r.state.AllContainers() + allCtrs, err := r.state.AllContainers(false) if err != nil { return err } diff --git a/libpod/runtime_renumber.go b/libpod/runtime_renumber.go index ff70081d8a..f75e0e3e04 100644 --- a/libpod/runtime_renumber.go +++ b/libpod/runtime_renumber.go @@ -20,7 +20,7 @@ func (r *Runtime) renumberLocks() error { return err } - allCtrs, err := r.state.AllContainers() + allCtrs, err := r.state.AllContainers(false) if err != nil { return err } diff --git a/libpod/state.go b/libpod/state.go index 6548a6828a..0499525ec1 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -96,9 +96,10 @@ type State interface { //nolint:interfacebloat // The container being checked must be part of the set namespace. ContainerInUse(ctr *Container) ([]string, error) // Retrieves all containers presently in state. + // If `loadState` is set, the containers' state will be loaded as well. // If a namespace is set, only containers within the namespace will be // returned. - AllContainers() ([]*Container, error) + AllContainers(loadState bool) ([]*Container, error) // Get networks the container is currently connected to. GetNetworks(ctr *Container) (map[string]types.PerNetworkOptions, error) diff --git a/libpod/state_test.go b/libpod/state_test.go index 7664f7c007..da891a4a5b 100644 --- a/libpod/state_test.go +++ b/libpod/state_test.go @@ -150,7 +150,7 @@ func TestAddDuplicateCtrIDFails(t *testing.T) { err = state.AddContainer(testCtr2) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(ctrs)) }) @@ -169,7 +169,7 @@ func TestAddDuplicateCtrNameFails(t *testing.T) { err = state.AddContainer(testCtr2) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(ctrs)) }) @@ -188,7 +188,7 @@ func TestAddCtrPodDupIDFails(t *testing.T) { err = state.AddContainer(testCtr) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -207,7 +207,7 @@ func TestAddCtrPodDupNameFails(t *testing.T) { err = state.AddContainer(testCtr) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -229,7 +229,7 @@ func TestAddCtrInPodFails(t *testing.T) { err = state.AddContainer(testCtr) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -259,7 +259,7 @@ func TestAddCtrDepInPodFails(t *testing.T) { err = state.AddContainer(testCtr2) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) require.Len(t, ctrs, 1) @@ -285,7 +285,7 @@ func TestAddCtrDepInSameNamespaceSucceeds(t *testing.T) { err = state.AddContainer(testCtr2) assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 2, len(ctrs)) }) @@ -309,7 +309,7 @@ func TestAddCtrDepInDifferentNamespaceFails(t *testing.T) { err = state.AddContainer(testCtr2) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(ctrs)) @@ -353,7 +353,7 @@ func TestAddCtrDifferentNamespaceFails(t *testing.T) { err = state.SetNamespace("") assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -843,14 +843,14 @@ func TestRemoveContainer(t *testing.T) { err = state.AddContainer(testCtr) assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(ctrs)) err = state.RemoveContainer(testCtr) assert.NoError(t, err) - ctrs2, err := state.AllContainers() + ctrs2, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs2)) }) @@ -877,7 +877,7 @@ func TestRemoveContainerNotInNamespaceFails(t *testing.T) { err = state.AddContainer(testCtr) assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(ctrs)) @@ -890,7 +890,7 @@ func TestRemoveContainerNotInNamespaceFails(t *testing.T) { err = state.SetNamespace("") assert.NoError(t, err) - ctrs2, err := state.AllContainers() + ctrs2, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(ctrs2)) }) @@ -898,7 +898,7 @@ func TestRemoveContainerNotInNamespaceFails(t *testing.T) { func TestGetAllContainersOnNewStateIsEmpty(t *testing.T) { runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) { - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -912,7 +912,7 @@ func TestGetAllContainersWithOneContainer(t *testing.T) { err = state.AddContainer(testCtr) assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) require.Len(t, ctrs, 1) @@ -933,7 +933,7 @@ func TestGetAllContainersTwoContainers(t *testing.T) { err = state.AddContainer(testCtr2) assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 2, len(ctrs)) }) @@ -952,7 +952,7 @@ func TestGetAllContainersNoContainerInNamespace(t *testing.T) { err = state.SetNamespace("test2") assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -977,7 +977,7 @@ func TestGetContainerOneContainerInNamespace(t *testing.T) { err = state.SetNamespace("test1") assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(ctrs)) @@ -1186,7 +1186,7 @@ func TestCannotRemoveContainerWithDependency(t *testing.T) { err = state.RemoveContainer(testCtr1) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 2, len(ctrs)) }) @@ -1210,7 +1210,7 @@ func TestCannotRemoveContainerWithGenericDependency(t *testing.T) { err = state.RemoveContainer(testCtr1) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 2, len(ctrs)) }) @@ -1237,7 +1237,7 @@ func TestCanRemoveContainerAfterDependencyRemoved(t *testing.T) { err = state.RemoveContainer(testCtr1) assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -1265,7 +1265,7 @@ func TestCanRemoveContainerAfterDependencyRemovedDuplicate(t *testing.T) { err = state.RemoveContainer(testCtr1) assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -1287,7 +1287,7 @@ func TestCannotUsePodAsDependency(t *testing.T) { err = state.AddContainer(testCtr) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -1317,7 +1317,7 @@ func TestCannotUseBadIDAsDependency(t *testing.T) { err = state.AddContainer(testCtr) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -1333,7 +1333,7 @@ func TestCannotUseBadIDAsGenericDependency(t *testing.T) { err = state.AddContainer(testCtr) assert.Error(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -2630,7 +2630,7 @@ func TestRemovePodContainersPreservesCtrOutsidePod(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(allCtrs)) }) @@ -2778,7 +2778,7 @@ func TestAddContainerToPodSucceeds(t *testing.T) { assert.NoError(t, err) require.Len(t, ctrs, 1) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) require.Len(t, allCtrs, 1) @@ -2813,7 +2813,7 @@ func TestAddContainerToPodTwoContainers(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 2, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 2, len(allCtrs)) }) @@ -2844,7 +2844,7 @@ func TestAddContainerToPodWithAddContainer(t *testing.T) { assert.NoError(t, err) require.Len(t, ctrs, 1) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 2, len(allCtrs)) @@ -2877,7 +2877,7 @@ func TestAddContainerToPodCtrIDConflict(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(allCtrs)) }) @@ -2908,7 +2908,7 @@ func TestAddContainerToPodCtrNameConflict(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(allCtrs)) }) @@ -2933,7 +2933,7 @@ func TestAddContainerToPodPodIDConflict(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(allCtrs)) }) @@ -2958,7 +2958,7 @@ func TestAddContainerToPodPodNameConflict(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(allCtrs)) }) @@ -3064,7 +3064,7 @@ func TestAddContainerToPodDependencyOutsidePodFails(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(allCtrs)) @@ -3137,7 +3137,7 @@ func TestAddContainerToPodDependencyInSeparateNamespaceFails(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(allCtrs)) @@ -3164,7 +3164,7 @@ func TestAddContainerToPodSameNamespaceSucceeds(t *testing.T) { err = state.AddContainerToPod(testPod, testCtr) assert.NoError(t, err) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(allCtrs)) testContainersEqual(t, allCtrs[0], testCtr, true) @@ -3188,7 +3188,7 @@ func TestAddContainerToPodDifferentNamespaceFails(t *testing.T) { err = state.AddContainerToPod(testPod, testCtr) assert.Error(t, err) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(allCtrs)) }) @@ -3210,7 +3210,7 @@ func TestAddContainerToPodNamespaceOnCtrFails(t *testing.T) { err = state.AddContainerToPod(testPod, testCtr) assert.Error(t, err) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(allCtrs)) }) @@ -3232,7 +3232,7 @@ func TestAddContainerToPodNamespaceOnPodFails(t *testing.T) { err = state.AddContainerToPod(testPod, testCtr) assert.Error(t, err) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(allCtrs)) }) @@ -3290,7 +3290,7 @@ func TestAddCtrToPodDifferentNamespaceFails(t *testing.T) { err = state.SetNamespace("") assert.NoError(t, err) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) }) @@ -3360,7 +3360,7 @@ func TestRemoveContainerFromPodCtrNotInPodFails(t *testing.T) { assert.True(t, testCtr.valid) - ctrs, err := state.AllContainers() + ctrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(ctrs)) }) @@ -3388,7 +3388,7 @@ func TestRemoveContainerFromPodSucceeds(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(allCtrs)) }) @@ -3424,7 +3424,7 @@ func TestRemoveContainerFromPodWithDependencyFails(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 2, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 2, len(allCtrs)) }) @@ -3463,7 +3463,7 @@ func TestRemoveContainerFromPodWithDependencySucceedsAfterDepRemoved(t *testing. assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(allCtrs)) }) @@ -3498,7 +3498,7 @@ func TestRemoveContainerFromPodSameNamespaceSucceeds(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 0, len(allCtrs)) }) @@ -3536,7 +3536,7 @@ func TestRemoveContainerFromPodDifferentNamespaceFails(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, len(ctrs)) - allCtrs, err := state.AllContainers() + allCtrs, err := state.AllContainers(false) assert.NoError(t, err) assert.Equal(t, 1, len(allCtrs)) }) diff --git a/pkg/api/handlers/compat/containers.go b/pkg/api/handlers/compat/containers.go index 5435f0d5d2..042fb9e893 100644 --- a/pkg/api/handlers/compat/containers.go +++ b/pkg/api/handlers/compat/containers.go @@ -146,7 +146,7 @@ func ListContainers(w http.ResponseWriter, r *http.Request) { filterFuncs = append(filterFuncs, runningOnly) } - containers, err := runtime.GetContainers(filterFuncs...) + containers, err := runtime.GetContainers(false, filterFuncs...) if err != nil { utils.InternalServerError(w, err) return diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index b058489241..5674f675f3 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -64,7 +64,7 @@ func getContainers(runtime *libpod.Runtime, options getContainersOptions) ([]con } filterFuncs = append(filterFuncs, generatedFunc) } - ctrs, err := runtime.GetContainers(filterFuncs...) + ctrs, err := runtime.GetContainers(false, filterFuncs...) if err != nil { return nil, err } @@ -665,7 +665,7 @@ func (ic *ContainerEngine) ContainerRestore(ctx context.Context, namesOrIds []st case options.Import != "": ctrs, err = checkpoint.CRImportCheckpointTar(ctx, ic.Libpod, options) case options.All: - ctrs, err = ic.Libpod.GetContainers(filterFuncs...) + ctrs, err = ic.Libpod.GetContainers(false, filterFuncs...) case options.Latest: containers, err := getContainers(ic.Libpod, getContainersOptions{latest: options.Latest, names: namesOrIds}) if err != nil { diff --git a/pkg/ps/ps.go b/pkg/ps/ps.go index b93f1eb745..30c1c8c87c 100644 --- a/pkg/ps/ps.go +++ b/pkg/ps/ps.go @@ -51,7 +51,14 @@ func GetContainerLists(runtime *libpod.Runtime, options entities.ContainerListOp filterFuncs = append(filterFuncs, runningOnly) } - cons, err := runtime.GetContainers(filterFuncs...) + // Load the containers with their states populated. This speeds things + // up considerably as we use a signel DB connection to load the + // containers' states instead of one per container. + // + // This may return slightly outdated states but that's acceptable for + // listing containers; any state is outdated the point a container lock + // gets released. + cons, err := runtime.GetContainers(true, filterFuncs...) if err != nil { return nil, err }