Skip to content

Refactor the volume ls command flagging process #1773

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

Merged
AkihiroSuda merged 1 commit into from
Jan 2, 2023
Merged

Refactor the volume ls command flagging process #1773

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
8 changes: 2 additions & 6 deletions cmd/nerdctl/volume.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package main

import (
"github.com/containerd/nerdctl/pkg/clientutil"
"github.com/containerd/nerdctl/pkg/cmd/volume"
"github.com/containerd/nerdctl/pkg/mountutil/volumestore"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -56,9 +56,5 @@ func getVolumeStore(cmd *cobra.Command) (volumestore.VolumeStore, error) {
if err != nil {
return nil, err
}
dataStore, err := clientutil.DataStore(dataRoot, address)
if err != nil {
return nil, err
}
return volumestore.New(dataStore, ns)
return volume.Store(ns, dataRoot, address)
Copy link
Copy Markdown
Member

@djdongjin djdongjin Jan 2, 2023

Choose a reason for hiding this comment

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

seems this getVolumeStore is no longer needed as volume.Store should be preferred.

}
222 changes: 28 additions & 194 deletions cmd/nerdctl/volume_ls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,9 @@
package main

import (
"bytes"
"errors"
"fmt"
"strconv"
"strings"
"text/tabwriter"
"text/template"

"github.com/containerd/containerd/pkg/progress"
"github.com/containerd/nerdctl/pkg/formatter"
types "github.com/containerd/nerdctl/pkg/api/types"
"github.com/containerd/nerdctl/pkg/cmd/volume"
"github.com/containerd/nerdctl/pkg/inspecttypes/native"
"github.com/sirupsen/logrus"

"github.com/spf13/cobra"
)
Expand All @@ -54,220 +45,63 @@ func newVolumeLsCommand() *cobra.Command {
return volumeLsCommand
}

type volumePrintable struct {
Driver string
Labels string
Mountpoint string
Name string
Scope string
Size string
// TODO: "Links"
}

func volumeLsAction(cmd *cobra.Command, args []string) error {
options := &types.VolumeLsCommandOptions{}
options.Writer = cmd.OutOrStdout()
quiet, err := cmd.Flags().GetBool("quiet")
if err != nil {
return err
}
volumeSize, err := cmd.Flags().GetBool("size")
options.Quiet = quiet
Copy link
Copy Markdown
Member

@djdongjin djdongjin Jan 2, 2023

Choose a reason for hiding this comment

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

nit: instead of storing in local variables everytime, you can do

options.Quiet, err := cmd.Flags().GetBool("quiet")
...

same for other options.

Copy link
Copy Markdown
Member

@djdongjin djdongjin Jan 2, 2023

Choose a reason for hiding this comment

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

another style I recommand is first storing all flags in local variables and at the end, create the option struct

format, err := cmd.Flags().GetBool("format")
...
quiet,  err := cmd.Flags().GetBool("quiet")
...

option := types.VolumeLsCommandOptions{
    Format: format,
    Quiet: quiet,
}

format, err := cmd.Flags().GetString("format")
if err != nil {
return err
}
if quiet && volumeSize {
logrus.Warn("cannot use --size and --quiet together, ignoring --size")
volumeSize = false
options.Format = format
size, err := cmd.Flags().GetBool("size")
if err != nil {
return err
}
options.Size = size
filters, err := cmd.Flags().GetStringSlice("filter")
if err != nil {
return err
}
labelFilterFuncs, nameFilterFuncs, sizeFilterFuncs, isFilter, err := getVolumeFilterFuncs(filters)
options.Filters = filters
ns, err := cmd.Flags().GetString("namespace")
if err != nil {
return err
}
if len(sizeFilterFuncs) > 0 && quiet {
logrus.Warn("cannot use --filter=size and --quiet together, ignoring --filter=size")
sizeFilterFuncs = nil
}
if len(sizeFilterFuncs) > 0 && !volumeSize {
logrus.Warn("should use --filter=size and --size together")
cmd.Flags().Set("size", "true")
volumeSize = true
}
w := cmd.OutOrStdout()
var tmpl *template.Template
format, err := cmd.Flags().GetString("format")
options.Namespace = ns
dataRoot, err := cmd.Flags().GetString("data-root")
if err != nil {
return err
}
switch format {
case "", "table", "wide":
w = tabwriter.NewWriter(cmd.OutOrStdout(), 4, 8, 4, ' ', 0)
if !quiet {
if volumeSize {
fmt.Fprintln(w, "VOLUME NAME\tDIRECTORY\tSIZE")
} else {
fmt.Fprintln(w, "VOLUME NAME\tDIRECTORY")
}
}
case "raw":
return errors.New("unsupported format: \"raw\"")
default:
if quiet {
return errors.New("format and quiet must not be specified together")
}
var err error
tmpl, err = formatter.ParseTemplate(format)
if err != nil {
return err
}
}

vols, err := getVolumes(cmd)
options.DataRoot = dataRoot
address, err := cmd.Flags().GetString("address")
if err != nil {
return err
}

for _, v := range vols {
if isFilter && !volumeMatchesFilter(v, labelFilterFuncs, nameFilterFuncs, sizeFilterFuncs) {
continue
}
p := volumePrintable{
Driver: "local",
Labels: "",
Mountpoint: v.Mountpoint,
Name: v.Name,
Scope: "local",
}
if v.Labels != nil {
p.Labels = formatter.FormatLabels(*v.Labels)
}
if volumeSize {
p.Size = progress.Bytes(v.Size).String()
}
if tmpl != nil {
var b bytes.Buffer
if err := tmpl.Execute(&b, p); err != nil {
return err
}
if _, err = fmt.Fprintf(w, b.String()+"\n"); err != nil {
return err
}
} else if quiet {
fmt.Fprintln(w, p.Name)
} else if volumeSize {
fmt.Fprintf(w, "%s\t%s\t%s\n", p.Name, p.Mountpoint, p.Size)
} else {
fmt.Fprintf(w, "%s\t%s\n", p.Name, p.Mountpoint)
}
}
if f, ok := w.(formatter.Flusher); ok {
return f.Flush()
}
return nil
options.Address = address
return volume.Ls(options)
}

func getVolumes(cmd *cobra.Command) (map[string]native.Volume, error) {
Copy link
Copy Markdown
Member

@djdongjin djdongjin Jan 2, 2023

Choose a reason for hiding this comment

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

is this getVolumes func still being used somewhere? This func seems no longer necessary since it only read 4 flags from cmd and call another func.

If it's not common enough (e.g., used a lot by other commands as well), then we should remove this helper and instead prefer calling volume.Volumes directly.

volStore, err := getVolumeStore(cmd)
ns, err := cmd.Flags().GetString("namespace")
if err != nil {
return nil, err
}
volumeSize, err := cmd.Flags().GetBool("size")
dataRoot, err := cmd.Flags().GetString("data-root")
if err != nil {
return nil, err
}
return volStore.List(volumeSize)
}

func getVolumeFilterFuncs(filters []string) ([]func(*map[string]string) bool, []func(string) bool, []func(int64) bool, bool, error) {
isFilter := len(filters) > 0
labelFilterFuncs := make([]func(*map[string]string) bool, 0)
nameFilterFuncs := make([]func(string) bool, 0)
sizeFilterFuncs := make([]func(int64) bool, 0)
sizeOperators := []struct {
Operand string
Compare func(int64, int64) bool
}{
{">=", func(size, volumeSize int64) bool {
return volumeSize >= size
}},
{"<=", func(size, volumeSize int64) bool {
return volumeSize <= size
}},
{">", func(size, volumeSize int64) bool {
return volumeSize > size
}},
{"<", func(size, volumeSize int64) bool {
return volumeSize < size
}},
{"=", func(size, volumeSize int64) bool {
return volumeSize == size
}},
}
for _, filter := range filters {
if strings.HasPrefix(filter, "name") || strings.HasPrefix(filter, "label") {
subs := strings.SplitN(filter, "=", 2)
if len(subs) < 2 {
continue
}
switch subs[0] {
case "name":
nameFilterFuncs = append(nameFilterFuncs, func(name string) bool {
return strings.Contains(name, subs[1])
})
case "label":
v, k, hasValue := "", subs[1], false
if subs := strings.SplitN(subs[1], "=", 2); len(subs) == 2 {
hasValue = true
k, v = subs[0], subs[1]
}
labelFilterFuncs = append(labelFilterFuncs, func(labels *map[string]string) bool {
if labels == nil {
return false
}
val, ok := (*labels)[k]
if !ok || (hasValue && val != v) {
return false
}
return true
})
}
continue
}
if strings.HasPrefix(filter, "size") {
for _, sizeOperator := range sizeOperators {
if subs := strings.SplitN(filter, sizeOperator.Operand, 2); len(subs) == 2 {
v, err := strconv.Atoi(subs[1])
if err != nil {
return nil, nil, nil, false, err
}
sizeFilterFuncs = append(sizeFilterFuncs, func(size int64) bool {
return sizeOperator.Compare(int64(v), size)
})
break
}
}
continue
}
}
return labelFilterFuncs, nameFilterFuncs, sizeFilterFuncs, isFilter, nil
}

func volumeMatchesFilter(vol native.Volume, labelFilterFuncs []func(*map[string]string) bool, nameFilterFuncs []func(string) bool, sizeFilterFuncs []func(int64) bool) bool {
for _, labelFilterFunc := range labelFilterFuncs {
if !labelFilterFunc(vol.Labels) {
return false
}
}
for _, nameFilterFunc := range nameFilterFuncs {
if !nameFilterFunc(vol.Name) {
return false
}
address, err := cmd.Flags().GetString("address")
if err != nil {
return nil, err
}
for _, sizeFilterFunc := range sizeFilterFuncs {
if !sizeFilterFunc(vol.Size) {
return false
}
volumeSize, err := cmd.Flags().GetBool("size")
if err != nil {
return nil, err
}
return true
return volume.Volumes(ns, dataRoot, address, volumeSize)
}
38 changes: 38 additions & 0 deletions pkg/api/types/volume_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
Copyright The containerd 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 types

import "io"

type VolumeLsCommandOptions struct {
// Writer is the output writer
Writer io.Writer
// Only display volume names
Quiet bool
// Format the output using the given go template
Format string
// Display the disk usage of volumes. Can be slow with volumes having loads of directories.
Size bool
// Filter matches volumes based on given conditions
Filters []string
// containerd namespace
Namespace string
// Root directory of persistent nerdctl state
DataRoot string
// containerd address
Address string
}
Loading