Unconditionally retrieve pod names via API

The ListContainers API previously had a Pod parameter, which
determined if pod name was returned (but, notably, not Pod ID,
which was returned unconditionally). This was fairly confusing,
so we decided to deprecate/remove the parameter and return it
unconditionally.

To do this without serious performance implications, we need to
avoid expensive JSON decodes of pod configuration in the DB. The
way our Bolt tables are structured, retrieving name given ID is
actually quite cheap, but we did not expose this via the Libpod
API. Add a new GetName API to do this.

Fixes #7214

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

11 files changed, 134 insertions, 14 deletions

diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go
index 38881d3e4..4e2630526 100644
--- a/libpod/boltdb_state.go
+++ b/libpod/boltdb_state.go
@@ -424,6 +424,61 @@ func (s *BoltState) SetNamespace(ns string) error {
 	return nil
 }
 
+// GetName returns the name associated with a given ID. Since IDs are globally
+// unique, it works for both containers and pods.
+// Returns ErrNoSuchCtr if the ID does not exist.
+func (s *BoltState) GetName(id string) (string, error) {
+	if id == "" {
+		return "", define.ErrEmptyID
+	}
+
+	if !s.valid {
+		return "", define.ErrDBClosed
+	}
+
+	idBytes := []byte(id)
+
+	db, err := s.getDBCon()
+	if err != nil {
+		return "", err
+	}
+	defer s.deferredCloseDBCon(db)
+
+	name := ""
+
+	err = db.View(func(tx *bolt.Tx) error {
+		idBkt, err := getIDBucket(tx)
+		if err != nil {
+			return err
+		}
+
+		nameBytes := idBkt.Get(idBytes)
+		if nameBytes == nil {
+			return define.ErrNoSuchCtr
+		}
+
+		if s.namespaceBytes != nil {
+			nsBkt, err := getNSBucket(tx)
+			if err != nil {
+				return err
+			}
+
+			idNs := nsBkt.Get(idBytes)
+			if !bytes.Equal(idNs, s.namespaceBytes) {
+				return define.ErrNoSuchCtr
+			}
+		}
+
+		name = string(nameBytes)
+		return nil
+	})
+	if err != nil {
+		return "", err
+	}
+
+	return name, nil
+}
+
 // Container retrieves a single container from the state by its full ID
 func (s *BoltState) Container(id string) (*Container, error) {
 	if id == "" {
diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go
index 794212bf0..b0eae0992 100644
--- a/libpod/in_memory_state.go
+++ b/libpod/in_memory_state.go
@@ -106,6 +106,36 @@ func (s *InMemoryState) SetNamespace(ns string) error {
 	return nil
 }
 
+// GetName retrieves the name associated with a given ID.
+// Works with both Container and Pod IDs.
+func (s *InMemoryState) GetName(id string) (string, error) {
+	if id == "" {
+		return "", define.ErrEmptyID
+	}
+
+	var idIndex *truncindex.TruncIndex
+	if s.namespace != "" {
+		nsIndex, ok := s.namespaceIndexes[s.namespace]
+		if !ok {
+			// We have no containers in the namespace
+			// Return false
+			return "", define.ErrNoSuchCtr
+		}
+		idIndex = nsIndex.idIndex
+	} else {
+		idIndex = s.idIndex
+	}
+
+	fullID, err := idIndex.Get(id)
+	if err != nil {
+		if err == truncindex.ErrNotExist {
+			return "", define.ErrNoSuchCtr
+		}
+		return "", errors.Wrapf(err, "error performing truncindex lookup for ID %s", id)
+	}
+	return fullID, nil
+}
+
 // Container retrieves a container from its full ID
 func (s *InMemoryState) Container(id string) (*Container, error) {
 	if id == "" {
diff --git a/libpod/runtime.go b/libpod/runtime.go
index 24370d50e..867644be7 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -732,6 +732,22 @@ func (r *Runtime) GetStore() storage.Store {
 	return r.store
 }
 
+// GetName retrieves the name associated with a given full ID.
+// This works for both containers and pods, and does not distinguish between the
+// two.
+// If the given ID does not correspond to any existing Pod or Container,
+// ErrNoSuchCtr is returned.
+func (r *Runtime) GetName(id string) (string, error) {
+	r.lock.RLock()
+	defer r.lock.RUnlock()
+
+	if !r.valid {
+		return "", define.ErrRuntimeStopped
+	}
+
+	return r.state.GetName(id)
+}
+
 // DBConfig is a set of Libpod runtime configuration settings that are saved in
 // a State when it is first created, and can subsequently be retrieved.
 type DBConfig struct {
diff --git a/libpod/state.go b/libpod/state.go
index 6206a2994..44632b02f 100644
--- a/libpod/state.go
+++ b/libpod/state.go
@@ -43,6 +43,12 @@ type State interface {
 	// containers and pods in all namespaces will be returned.
 	SetNamespace(ns string) error
 
+	// Resolve an ID into a Name. Since Podman names and IDs are globally
+	// unique between Pods and Containers, the ID may belong to either a pod
+	// or container. Despite this, we will always return ErrNoSuchCtr if the
+	// ID does not exist.
+	GetName(id string) (string, error)
+
 	// Return a container from the database from its full ID.
 	// If the container is not in the set namespace, an error will be
 	// returned.
diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go
index 2303ff17a..abfc79a0b 100644
--- a/pkg/api/handlers/libpod/containers.go
+++ b/pkg/api/handlers/libpod/containers.go
@@ -39,7 +39,6 @@ func ListContainers(w http.ResponseWriter, r *http.Request) {
 		Filters   map[string][]string `schema:"filters"`
 		Last      int                 `schema:"last"`
 		Namespace bool                `schema:"namespace"`
-		Pod       bool                `schema:"pod"`
 		Size      bool                `schema:"size"`
 		Sync      bool                `schema:"sync"`
 	}{
@@ -59,7 +58,7 @@ func ListContainers(w http.ResponseWriter, r *http.Request) {
 		Size:      query.Size,
 		Sort:      "",
 		Namespace: query.Namespace,
-		Pod:       query.Pod,
+		Pod:       true,
 		Sync:      query.Sync,
 	}
 	pss, err := ps.GetContainerLists(runtime, opts)
diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go
index 18ff2f423..a0c9ac574 100644
--- a/pkg/api/server/register_containers.go
+++ b/pkg/api/server/register_containers.go
@@ -661,11 +661,10 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error {
 	//    type: boolean
 	//    description: Include namespace information
 	//    default: false
-	//  - in: query
 	//    name: pod
 	//    type: boolean
 	//    default: false
-	//    description: Include Pod ID and Name if applicable
+	//    description: Ignored. Previously included details on pod name and ID that are currently included by default.
 	//  - in: query
 	//    name: size
 	//    type: boolean
diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go
index c479e5dcb..d7e67567b 100644
--- a/pkg/bindings/containers/containers.go
+++ b/pkg/bindings/containers/containers.go
@@ -24,7 +24,7 @@ var (
 // the most recent number of containers.  The pod and size booleans indicate that pod information and rootfs
 // size information should also be included.  Finally, the sync bool synchronizes the OCI runtime and
 // container state.
-func List(ctx context.Context, filters map[string][]string, all *bool, last *int, pod, size, sync *bool) ([]entities.ListContainer, error) { // nolint:typecheck
+func List(ctx context.Context, filters map[string][]string, all *bool, last *int, size, sync *bool) ([]entities.ListContainer, error) { // nolint:typecheck
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
 		return nil, err
@@ -37,9 +37,6 @@ func List(ctx context.Context, filters map[string][]string, all *bool, last *int
 	if last != nil {
 		params.Set("last", strconv.Itoa(*last))
 	}
-	if pod != nil {
-		params.Set("pod", strconv.FormatBool(*pod))
-	}
 	if size != nil {
 		params.Set("size", strconv.FormatBool(*size))
 	}
diff --git a/pkg/bindings/test/containers_test.go b/pkg/bindings/test/containers_test.go
index 4b2c78353..0685a3377 100644
--- a/pkg/bindings/test/containers_test.go
+++ b/pkg/bindings/test/containers_test.go
@@ -509,7 +509,7 @@ var _ = Describe("Podman containers ", func() {
 		Expect(err).To(BeNil())
 		_, err = bt.RunTopContainer(&name2, bindings.PFalse, nil)
 		Expect(err).To(BeNil())
-		containerLatestList, err := containers.List(bt.conn, nil, nil, &latestContainers, nil, nil, nil)
+		containerLatestList, err := containers.List(bt.conn, nil, nil, &latestContainers, nil, nil)
 		Expect(err).To(BeNil())
 		err = containers.Kill(bt.conn, containerLatestList[0].Names[0], "SIGTERM")
 		Expect(err).To(BeNil())
@@ -754,8 +754,23 @@ var _ = Describe("Podman containers ", func() {
 		// Validate list container with id filter
 		filters := make(map[string][]string)
 		filters["id"] = []string{cid}
-		c, err := containers.List(bt.conn, filters, bindings.PTrue, nil, nil, nil, nil)
+		c, err := containers.List(bt.conn, filters, bindings.PTrue, nil, nil, nil)
 		Expect(err).To(BeNil())
 		Expect(len(c)).To(Equal(1))
 	})
+
+	It("List containers always includes pod information", func() {
+		podName := "testpod"
+		ctrName := "testctr"
+		bt.Podcreate(&podName)
+		_, err := bt.RunTopContainer(&ctrName, bindings.PTrue, &podName)
+		Expect(err).To(BeNil())
+
+		lastNum := 1
+
+		c, err := containers.List(bt.conn, nil, bindings.PTrue, &lastNum, nil, nil)
+		Expect(err).To(BeNil())
+		Expect(len(c)).To(Equal(1))
+		Expect(c[0].PodName).To(Equal(podName))
+	})
 })
diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go
index 8835248ca..dd72064a3 100644
--- a/pkg/domain/infra/tunnel/containers.go
+++ b/pkg/domain/infra/tunnel/containers.go
@@ -496,7 +496,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri
 }
 
 func (ic *ContainerEngine) ContainerList(ctx context.Context, options entities.ContainerListOptions) ([]entities.ListContainer, error) {
-	return containers.List(ic.ClientCxt, options.Filters, &options.All, &options.Last, &options.Pod, &options.Size, &options.Sync)
+	return containers.List(ic.ClientCxt, options.Filters, &options.All, &options.Last, &options.Size, &options.Sync)
 }
 
 func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.ContainerRunOptions) (*entities.ContainerRunReport, error) {
diff --git a/pkg/domain/infra/tunnel/helpers.go b/pkg/domain/infra/tunnel/helpers.go
index 9974c4d1d..90e52c5c0 100644
--- a/pkg/domain/infra/tunnel/helpers.go
+++ b/pkg/domain/infra/tunnel/helpers.go
@@ -20,7 +20,7 @@ func getContainersByContext(contextWithConnection context.Context, all bool, nam
 	if all && len(namesOrIDs) > 0 {
 		return nil, errors.New("cannot lookup containers and all")
 	}
-	c, err := containers.List(contextWithConnection, nil, bindings.PTrue, nil, nil, nil, bindings.PTrue)
+	c, err := containers.List(contextWithConnection, nil, bindings.PTrue, nil, nil, bindings.PTrue)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/ps/ps.go b/pkg/ps/ps.go
index 2b81311af..d4fd96596 100644
--- a/pkg/ps/ps.go
+++ b/pkg/ps/ps.go
@@ -175,11 +175,14 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
 		State:     conState.String(),
 	}
 	if opts.Pod && len(conConfig.Pod) > 0 {
-		pod, err := rt.GetPod(conConfig.Pod)
+		podName, err := rt.GetName(conConfig.Pod)
 		if err != nil {
+			if errors.Cause(err) == define.ErrNoSuchCtr {
+				return entities.ListContainer{}, errors.Wrapf(define.ErrNoSuchPod, "could not find container %s pod (id %s) in state", conConfig.ID, conConfig.Pod)
+			}
 			return entities.ListContainer{}, err
 		}
-		ps.PodName = pod.Name()
+		ps.PodName = podName
 	}
 
 	if opts.Namespace {