From a6bea7907eb7db5bb9b51309cf0c0c6f72b62f9e Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Tue, 21 Jan 2025 09:55:22 +0100 Subject: [PATCH] Fix `--image` filter for crictl `inspect` and `exec` Signed-off-by: Sascha Grunert --- cmd/crictl/container.go | 34 ++++++++----- cmd/crictl/container_test.go | 3 +- cmd/crictl/exec.go | 7 ++- cmd/crictl/util.go | 2 +- test/e2e/inspect_test.go | 92 ++++++++++++++++++++++++++++++++++++ test/e2e/inspecti_test.go | 1 - test/e2e/suite_test.go | 2 + 7 files changed, 126 insertions(+), 15 deletions(-) create mode 100644 test/e2e/inspect_test.go diff --git a/cmd/crictl/container.go b/cmd/crictl/container.go index 70b93ad8f6..05d708a9c9 100644 --- a/cmd/crictl/container.go +++ b/cmd/crictl/container.go @@ -532,6 +532,11 @@ 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) @@ -539,6 +544,11 @@ var containerStatusCommand = &cli.Command{ 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 } diff --git a/cmd/crictl/container_test.go b/cmd/crictl/container_test.go index a73c6985f5..69e33209e6 100644 --- a/cmd/crictl/container_test.go +++ b/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]) diff --git a/cmd/crictl/exec.go b/cmd/crictl/exec.go index df601d9ee0..725b86cf71 100644 --- a/cmd/crictl/exec.go +++ b/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) } diff --git a/cmd/crictl/util.go b/cmd/crictl/util.go index 2fa69e36c3..5802c899a1 100644 --- a/cmd/crictl/util.go +++ b/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) diff --git a/test/e2e/inspect_test.go b/test/e2e/inspect_test.go new file mode 100644 index 0000000000..5c7b2f83f8 --- /dev/null +++ b/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()) + }) +}) diff --git a/test/e2e/inspecti_test.go b/test/e2e/inspecti_test.go index f80afa8b69..bff4ca9a93 100644 --- a/test/e2e/inspecti_test.go +++ b/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" ) diff --git a/test/e2e/suite_test.go b/test/e2e/suite_test.go index 942025d0dd..fba57cf40f 100644 --- a/test/e2e/suite_test.go +++ b/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()