Merge pull request kubernetes-sigs#1752 from saschagrunert/inspect-image

Fix `--image` filter for crictl `inspect` and `exec`

cmd/crictl/container.go

@@ -532,13 +532,23 @@ var containerStatusCommand = &cli.Command{
 			Aliases: []string{"n"},
 			Usage:   "Show last n recently created containers (includes all states). Set 0 for unlimited.",
 		},
+		&cli.BoolFlag{
+			Name:    "all",
+			Aliases: []string{"a"},
+			Usage:   "Show all containers",
+		},
 	},
 	Action: func(c *cli.Context) error {
 		runtimeClient, err := getRuntimeService(c, 0)
 		if err != nil {
 			return err
 		}
 
+		imageClient, err := getImageService(c)
+		if err != nil {
+			return err
+		}
+
 		ids := c.Args().Slice()
 
 		if len(ids) == 0 {
@@ -550,13 +560,14 @@ var containerStatusCommand = &cli.Command{
 				state:              c.String("state"),
 				latest:             c.Bool("latest"),
 				last:               c.Int("last"),
+				all:                c.Bool("all"),
 			}
 			opts.labels, err = parseLabelStringSlice(c.StringSlice("label"))
 			if err != nil {
 				return err
 			}
 
-			ctrs, err := ListContainers(runtimeClient, opts)
+			ctrs, err := ListContainers(runtimeClient, imageClient, opts)
 			if err != nil {
 				return fmt.Errorf("listing containers: %w", err)
 			}
@@ -1145,7 +1156,7 @@ func outputContainerStatusTable(r *pb.ContainerStatusResponse, verbose bool) {
 
 // ListContainers sends a ListContainerRequest to the server, and parses
 // the returned ListContainerResponse.
-func ListContainers(runtimeClient internalapi.RuntimeService, opts *listOptions) ([]*pb.Container, error) {
+func ListContainers(runtimeClient internalapi.RuntimeService, imageClient internalapi.ImageManagerService, opts *listOptions) ([]*pb.Container, error) {
 	filter := &pb.ContainerFilter{}
 	if opts.id != "" {
 		filter.Id = opts.id
@@ -1191,13 +1202,13 @@ func ListContainers(runtimeClient internalapi.RuntimeService, opts *listOptions)
 	if err != nil {
 		return nil, fmt.Errorf("call list containers RPC: %w", err)
 	}
-	return getContainersList(r, opts), nil
+	return getContainersList(imageClient, r, opts)
 }
 
 // OutputContainers sends a ListContainerRequest to the server, and parses
 // the returned ListContainerResponse for output.
 func OutputContainers(runtimeClient internalapi.RuntimeService, imageClient internalapi.ImageManagerService, opts *listOptions) error {
-	r, err := ListContainers(runtimeClient, opts)
+	r, err := ListContainers(runtimeClient, imageClient, opts)
 	if err != nil {
 		return fmt.Errorf("list containers: %w", err)
 	}
@@ -1218,11 +1229,6 @@ func OutputContainers(runtimeClient internalapi.RuntimeService, imageClient inte
 		display.AddRow([]string{columnContainer, columnImage, columnCreated, columnState, columnName, columnAttempt, columnPodID, columnPodName, columnNamespace})
 	}
 	for _, c := range r {
-		if match, err := matchesImage(imageClient, opts.image, c.GetImage().GetImage()); err != nil {
-			return fmt.Errorf("check image match: %w", err)
-		} else if !match {
-			continue
-		}
 		if opts.quiet {
 			fmt.Printf("%s\n", c.Id)
 			continue
@@ -1326,9 +1332,15 @@ func getFromLabels(labels map[string]string, label string) string {
 	return "unknown"
 }
 
-func getContainersList(containersList []*pb.Container, opts *listOptions) []*pb.Container {
+func getContainersList(imageClient internalapi.ImageManagerService, containersList []*pb.Container, opts *listOptions) ([]*pb.Container, error) {
 	filtered := []*pb.Container{}
 	for _, c := range containersList {
+		if match, err := matchesImage(imageClient, opts.image, c.GetImage().GetImage()); err != nil {
+			return nil, fmt.Errorf("check image match: %w", err)
+		} else if !match {
+			continue
+		}
+
 		podNamespace := getPodNamespaceFromLabels(c.Labels)
 		// Filter by pod name/namespace regular expressions.
 		if c.Metadata != nil &&
@@ -1353,5 +1365,5 @@ func getContainersList(containersList []*pb.Container, opts *listOptions) []*pb.
 		return b
 	}(n, len(filtered))
 
-	return filtered[:n]
+	return filtered[:n], nil
 }

cmd/crictl/container_test.go

@@ -47,7 +47,8 @@ func fakeContainer(name string, createdAt int64) *pb.Container {
 
 var _ = DescribeTable("getContainersList",
 	func(input []*pb.Container, options *listOptions, indexes []int) {
-		actual := getContainersList(input, options)
+		actual, err := getContainersList(nil, input, options)
+		Expect(err).NotTo(HaveOccurred())
 		var expected []*pb.Container
 		for _, i := range indexes {
 			expected = append(expected, input[i])

cmd/crictl/exec.go

@@ -154,6 +154,11 @@ var runtimeExecCommand = &cli.Command{
 			return err
 		}
 
+		imageClient, err := getImageService(c)
+		if err != nil {
+			return err
+		}
+
 		// Assume a regular exec where the first arg is the container ID.
 		ids := []string{c.Args().First()}
 		cmd := c.Args().Slice()[1:]
@@ -193,7 +198,7 @@ var runtimeExecCommand = &cli.Command{
 				return err
 			}
 
-			ctrs, err := ListContainers(runtimeClient, opts)
+			ctrs, err := ListContainers(runtimeClient, imageClient, opts)
 			if err != nil {
 				return fmt.Errorf("listing containers: %w", err)
 			}

cmd/crictl/util.go

@@ -492,7 +492,7 @@ func matchesRegex(pattern, target string) bool {
 }
 
 func matchesImage(imageClient internalapi.ImageManagerService, image, containerImage string) (bool, error) {
-	if image == "" {
+	if image == "" || imageClient == nil {
 		return true, nil
 	}
 	r1, err := ImageStatus(imageClient, image, false)

test/e2e/inspect_test.go

@@ -0,0 +1,92 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"os"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gexec"
+)
+
+// The actual test suite.
+var _ = t.Describe("inspect", func() {
+	const imageLatest = registry + "test-image-latest"
+	sandbox := ""
+
+	BeforeEach(func() {
+		sb, err := os.CreateTemp("", "sandbox-")
+		Expect(err).NotTo(HaveOccurred())
+		_, err = fmt.Fprintf(sb, `{ "metadata": { "name": "sb", "uid": "uid", "namespace": "ns" }}`)
+		Expect(err).NotTo(HaveOccurred())
+
+		ctr, err := os.CreateTemp("", "container-")
+		Expect(err).NotTo(HaveOccurred())
+		_, err = fmt.Fprintf(ctr, `{ "metadata": { "name": "ctr" }, "image": { "image": "`+imageLatest+`" }, "args": [] }`)
+		Expect(err).NotTo(HaveOccurred())
+
+		res := t.Crictl(fmt.Sprintf("run %s %s", ctr.Name(), sb.Name()))
+		Expect(res).To(Exit(0))
+		Expect(os.RemoveAll(sb.Name())).NotTo(HaveOccurred())
+		Expect(os.RemoveAll(ctr.Name())).NotTo(HaveOccurred())
+
+		res = t.Crictl("pods --name sb -q")
+		Expect(res).To(Exit(0))
+		sandbox = string(bytes.TrimSpace(res.Out.Contents()))
+	})
+
+	AfterEach(func() {
+		res := t.Crictl("rmp -f " + sandbox)
+		Expect(res).To(Exit(0))
+	})
+
+	validateSingleResponse := func(contents []byte) {
+		singleResponse := map[string]any{}
+		Expect(json.Unmarshal(contents, &singleResponse)).NotTo(HaveOccurred())
+		Expect(singleResponse).To(HaveKey("info"))
+		Expect(singleResponse).To(HaveKey("status"))
+	}
+
+	expectNothingFound := func(contents []byte) {
+		Expect(string(contents)).To(ContainSubstring("nothing found per filter"))
+	}
+
+	It("should succeed", func() {
+		res := t.Crictl("inspect -a")
+		Expect(res).To(Exit(0))
+		validateSingleResponse(res.Out.Contents())
+
+		// Not output without `--all` since container is exited
+		res = t.Crictl("inspect")
+		Expect(res).To(Exit(0))
+		expectNothingFound(res.Err.Contents())
+
+		// Should allow filter per image name
+		res = t.Crictl("inspect -a --image " + imageLatest)
+		Expect(res).To(Exit(0))
+		validateSingleResponse(res.Out.Contents())
+
+		// Should filter nothing if image does not match
+		res = t.Crictl("inspect -a --image wrong")
+		Expect(res).To(Exit(0))
+		expectNothingFound(res.Err.Contents())
+	})
+})

test/e2e/inspecti_test.go

@@ -29,7 +29,6 @@ import (
 var _ = t.Describe("inspecti", func() {
 	const (
 		imageSuccessText = "Image is up to date"
-		registry         = "gcr.io/k8s-staging-cri-tools/"
 		image1           = registry + "test-image-2"
 		image2           = registry + "test-image-3"
 	)

test/e2e/suite_test.go

@@ -25,6 +25,8 @@ import (
 	. "sigs.k8s.io/cri-tools/test/framework"
 )
 
+const registry = "gcr.io/k8s-staging-cri-tools/"
+
 // TestE2E runs the created specs.
 func TestE2E(t *testing.T) {
 	t.Parallel()