Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve crictl inspect[pi] commands to allow filtering #1553

Merged
merged 1 commit into from
Aug 7, 2024
Merged

Improve crictl inspect[pi] commands to allow filtering #1553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 76 additions & 8 deletions cmd/crictl/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,19 +502,78 @@ var containerStatusCommand = &cli.Command{
Name: "template",
Usage: "The template string is only used when output is go-template; The Template format is golang template",
},
&cli.StringFlag{
Name: "name",
Usage: "Filter by container name regular expression pattern",
},
&cli.StringFlag{
Name: "pod",
Aliases: []string{"p"},
Usage: "Filter by pod id",
},
&cli.StringFlag{
Name: "image",
Usage: "Filter by container image",
},
&cli.StringFlag{
Name: "state",
Aliases: []string{"s"},
Usage: "Filter by container state",
},
&cli.StringSliceFlag{
Name: "label",
Usage: "Filter by key=value label",
},
&cli.BoolFlag{
Name: "latest",
Aliases: []string{"l"},
Usage: "Show the most recently created container (includes all states)",
},
&cli.IntFlag{
Name: "last",
Aliases: []string{"n"},
Usage: "Show last n recently created containers (includes all states). Set 0 for unlimited.",
},
},
Action: func(c *cli.Context) error {
if c.NArg() == 0 {
return errors.New("ID cannot be empty")
}
runtimeClient, err := getRuntimeService(c, 0)
if err != nil {
return err
}

ids := c.Args().Slice()

if len(ids) == 0 {
opts := &listOptions{
nameRegexp: c.String("name"),
podID: c.String("pod"),
image: c.String("image"),
state: c.String("state"),
latest: c.Bool("latest"),
last: c.Int("last"),
}
opts.labels, err = parseLabelStringSlice(c.StringSlice("label"))
if err != nil {
return err
}

ctrs, err := ListContainers(runtimeClient, opts)
if err != nil {
return fmt.Errorf("listing containers: %w", err)
}
for _, ctr := range ctrs {
ids = append(ids, ctr.GetId())
}
}

if len(ids) == 0 {
logrus.Error("No IDs provided or nothing found per filter")
return cli.ShowSubcommandHelp(c)
}

if err := containerStatus(
runtimeClient,
c.Args().Slice(),
ids,
c.String("output"),
c.String("template"),
c.Bool("quiet"),
Expand Down Expand Up @@ -633,7 +692,7 @@ var listContainersCommand = &cli.Command{
return err
}

if err = ListContainers(runtimeClient, imageClient, opts); err != nil {
if err = OutputContainers(runtimeClient, imageClient, opts); err != nil {
return fmt.Errorf("listing containers: %w", err)
}
return nil
Expand Down Expand Up @@ -1080,7 +1139,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, imageClient internalapi.ImageManagerService, opts *listOptions) error {
func ListContainers(runtimeClient internalapi.RuntimeService, opts *listOptions) ([]*pb.Container, error) {
filter := &pb.ContainerFilter{}
if opts.id != "" {
filter.Id = opts.id
Expand Down Expand Up @@ -1124,9 +1183,18 @@ func ListContainers(runtimeClient internalapi.RuntimeService, imageClient intern
})
logrus.Debugf("ListContainerResponse: %v", r)
if err != nil {
return err
return nil, fmt.Errorf("call list containers RPC: %w", err)
}
return getContainersList(r, opts), nil
}

// 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)
if err != nil {
return fmt.Errorf("list containers: %w", err)
}
r = getContainersList(r, opts)

switch opts.output {
case outputTypeJSON:
Expand Down
58 changes: 40 additions & 18 deletions cmd/crictl/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,20 +191,11 @@ var listImageCommand = &cli.Command{
return err
}

r, err := ListImages(imageClient, c.Args().First())
r, err := ListImages(imageClient, c.Args().First(), c.StringSlice("filter"))
if err != nil {
return fmt.Errorf("listing images: %w", err)
}

sort.Sort(imageByRef(r.Images))

if len(c.StringSlice("filter")) > 0 && len(r.Images) > 0 {
r.Images, err = filterImagesList(r.Images, c.StringSlice("filter"))
if err != nil {
return fmt.Errorf("listing images: %w", err)
}
}

switch c.String("output") {
case outputTypeJSON:
return outputProtobufObjAsJSON(r)
Expand Down Expand Up @@ -303,11 +294,17 @@ var imageStatusCommand = &cli.Command{
Name: "template",
Usage: "The template string is only used when output is go-template; The Template format is golang template",
},
&cli.StringFlag{
Name: "name",
Usage: "Filter by image name",
},
&cli.StringSliceFlag{
Name: "filter",
Aliases: []string{"f"},
Usage: "Filter output based on provided conditions.\nAvailable filters: \n* dangling=(boolean - true or false)\n* reference=/regular expression/\n* before=<image-name>[:<tag>]|<image id>|<image@digest>\n* since=<image-name>[:<tag>]|<image id>|<image@digest>\nMultiple filters can be combined together.",
},
},
Action: func(c *cli.Context) error {
if c.NArg() == 0 {
return cli.ShowSubcommandHelp(c)
}
imageClient, err := getImageService(c)
if err != nil {
return err
Expand All @@ -320,10 +317,25 @@ var imageStatusCommand = &cli.Command{
}
tmplStr := c.String("template")

statuses := []statusData{}
for i := range c.NArg() {
id := c.Args().Get(i)
ids := c.Args().Slice()

if len(ids) == 0 {
r, err := ListImages(imageClient, c.String("name"), c.StringSlice("filter"))
if err != nil {
return fmt.Errorf("listing images: %w", err)
}
for _, img := range r.GetImages() {
ids = append(ids, img.GetId())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to deduplicate here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say no, because right now it behaves in the same way to output two images when doing something like:

sudo crictl inspecti 324bc02ae1231 324bc02ae1231

}
}

if len(ids) == 0 {
logrus.Error("No IDs provided or nothing found per filter")
return cli.ShowSubcommandHelp(c)
}

statuses := []statusData{}
for _, id := range ids {
r, err := ImageStatus(imageClient, id, verbose)
if err != nil {
return fmt.Errorf("image status for %q request: %w", id, err)
Expand Down Expand Up @@ -684,8 +696,8 @@ func PullImageWithSandbox(client internalapi.ImageManagerService, image string,

// ListImages sends a ListImagesRequest to the server, and parses
// the returned ListImagesResponse.
func ListImages(client internalapi.ImageManagerService, image string) (*pb.ListImagesResponse, error) {
request := &pb.ListImagesRequest{Filter: &pb.ImageFilter{Image: &pb.ImageSpec{Image: image}}}
func ListImages(client internalapi.ImageManagerService, nameFilter string, conditionFilters []string) (*pb.ListImagesResponse, error) {
request := &pb.ListImagesRequest{Filter: &pb.ImageFilter{Image: &pb.ImageSpec{Image: nameFilter}}}
logrus.Debugf("ListImagesRequest: %v", request)
res, err := InterruptableRPC(nil, func(ctx context.Context) ([]*pb.Image, error) {
return client.ListImages(ctx, request.Filter)
Expand All @@ -695,6 +707,16 @@ func ListImages(client internalapi.ImageManagerService, image string) (*pb.ListI
}
resp := &pb.ListImagesResponse{Images: res}
logrus.Debugf("ListImagesResponse: %v", resp)

sort.Sort(imageByRef(resp.Images))

if len(conditionFilters) > 0 && len(resp.Images) > 0 {
resp.Images, err = filterImagesList(resp.Images, conditionFilters)
if err != nil {
return nil, fmt.Errorf("filter images: %w", err)
}
}

return resp, nil
}

Expand Down
89 changes: 75 additions & 14 deletions cmd/crictl/sandbox.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,19 +218,71 @@ var podStatusCommand = &cli.Command{
Name: "template",
Usage: "The template string is only used when output is go-template; The Template format is golang template",
},
&cli.StringFlag{
Name: "name",
Usage: "Filter by pod name regular expression pattern",
},
&cli.StringFlag{
Name: "namespace",
Usage: "Filter by pod namespace regular expression pattern",
},
&cli.StringFlag{
Name: "state",
Aliases: []string{"s"},
Usage: "Filter by pod state",
},
&cli.StringSliceFlag{
Name: "label",
Usage: "Filter by key=value label",
},
&cli.BoolFlag{
Name: "latest",
Aliases: []string{"l"},
Usage: "Show the most recently created pod",
},
&cli.IntFlag{
Name: "last",
Aliases: []string{"n"},
Usage: "Show last n recently created pods. Set 0 for unlimited",
},
},
Action: func(c *cli.Context) error {
if c.NArg() == 0 {
return cli.ShowSubcommandHelp(c)
}
runtimeClient, err := getRuntimeService(c, 0)
if err != nil {
return err
}

ids := c.Args().Slice()

if len(ids) == 0 {
opts := &listOptions{
nameRegexp: c.String("name"),
podNamespaceRegexp: c.String("namespace"),
state: c.String("state"),
latest: c.Bool("latest"),
last: c.Int("last"),
}
opts.labels, err = parseLabelStringSlice(c.StringSlice("label"))
if err != nil {
return fmt.Errorf("parse label string slice: %w", err)
}
sbs, err := ListPodSandboxes(runtimeClient, opts)
if err != nil {
return fmt.Errorf("listing pod sandboxes: %w", err)
}
for _, sb := range sbs {
ids = append(ids, sb.GetId())
}
}

if len(ids) == 0 {
logrus.Error("No IDs provided or nothing found per filter")
return cli.ShowSubcommandHelp(c)
}

if err := podSandboxStatus(
runtimeClient,
c.Args().Slice(),
ids,
c.String("output"),
c.Bool("quiet"),
c.String("template"),
Expand All @@ -253,30 +305,30 @@ var listPodCommand = &cli.Command{
},
&cli.StringFlag{
Name: "name",
Usage: "filter by pod name regular expression pattern",
Usage: "Filter by pod name regular expression pattern",
},
&cli.StringFlag{
Name: "namespace",
Usage: "filter by pod namespace regular expression pattern",
Usage: "Filter by pod namespace regular expression pattern",
},
&cli.StringFlag{
Name: "state",
Aliases: []string{"s"},
Usage: "filter by pod state",
Usage: "Filter by pod state",
},
&cli.StringSliceFlag{
Name: "label",
Usage: "filter by key=value label",
Usage: "Filter by key=value label",
},
&cli.BoolFlag{
Name: "verbose",
Aliases: []string{"v"},
Usage: "show verbose info for pods",
Usage: "Show verbose info for pods",
},
&cli.BoolFlag{
Name: "quiet",
Aliases: []string{"q"},
Usage: "list only pod IDs",
Usage: "List only pod IDs",
},
&cli.StringFlag{
Name: "output",
Expand Down Expand Up @@ -322,7 +374,7 @@ var listPodCommand = &cli.Command{
if err != nil {
return err
}
if err = ListPodSandboxes(runtimeClient, opts); err != nil {
if err = OutputPodSandboxes(runtimeClient, opts); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if filtered by reference, what image reference will the output use as a main one? My understanding is that even with the filter, the output may show different reference as a "main" name of an image and tag one filtered by in a list of alternative names. Is it right? I wonder this is actually the case and whether we can either improve output or make it clear in docs so not to confuse users

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, pods are not filterable by reference, are we speaking about crictl inspecti? My main goal was to mimic closely as possible the existing crictl pods, ps and images subcommand flags to avoid confusion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in case of various filters, potential improvement over time may be to show filter-specific additional column. For since it may be time passed since that value in the filter, for example. This is just a nit.

return fmt.Errorf("listing pod sandboxes: %w", err)
}
return nil
Expand Down Expand Up @@ -482,7 +534,7 @@ func outputPodSandboxStatusTable(r *pb.PodSandboxStatusResponse, verbose bool) {

// ListPodSandboxes sends a ListPodSandboxRequest to the server, and parses
// the returned ListPodSandboxResponse.
func ListPodSandboxes(client internalapi.RuntimeService, opts *listOptions) error {
func ListPodSandboxes(client internalapi.RuntimeService, opts *listOptions) ([]*pb.PodSandbox, error) {
filter := &pb.PodSandboxFilter{}
if opts.id != "" {
filter.Id = opts.id
Expand Down Expand Up @@ -513,9 +565,18 @@ func ListPodSandboxes(client internalapi.RuntimeService, opts *listOptions) erro
})
logrus.Debugf("ListPodSandboxResponse: %v", r)
if err != nil {
return err
return nil, fmt.Errorf("call list sandboxes RPC: %w", err)
}
return getSandboxesList(r, opts), nil
}

// OutputPodSandboxes sends a ListPodSandboxRequest to the server, and parses
// the returned ListPodSandboxResponse for output.
func OutputPodSandboxes(client internalapi.RuntimeService, opts *listOptions) error {
r, err := ListPodSandboxes(client, opts)
if err != nil {
return fmt.Errorf("list pod sandboxes: %w", err)
}
r = getSandboxesList(r, opts)

switch opts.output {
case outputTypeJSON:
Expand Down