From 53a89a416703cd19c5addc4e8719f2752866c376 Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Wed, 20 Mar 2024 14:04:57 +0100 Subject: [PATCH 01/12] Improve list function with more control over options --- .github/workflows/test.yaml | 4 +- Dockerfile | 14 ++++- zfs.go | 121 ++++++++++++++++++++---------------- zfs_test.go | 64 +++++++++++++------ 4 files changed, 125 insertions(+), 78 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 1dd2a28..1797f17 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -4,14 +4,14 @@ jobs: test: strategy: matrix: - go-version: [1.21.x] + go-version: [1.21.x, 1.22.x] os: [ubuntu-latest] runs-on: ${{ matrix.os }} steps: - name: Install Linux packages if: matrix.os == 'ubuntu-latest' - run: sudo add-apt-repository ppa:jonathonf/zfs && sudo apt-get update && sudo apt install -y --no-install-recommends zfsutils-linux + run: sudo apt install -y --no-install-recommends zfsutils-linux - name: Install Go uses: actions/setup-go@v4 diff --git a/Dockerfile b/Dockerfile index 660b766..339da40 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,15 @@ -FROM ubuntu:bionic AS golang-zfs +FROM ubuntu:jammy AS golang-zfs # Install zfsutils -RUN apt-get update && apt-get install -y --no-install-recommends \ +RUN apt-get update \ + && apt-get install -y \ + software-properties-common \ + sudo \ + && add-apt-repository ppa:longsleep/golang-backports \ + && apt-get update \ + && apt-get install -y --no-install-recommends \ + golang-go \ zfsutils-linux \ - && rm -rf /var/lib/apt/lists/* + && rm -rf /var/lib/apt/lists/* \ + && /sbin/modprobe zfs diff --git a/zfs.go b/zfs.go index da32b7c..fb1b4cc 100644 --- a/zfs.go +++ b/zfs.go @@ -15,14 +15,68 @@ const ( Binary = "zfs" ) -// ListByType lists the datasets by type and allows you to fetch extra custom fields -func ListByType(ctx context.Context, t DatasetType, filter string, extraProps ...string) ([]Dataset, error) { - allFields := append(dsPropList, extraProps...) // nolint: gocritic +// PropertySource specifies the source of a property +type PropertySource string +const ( + PropertySourceLocal PropertySource = "local" + PropertySourceDefault = "default" + PropertySourceInherited = "inherited" + PropertySourceTemporary = "temporary" + PropertySourceReceived = "received" + PropertySourceNone = "none" +) + +// ListOptions are options you can specify to customize the list command +type ListOptions struct { + // ParentDataset filters by parent dataset, empty lists all + ParentDataset string + // DatasetType filters the results by type + DatasetType DatasetType + // ExtraProperties lists the properties to retrieve besides the ones in the Dataset struct (in the ExtraProps key) + ExtraProperties []string + // Recursive, if true will list all under the parent dataset + Recursive bool + // Depth specifies the depth to go below the parent dataset (or root if no parent) + Depth int + // PropertySources is a list of sources to display. Those properties coming from a source other than those in this + // list are ignored + PropertySources []PropertySource +} + +func (lo ListOptions) propertySourceStrings() []string { + sl := make([]string, len(lo.PropertySources)) + for i, ps := range lo.PropertySources { + sl[i] = string(ps) + } + return sl +} + +// ListDatasets lists the datasets by type and allows you to fetch extra custom fields +func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { + allFields := append(dsPropList, options.ExtraProperties...) // nolint: gocritic dsPropListOptions := strings.Join(allFields, ",") - args := []string{"list", "-rHp", "-t", string(t), "-o", dsPropListOptions} - if filter != "" { - args = append(args, filter) + + args := make([]string, 0, 16) + args = append(args, "list", "-Hp", "-o", dsPropListOptions) + if options.DatasetType != "" { + args = append(args, "-t", string(options.DatasetType)) + } + + if options.Recursive { + args = append(args, "-r") + } + + if options.Depth > 0 { + args = append(args, "-d", strconv.Itoa(options.Depth)) + } + + if len(options.PropertySources) > 0 { + args = append(args, "-s", strings.Join(options.propertySourceStrings(), ",")) + } + + if options.ParentDataset != "" { + args = append(args, options.ParentDataset) } out, err := zfsOutput(ctx, args...) @@ -36,7 +90,7 @@ func ListByType(ctx context.Context, t DatasetType, filter string, extraProps .. } for _, fields := range out { - ds, err := datasetFromFields(fields, extraProps) + ds, err := datasetFromFields(fields, options.ExtraProperties) if err != nil { return datasets, err } @@ -46,52 +100,6 @@ func ListByType(ctx context.Context, t DatasetType, filter string, extraProps .. return datasets, nil } -// Datasets returns a slice of ZFS datasets, regardless of type. -// A filter argument may be passed to select a dataset with the matching name, or empty string ("") may be used to select all datasets. -func Datasets(ctx context.Context, filter string, extraProperties ...string) ([]Dataset, error) { - return ListByType(ctx, DatasetAll, filter, extraProperties...) -} - -// Snapshots returns a slice of ZFS snapshots. -// A filter argument may be passed to select a snapshot with the matching name, or empty string ("") may be used to select all snapshots. -func Snapshots(ctx context.Context, filter string, extraProperties ...string) ([]Dataset, error) { - return ListByType(ctx, DatasetSnapshot, filter, extraProperties...) -} - -// Filesystems returns a slice of ZFS filesystems. -// A filter argument may be passed to select a filesystem with the matching name, or empty string ("") may be used to select all filesystems. -func Filesystems(ctx context.Context, filter string, extraProperties ...string) ([]Dataset, error) { - return ListByType(ctx, DatasetFilesystem, filter, extraProperties...) -} - -// Volumes returns a slice of ZFS volumes. -// A filter argument may be passed to select a volume with the matching name, or empty string ("") may be used to select all volumes. -func Volumes(ctx context.Context, filter string, extraProperties ...string) ([]Dataset, error) { - return ListByType(ctx, DatasetVolume, filter, extraProperties...) -} - -// ListWithProperty returns a map of dataset names mapped to the properties value for datasets which have the given ZFS property. -func ListWithProperty(ctx context.Context, t DatasetType, filter, prop string) (map[string]string, error) { - c := command{ - cmd: Binary, - ctx: ctx, - } - lines, err := c.Run("get", "-t", string(t), "-Hp", "-o", "name,value", "-r", "-s", "local", prop, filter) - if err != nil { - return nil, err - } - result := make(map[string]string, len(lines)) - for _, line := range lines { - switch len(line) { - case 2: - result[line[0]] = line[1] - case 1: - result[line[0]] = PropertyUnset - } - } - return result, nil -} - // GetDataset retrieves a single ZFS dataset by name. // This dataset could be any valid ZFS dataset type, such as a clone, filesystem, snapshot, or volume. func GetDataset(ctx context.Context, name string, extraProperties ...string) (*Dataset, error) { @@ -581,8 +589,11 @@ func (d *Dataset) Rename(ctx context.Context, name string, options RenameOptions } // Snapshots returns a slice of all ZFS snapshots of a given dataset. -func (d *Dataset) Snapshots(ctx context.Context, extraProperties ...string) ([]Dataset, error) { - return Snapshots(ctx, d.Name, extraProperties...) +func (d *Dataset) Snapshots(ctx context.Context, options ListOptions) ([]Dataset, error) { + options.ParentDataset = d.Name + options.DatasetType = DatasetSnapshot + options.Recursive = true + return ListDatasets(ctx, options) } // CreateFilesystemOptions are options you can specify to customize the create filesystem command diff --git a/zfs_test.go b/zfs_test.go index 9bbe2eb..fec7d89 100644 --- a/zfs_test.go +++ b/zfs_test.go @@ -18,7 +18,7 @@ func TestDatasets(t *testing.T) { t.Helper() TestZPool(testZPool, func() { - _, err := Datasets(context.Background(), "") + _, err := ListDatasets(context.Background(), ListOptions{}) require.NoError(t, err) ds, err := GetDataset(context.Background(), testZPool) @@ -85,7 +85,7 @@ func TestDatasetSetInheritProperty(t *testing.T) { func TestSnapshots(t *testing.T) { TestZPool(testZPool, func() { - snapshots, err := Snapshots(context.Background(), "") + snapshots, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetSnapshot}) require.NoError(t, err) for _, snapshot := range snapshots { @@ -101,7 +101,7 @@ func TestFilesystems(t *testing.T) { }) require.NoError(t, err) - filesystems, err := Filesystems(context.Background(), "") + filesystems, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetFilesystem}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -123,7 +123,7 @@ func TestCreateFilesystemWithProperties(t *testing.T) { require.NoError(t, err) require.Equal(t, "lz4", f.Compression) - filesystems, err := Filesystems(context.Background(), "") + filesystems, err := ListDatasets(context.Background(), ListOptions{}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -143,7 +143,7 @@ func TestVolumes(t *testing.T) { time.Sleep(time.Second) require.Equal(t, DatasetVolume, v.Type) - volumes, err := Volumes(context.Background(), "") + volumes, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetVolume}) require.NoError(t, err) for _, volume := range volumes { @@ -161,7 +161,7 @@ func TestSnapshot(t *testing.T) { }) require.NoError(t, err) - filesystems, err := Filesystems(context.Background(), "") + filesystems, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetFilesystem}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -183,18 +183,38 @@ func TestListWithProperty(t *testing.T) { TestZPool(testZPool, func() { const prop = "nl.test:bla" - f, err := CreateFilesystem(context.Background(), testZPool+"/list-test", CreateFilesystemOptions{ + f1, err := CreateFilesystem(context.Background(), testZPool+"/list-test", CreateFilesystemOptions{ Properties: noMountProps, }) require.NoError(t, err) - require.NoError(t, f.SetProperty(context.Background(), prop, "123")) + require.NoError(t, f1.SetProperty(context.Background(), prop, "123")) - ls, err := ListWithProperty(context.Background(), DatasetFilesystem, testZPool+"/list-test", prop) + f2, err := CreateFilesystem(context.Background(), testZPool+"/list-2", CreateFilesystemOptions{ + Properties: noMountProps, + }) require.NoError(t, err) - require.Len(t, ls, 1) - require.Equal(t, map[string]string{ - testZPool + "/list-test": "123", - }, ls) + require.NoError(t, f2.SetProperty(context.Background(), prop, "321")) + + _, err = CreateFilesystem(context.Background(), testZPool+"/list-3", CreateFilesystemOptions{ + Properties: noMountProps, + }) + require.NoError(t, err) + + ls, err := ListDatasets(context.Background(), ListOptions{ + ParentDataset: testZPool, + DatasetType: DatasetFilesystem, + ExtraProperties: []string{prop}, + Recursive: true, + PropertySources: []PropertySource{PropertySourceLocal}, + }) + require.NoError(t, err) + require.Len(t, ls, 2) + + require.Equal(t, f1.Name, ls[0].Name) + require.Equal(t, "123", ls[0].ExtraProps[prop]) + + require.Equal(t, f2.Name, ls[1].Name) + require.Equal(t, "321", ls[1].ExtraProps[prop]) }) } @@ -205,7 +225,7 @@ func TestClone(t *testing.T) { }) require.NoError(t, err) - filesystems, err := Filesystems(context.Background(), "") + filesystems, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetFilesystem}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -236,7 +256,7 @@ func TestSendSnapshot(t *testing.T) { }) require.NoError(t, err) - filesystems, err := Filesystems(context.Background(), "") + filesystems, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetFilesystem}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -279,7 +299,11 @@ func TestSendSnapshotResume(t *testing.T) { require.True(t, errors.As(err, &zfsErr)) require.NotEmpty(t, zfsErr.ResumeToken(), zfsErr) - list, err := Filesystems(context.Background(), testZPool+"/recv-test", PropertyReceiveResumeToken) + list, err := ListDatasets(context.Background(), ListOptions{ + DatasetType: DatasetFilesystem, + ParentDataset: testZPool + "/recv-test", + ExtraProperties: []string{PropertyReceiveResumeToken}, + }) require.NoError(t, err) require.Len(t, list, 1) require.Len(t, list[0].ExtraProps, 1) @@ -303,7 +327,11 @@ func TestSendSnapshotResume(t *testing.T) { }) require.NoError(t, err) - snaps, err := Snapshots(context.Background(), testZPool+"/recv-test") + snaps, err := ListDatasets(context.Background(), ListOptions{ + DatasetType: DatasetSnapshot, + ParentDataset: testZPool + "/recv-test", + Recursive: true, + }) require.NoError(t, err) require.Len(t, snaps, 1) require.Equal(t, snaps[0].Name, testZPool+"/recv-test@test") @@ -343,7 +371,7 @@ func TestRollback(t *testing.T) { }) require.NoError(t, err) - filesystems, err := Filesystems(context.Background(), "") + filesystems, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetFilesystem}) require.NoError(t, err) for _, filesystem := range filesystems { From c8753c786fa61f554a022bd6db4addc0f642ede4 Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Wed, 20 Mar 2024 14:20:48 +0100 Subject: [PATCH 02/12] Use zfs get --- Dockerfile | 3 +-- zfs.go | 9 +++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Dockerfile b/Dockerfile index 339da40..d928200 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,6 +10,5 @@ RUN apt-get update \ && apt-get install -y --no-install-recommends \ golang-go \ zfsutils-linux \ - && rm -rf /var/lib/apt/lists/* \ - && /sbin/modprobe zfs + && rm -rf /var/lib/apt/lists/* diff --git a/zfs.go b/zfs.go index fb1b4cc..18e382e 100644 --- a/zfs.go +++ b/zfs.go @@ -54,11 +54,8 @@ func (lo ListOptions) propertySourceStrings() []string { // ListDatasets lists the datasets by type and allows you to fetch extra custom fields func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { - allFields := append(dsPropList, options.ExtraProperties...) // nolint: gocritic - dsPropListOptions := strings.Join(allFields, ",") - args := make([]string, 0, 16) - args = append(args, "list", "-Hp", "-o", dsPropListOptions) + args = append(args, "get", "-Hp") if options.DatasetType != "" { args = append(args, "-t", string(options.DatasetType)) } @@ -75,6 +72,10 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { args = append(args, "-s", strings.Join(options.propertySourceStrings(), ",")) } + allFields := append(dsPropList, options.ExtraProperties...) // nolint: gocritic + dsPropListOptions := strings.Join(allFields, ",") + args = append(args, dsPropListOptions) + if options.ParentDataset != "" { args = append(args, options.ParentDataset) } From 3749fb1867c31ce47a467207184e4686303649ff Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Wed, 20 Mar 2024 16:10:41 +0100 Subject: [PATCH 03/12] Rewrite output parsing for zfs get instead of zfs list --- dataset.go | 130 +++++++++++++++++++++++++++--------------------- dataset_test.go | 74 +++++++++++++++++++++++++++ utils.go | 10 ++-- zfs.go | 67 ++++++------------------- zfs_test.go | 4 +- 5 files changed, 169 insertions(+), 116 deletions(-) create mode 100644 dataset_test.go diff --git a/dataset.go b/dataset.go index af69662..fc64f16 100644 --- a/dataset.go +++ b/dataset.go @@ -3,6 +3,7 @@ package zfs import ( "fmt" "strconv" + "strings" ) // DatasetType is the zfs dataset type @@ -26,7 +27,7 @@ type Dataset struct { Type DatasetType `json:"Type"` Origin string `json:"Origin"` Used uint64 `json:"Used"` - Avail uint64 `json:"Avail"` + Available uint64 `json:"Available"` Mountpoint string `json:"Mountpoint"` Compression string `json:"Compression"` Written uint64 `json:"Written"` @@ -34,85 +35,98 @@ type Dataset struct { Logicalused uint64 `json:"Logicalused"` Usedbydataset uint64 `json:"Usedbydataset"` Quota uint64 `json:"Quota"` + Refquota uint64 `json:"Refquota"` Referenced uint64 `json:"Referenced"` ExtraProps map[string]string `json:"ExtraProps"` } -func datasetFromFields(fields, extraProps []string) (*Dataset, error) { - if len(fields) != len(dsPropList)+len(extraProps) { - return nil, fmt.Errorf("output invalid: %d fields where %d were expected", len(fields), len(dsPropList)+len(extraProps)) - } +const ( + nameField = iota + propertyField + valueField +) - d := &Dataset{ - Name: fields[0], - Type: DatasetType(fields[1]), +func readDatasets(output [][]string, extraProps []string) ([]Dataset, error) { + multiple := len(dsPropList) + len(extraProps) + if len(output)%multiple != 0 { + return nil, fmt.Errorf("output invalid: %d lines where a multiple of %d was expected", len(output), multiple) } - fields = setString(&d.Origin, fields[2:]) - fields, err := setUint(&d.Used, fields) - if err != nil { - return nil, err - } - fields, err = setUint(&d.Avail, fields) - if err != nil { - return nil, err - } - fields = setString(&d.Mountpoint, fields) - fields = setString(&d.Compression, fields) - fields, err = setUint(&d.Volsize, fields) - if err != nil { - return nil, err - } - fields, err = setUint(&d.Quota, fields) - if err != nil { - return nil, err - } - fields, err = setUint(&d.Referenced, fields) - if err != nil { - return nil, err - } - fields, err = setUint(&d.Written, fields) - if err != nil { - return nil, err - } - fields, err = setUint(&d.Logicalused, fields) - if err != nil { - return nil, err - } - fields, err = setUint(&d.Usedbydataset, fields) - if err != nil { - return nil, err - } + count := len(output) / (len(dsPropList) + len(extraProps)) + curDataset := 0 + datasets := make([]Dataset, count) + for i, fields := range output { + if len(fields) != 3 { + return nil, fmt.Errorf("output contains line with %d fields: %s", len(fields), strings.Join(fields, " ")) + } + + if i > 0 && fields[nameField] != datasets[curDataset].Name { + curDataset++ + } + + ds := &datasets[curDataset] + ds.ExtraProps = make(map[string]string, len(extraProps)) + ds.Name = fields[nameField] - d.ExtraProps = make(map[string]string, len(extraProps)) - for i, field := range extraProps { - d.ExtraProps[field] = fields[i] + val := fields[valueField] + + var setError error + switch fields[propertyField] { + case PropertyName: + ds.Name = val + case PropertyType: + ds.Type = DatasetType(val) + case PropertyOrigin: + setString(&ds.Origin, val) + case PropertyUsed: + setError = setUint(&ds.Used, val) + case PropertyAvailable: + setError = setUint(&ds.Available, val) + case PropertyMountPoint: + setString(&ds.Mountpoint, val) + case PropertyCompression: + setString(&ds.Compression, val) + case PropertyWritten: + setError = setUint(&ds.Written, val) + case PropertyVolSize: + setError = setUint(&ds.Volsize, val) + case PropertyLogicalUsed: + setError = setUint(&ds.Logicalused, val) + case PropertyUsedByDataset: + setError = setUint(&ds.Usedbydataset, val) + case PropertyQuota: + setError = setUint(&ds.Quota, val) + case PropertyRefQuota: + setError = setUint(&ds.Refquota, val) + case PropertyReferenced: + setError = setUint(&ds.Referenced, val) + default: + ds.ExtraProps[fields[propertyField]] = val + } + if setError != nil { + return nil, fmt.Errorf("error in dataset %d field %s [%s]: %w", curDataset, fields[propertyField], fields[valueField], setError) + } } - return d, nil + return datasets, nil } -func setString(field *string, values []string) []string { - val, values := values[0], values[1:] +func setString(field *string, val string) { if val == PropertyUnset { - return values + return } *field = val - return values } -func setUint(field *uint64, values []string) ([]string, error) { - var val string - val, values = values[0], values[1:] +func setUint(field *uint64, val string) error { if val == PropertyUnset { - return values, nil + return nil } v, err := strconv.ParseUint(val, 10, 64) if err != nil { - return values, err + return err } - *field = v - return values, nil + return nil } diff --git a/dataset_test.go b/dataset_test.go new file mode 100644 index 0000000..7f3ad3c --- /dev/null +++ b/dataset_test.go @@ -0,0 +1,74 @@ +package zfs + +import ( + "github.com/stretchr/testify/require" + "testing" +) + +func Test_readDatasets(t *testing.T) { + in := splitOutput(testInput) + + const prop = "nl.test:hiephoi" + ds, err := readDatasets(in, []string{prop}) + require.NoError(t, err) + require.Len(t, ds, 3) + require.Equal(t, ds[0].Name, "testpool/ds0") + require.Equal(t, ds[1].Name, "testpool/ds1") + require.Equal(t, ds[2].Name, "testpool/ds10") + + for i := range ds { + require.NotEmpty(t, ds[i].Name) + require.NotEmpty(t, ds[i].Mountpoint) + require.NotZero(t, ds[i].Referenced) + require.NotZero(t, ds[i].Used) + require.NotZero(t, ds[i].Available) + require.Equal(t, "42", ds[i].ExtraProps[prop]) + } +} + +const testInput = `testpool/ds0 name testpool/ds0 +testpool/ds0 type filesystem +testpool/ds0 origin - +testpool/ds0 used 196416 +testpool/ds0 available 186368146928528 +testpool/ds0 mountpoint none +testpool/ds0 compression off +testpool/ds0 volsize - +testpool/ds0 quota 0 +testpool/ds0 refquota 0 +testpool/ds0 referenced 196416 +testpool/ds0 written 196416 +testpool/ds0 logicalused 43520 +testpool/ds0 usedbydataset 196416 +testpool/ds0 nl.test:hiephoi 42 +testpool/ds1 name testpool/ds1 +testpool/ds1 type filesystem +testpool/ds1 origin - +testpool/ds1 used 196416 +testpool/ds1 available 186368146928528 +testpool/ds1 mountpoint none +testpool/ds1 compression off +testpool/ds1 volsize - +testpool/ds1 quota 0 +testpool/ds1 refquota 0 +testpool/ds1 referenced 196416 +testpool/ds1 written 196416 +testpool/ds1 logicalused 43520 +testpool/ds1 usedbydataset 196416 +testpool/ds1 nl.test:hiephoi 42 +testpool/ds10 name testpool/ds10 +testpool/ds10 type filesystem +testpool/ds10 origin - +testpool/ds10 used 196416 +testpool/ds10 available 186368146928528 +testpool/ds10 mountpoint none +testpool/ds10 compression off +testpool/ds10 volsize - +testpool/ds10 quota 0 +testpool/ds10 refquota 0 +testpool/ds10 referenced 196416 +testpool/ds10 written 196416 +testpool/ds10 logicalused 43520 +testpool/ds10 usedbydataset 196416 +testpool/ds10 nl.test:hiephoi 42 +` diff --git a/utils.go b/utils.go index 6df0c5c..da9e7db 100644 --- a/utils.go +++ b/utils.go @@ -20,6 +20,7 @@ var dsPropList = []string{ PropertyCompression, PropertyVolSize, PropertyQuota, + PropertyRefQuota, PropertyReferenced, PropertyWritten, PropertyLogicalUsed, @@ -75,7 +76,11 @@ func (c *command) Run(arg ...string) ([][]string, error) { return nil, nil } - lines := strings.Split(stdout.String(), "\n") + return splitOutput(stdout.String()), nil +} + +func splitOutput(out string) [][]string { + lines := strings.Split(out, "\n") // last line is always blank lines = lines[0 : len(lines)-1] @@ -83,8 +88,7 @@ func (c *command) Run(arg ...string) ([][]string, error) { for i, l := range lines { output[i] = strings.Split(l, fieldSeparator) } - - return output, nil + return output } func propsSlice(properties map[string]string) []string { diff --git a/zfs.go b/zfs.go index 18e382e..c65cafb 100644 --- a/zfs.go +++ b/zfs.go @@ -55,7 +55,7 @@ func (lo ListOptions) propertySourceStrings() []string { // ListDatasets lists the datasets by type and allows you to fetch extra custom fields func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { args := make([]string, 0, 16) - args = append(args, "get", "-Hp") + args = append(args, "get", "-Hp", "-o", "name,property,value") if options.DatasetType != "" { args = append(args, "-t", string(options.DatasetType)) } @@ -85,36 +85,25 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { return nil, err } - datasets := make([]Dataset, 0, len(out)) - if len(out) == 0 { - return datasets, nil - } - - for _, fields := range out { - ds, err := datasetFromFields(fields, options.ExtraProperties) - if err != nil { - return datasets, err - } - datasets = append(datasets, *ds) - } - - return datasets, nil + return readDatasets(out, options.ExtraProperties) } // GetDataset retrieves a single ZFS dataset by name. // This dataset could be any valid ZFS dataset type, such as a clone, filesystem, snapshot, or volume. func GetDataset(ctx context.Context, name string, extraProperties ...string) (*Dataset, error) { - fields := append(dsPropList, extraProperties...) // nolint: gocritic - out, err := zfsOutput(ctx, "list", "-Hp", "-o", strings.Join(fields, ","), name) + ds, err := ListDatasets(ctx, ListOptions{ + ParentDataset: name, + Recursive: false, + ExtraProperties: extraProperties, + }) if err != nil { return nil, err } - if len(out) > 1 { - return nil, fmt.Errorf("more output than expected: %v", out) + if len(ds) > 1 { + return nil, fmt.Errorf("more datasets than expected: %d", len(ds)) } - - return datasetFromFields(out[0], extraProperties) + return &ds[0], nil } // CloneOptions are options you can specify to customize the clone command @@ -726,36 +715,8 @@ func (d *Dataset) Rollback(ctx context.Context, options RollbackOptions) error { // Children returns a slice of children of the receiving ZFS dataset. // A recursion depth may be specified, or a depth of 0 allows unlimited recursion. -func (d *Dataset) Children(ctx context.Context, depth uint64, extraProperties ...string) ([]Dataset, error) { - allFields := append(dsPropList, extraProperties...) // nolint: gocritic - - args := make([]string, 1, 16) - args[0] = "list" - if depth > 0 { - args = append(args, "-d") - args = append(args, strconv.FormatUint(depth, 10)) - } else { - args = append(args, "-r") - } - args = append(args, "-t", "all", "-Hp", "-o", strings.Join(allFields, ",")) - args = append(args, d.Name) - - out, err := zfsOutput(ctx, args...) - if err != nil { - return nil, err - } - - datasets := make([]Dataset, 0, len(out)-1) - for i, fields := range out { - if i == 0 { // Skip the first parent entry, because we are looking for its children - continue - } - - ds, err := datasetFromFields(fields, extraProperties) - if err != nil { - return nil, err - } - datasets = append(datasets, *ds) - } - return datasets, nil +func (d *Dataset) Children(ctx context.Context, options ListOptions) ([]Dataset, error) { + options.ParentDataset = d.Name + options.Recursive = true + return ListDatasets(ctx, options) } diff --git a/zfs_test.go b/zfs_test.go index fec7d89..9edebad 100644 --- a/zfs_test.go +++ b/zfs_test.go @@ -351,13 +351,13 @@ func TestChildren(t *testing.T) { require.Equal(t, DatasetSnapshot, s.Type) require.Equal(t, testZPool+"/snapshot-test@test", s.Name) - children, err := f.Children(context.Background(), 0, PropertyRefQuota) + children, err := f.Children(context.Background(), ListOptions{Depth: 0, ExtraProperties: []string{PropertyMounted}}) require.NoError(t, err) require.Equal(t, 1, len(children)) require.Equal(t, testZPool+"/snapshot-test@test", children[0].Name) require.Len(t, children[0].ExtraProps, 1) - require.Equal(t, children[0].ExtraProps, map[string]string{PropertyRefQuota: PropertyUnset}) + require.Equal(t, children[0].ExtraProps, map[string]string{PropertyMounted: PropertyUnset}) require.NoError(t, s.Destroy(context.Background(), DestroyOptions{})) require.NoError(t, f.Destroy(context.Background(), DestroyOptions{})) From 10580389d0638136c9a3b66b68d8548b364e0143 Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Wed, 20 Mar 2024 16:21:51 +0100 Subject: [PATCH 04/12] Fix some tests --- dataset.go | 15 +++++++++++---- dataset_test.go | 1 + zfs.go | 11 ++++++++++- zfs_test.go | 23 +++++++++++++---------- 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/dataset.go b/dataset.go index fc64f16..c1dd00f 100644 --- a/dataset.go +++ b/dataset.go @@ -49,7 +49,9 @@ const ( func readDatasets(output [][]string, extraProps []string) ([]Dataset, error) { multiple := len(dsPropList) + len(extraProps) if len(output)%multiple != 0 { - return nil, fmt.Errorf("output invalid: %d lines where a multiple of %d was expected", len(output), multiple) + return nil, fmt.Errorf("output invalid: %d lines where a multiple of %d was expected: %s", + len(output), multiple, strings.Join(output[0], " "), + ) } count := len(output) / (len(dsPropList) + len(extraProps)) @@ -68,10 +70,11 @@ func readDatasets(output [][]string, extraProps []string) ([]Dataset, error) { ds.ExtraProps = make(map[string]string, len(extraProps)) ds.Name = fields[nameField] + prop := fields[propertyField] val := fields[valueField] var setError error - switch fields[propertyField] { + switch prop { case PropertyName: ds.Name = val case PropertyType: @@ -101,10 +104,14 @@ func readDatasets(output [][]string, extraProps []string) ([]Dataset, error) { case PropertyReferenced: setError = setUint(&ds.Referenced, val) default: - ds.ExtraProps[fields[propertyField]] = val + if val == PropertyUnset { + ds.ExtraProps[prop] = "" + continue + } + ds.ExtraProps[prop] = val } if setError != nil { - return nil, fmt.Errorf("error in dataset %d field %s [%s]: %w", curDataset, fields[propertyField], fields[valueField], setError) + return nil, fmt.Errorf("error in dataset %d (%s) field %s [%s]: %w", curDataset, ds.Name, prop, val, setError) } } diff --git a/dataset_test.go b/dataset_test.go index 7f3ad3c..26b518a 100644 --- a/dataset_test.go +++ b/dataset_test.go @@ -17,6 +17,7 @@ func Test_readDatasets(t *testing.T) { require.Equal(t, ds[2].Name, "testpool/ds10") for i := range ds { + require.Equal(t, "", ds[i].Origin) require.NotEmpty(t, ds[i].Name) require.NotEmpty(t, ds[i].Mountpoint) require.NotZero(t, ds[i].Referenced) diff --git a/zfs.go b/zfs.go index c65cafb..17bb516 100644 --- a/zfs.go +++ b/zfs.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "io" + "slices" "strconv" "strings" @@ -69,6 +70,9 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { } if len(options.PropertySources) > 0 { + if !slices.Contains(options.PropertySources, PropertySourceNone) { + options.PropertySources = append(options.PropertySources, PropertySourceNone) + } args = append(args, "-s", strings.Join(options.propertySourceStrings(), ",")) } @@ -718,5 +722,10 @@ func (d *Dataset) Rollback(ctx context.Context, options RollbackOptions) error { func (d *Dataset) Children(ctx context.Context, options ListOptions) ([]Dataset, error) { options.ParentDataset = d.Name options.Recursive = true - return ListDatasets(ctx, options) + ds, err := ListDatasets(ctx, options) + if err != nil { + return nil, err + } + // Skip the first parent entry, because we are looking for its children + return ds[1:], nil } diff --git a/zfs_test.go b/zfs_test.go index 9edebad..bb55b9b 100644 --- a/zfs_test.go +++ b/zfs_test.go @@ -31,12 +31,12 @@ func TestDatasets(t *testing.T) { func TestDatasetsWithProps(t *testing.T) { TestZPool(testZPool, func() { - ds, err := GetDataset(context.Background(), testZPool, "name", "refquota") + ds, err := GetDataset(context.Background(), testZPool, "readonly", "canmount") require.NoError(t, err) require.Len(t, ds.ExtraProps, 2) - require.Equal(t, ds.ExtraProps["name"], testZPool) - require.Equal(t, ds.ExtraProps["refquota"], "0") + require.Equal(t, "on", ds.ExtraProps["readonly"]) + require.Equal(t, "off", ds.ExtraProps["canmount"]) }) } @@ -200,7 +200,7 @@ func TestListWithProperty(t *testing.T) { }) require.NoError(t, err) - ls, err := ListDatasets(context.Background(), ListOptions{ + ds, err := ListDatasets(context.Background(), ListOptions{ ParentDataset: testZPool, DatasetType: DatasetFilesystem, ExtraProperties: []string{prop}, @@ -208,13 +208,13 @@ func TestListWithProperty(t *testing.T) { PropertySources: []PropertySource{PropertySourceLocal}, }) require.NoError(t, err) - require.Len(t, ls, 2) + require.Len(t, ds, 2) - require.Equal(t, f1.Name, ls[0].Name) - require.Equal(t, "123", ls[0].ExtraProps[prop]) + require.Equal(t, f1.Name, ds[0].Name) + require.Equal(t, "123", ds[0].ExtraProps[prop]) - require.Equal(t, f2.Name, ls[1].Name) - require.Equal(t, "321", ls[1].ExtraProps[prop]) + require.Equal(t, f2.Name, ds[1].Name) + require.Equal(t, "321", ds[1].ExtraProps[prop]) }) } @@ -351,7 +351,10 @@ func TestChildren(t *testing.T) { require.Equal(t, DatasetSnapshot, s.Type) require.Equal(t, testZPool+"/snapshot-test@test", s.Name) - children, err := f.Children(context.Background(), ListOptions{Depth: 0, ExtraProperties: []string{PropertyMounted}}) + children, err := f.Children(context.Background(), ListOptions{ + DatasetType: DatasetSnapshot, + ExtraProperties: []string{PropertyMounted}, + }) require.NoError(t, err) require.Equal(t, 1, len(children)) From 1662b2ce7f30e77177637d09a96ec79eecb64bd6 Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Wed, 20 Mar 2024 16:50:56 +0100 Subject: [PATCH 05/12] Remove property source --- zfs.go | 38 +------------------------------------- zfs_test.go | 1 - 2 files changed, 1 insertion(+), 38 deletions(-) diff --git a/zfs.go b/zfs.go index 17bb516..f4d3b72 100644 --- a/zfs.go +++ b/zfs.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "io" - "slices" "strconv" "strings" @@ -16,18 +15,6 @@ const ( Binary = "zfs" ) -// PropertySource specifies the source of a property -type PropertySource string - -const ( - PropertySourceLocal PropertySource = "local" - PropertySourceDefault = "default" - PropertySourceInherited = "inherited" - PropertySourceTemporary = "temporary" - PropertySourceReceived = "received" - PropertySourceNone = "none" -) - // ListOptions are options you can specify to customize the list command type ListOptions struct { // ParentDataset filters by parent dataset, empty lists all @@ -40,17 +27,6 @@ type ListOptions struct { Recursive bool // Depth specifies the depth to go below the parent dataset (or root if no parent) Depth int - // PropertySources is a list of sources to display. Those properties coming from a source other than those in this - // list are ignored - PropertySources []PropertySource -} - -func (lo ListOptions) propertySourceStrings() []string { - sl := make([]string, len(lo.PropertySources)) - for i, ps := range lo.PropertySources { - sl[i] = string(ps) - } - return sl } // ListDatasets lists the datasets by type and allows you to fetch extra custom fields @@ -69,13 +45,6 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { args = append(args, "-d", strconv.Itoa(options.Depth)) } - if len(options.PropertySources) > 0 { - if !slices.Contains(options.PropertySources, PropertySourceNone) { - options.PropertySources = append(options.PropertySources, PropertySourceNone) - } - args = append(args, "-s", strings.Join(options.propertySourceStrings(), ",")) - } - allFields := append(dsPropList, options.ExtraProperties...) // nolint: gocritic dsPropListOptions := strings.Join(allFields, ",") args = append(args, dsPropListOptions) @@ -722,10 +691,5 @@ func (d *Dataset) Rollback(ctx context.Context, options RollbackOptions) error { func (d *Dataset) Children(ctx context.Context, options ListOptions) ([]Dataset, error) { options.ParentDataset = d.Name options.Recursive = true - ds, err := ListDatasets(ctx, options) - if err != nil { - return nil, err - } - // Skip the first parent entry, because we are looking for its children - return ds[1:], nil + return ListDatasets(ctx, options) } diff --git a/zfs_test.go b/zfs_test.go index bb55b9b..5e24070 100644 --- a/zfs_test.go +++ b/zfs_test.go @@ -205,7 +205,6 @@ func TestListWithProperty(t *testing.T) { DatasetType: DatasetFilesystem, ExtraProperties: []string{prop}, Recursive: true, - PropertySources: []PropertySource{PropertySourceLocal}, }) require.NoError(t, err) require.Len(t, ds, 2) From bccd55c4d65c87aec43718e7e916627cc2c4af7c Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Wed, 20 Mar 2024 17:04:34 +0100 Subject: [PATCH 06/12] Adjust tests --- zfs_test.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/zfs_test.go b/zfs_test.go index 5e24070..4bfd87b 100644 --- a/zfs_test.go +++ b/zfs_test.go @@ -3,6 +3,7 @@ package zfs import ( "context" "errors" + "fmt" "io" "testing" "time" @@ -31,11 +32,11 @@ func TestDatasets(t *testing.T) { func TestDatasetsWithProps(t *testing.T) { TestZPool(testZPool, func() { - ds, err := GetDataset(context.Background(), testZPool, "readonly", "canmount") + ds, err := GetDataset(context.Background(), testZPool, "xattr", "canmount") require.NoError(t, err) - require.Len(t, ds.ExtraProps, 2) - require.Equal(t, "on", ds.ExtraProps["readonly"]) + require.Len(t, ds.ExtraProps, 2, fmt.Sprintf("%#v", ds.ExtraProps)) + require.Equal(t, "off", ds.ExtraProps["xattr"]) require.Equal(t, "off", ds.ExtraProps["canmount"]) }) } @@ -195,11 +196,6 @@ func TestListWithProperty(t *testing.T) { require.NoError(t, err) require.NoError(t, f2.SetProperty(context.Background(), prop, "321")) - _, err = CreateFilesystem(context.Background(), testZPool+"/list-3", CreateFilesystemOptions{ - Properties: noMountProps, - }) - require.NoError(t, err) - ds, err := ListDatasets(context.Background(), ListOptions{ ParentDataset: testZPool, DatasetType: DatasetFilesystem, @@ -207,12 +203,14 @@ func TestListWithProperty(t *testing.T) { Recursive: true, }) require.NoError(t, err) - require.Len(t, ds, 2) + require.Len(t, ds, 3) + + require.Equal(t, testZPool, ds[0].Name) - require.Equal(t, f1.Name, ds[0].Name) + require.Equal(t, f1.Name, ds[1].Name) require.Equal(t, "123", ds[0].ExtraProps[prop]) - require.Equal(t, f2.Name, ds[1].Name) + require.Equal(t, f2.Name, ds[2].Name) require.Equal(t, "321", ds[1].ExtraProps[prop]) }) } @@ -359,7 +357,7 @@ func TestChildren(t *testing.T) { require.Equal(t, 1, len(children)) require.Equal(t, testZPool+"/snapshot-test@test", children[0].Name) require.Len(t, children[0].ExtraProps, 1) - require.Equal(t, children[0].ExtraProps, map[string]string{PropertyMounted: PropertyUnset}) + require.Equal(t, children[0].ExtraProps, map[string]string{PropertyMounted: ""}) require.NoError(t, s.Destroy(context.Background(), DestroyOptions{})) require.NoError(t, f.Destroy(context.Background(), DestroyOptions{})) From 63cf9afe1dfacf27bd3b2710a03fd1113da0c597 Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Wed, 20 Mar 2024 17:19:31 +0100 Subject: [PATCH 07/12] More test improvements, filter parent in Children() --- zfs.go | 36 +++++++++++++++++++++++++++++++++++- zfs_test.go | 37 +++++++++++++++++-------------------- 2 files changed, 52 insertions(+), 21 deletions(-) diff --git a/zfs.go b/zfs.go index f4d3b72..ed030cb 100644 --- a/zfs.go +++ b/zfs.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "io" + "slices" "strconv" "strings" @@ -26,6 +27,8 @@ type ListOptions struct { // Recursive, if true will list all under the parent dataset Recursive bool // Depth specifies the depth to go below the parent dataset (or root if no parent) + // Recursively display any children of the dataset, limiting the recursion to depth. + // A depth of 1 will display only the dataset and its direct children. Depth int } @@ -61,6 +64,27 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { return readDatasets(out, options.ExtraProperties) } +// Volumes returns a slice of ZFS volumes. +// A filter argument may be passed to select a volume with the matching name, or empty string ("") may be used to select all volumes. +func Volumes(ctx context.Context, options ListOptions) ([]Dataset, error) { + options.DatasetType = DatasetVolume + return ListDatasets(ctx, options) +} + +// Filesystems returns a slice of ZFS filesystems. +// A filter argument may be passed to select a filesystem with the matching name, or empty string ("") may be used to select all filesystems. +func Filesystems(ctx context.Context, options ListOptions) ([]Dataset, error) { + options.DatasetType = DatasetFilesystem + return ListDatasets(ctx, options) +} + +// Snapshots returns a slice of ZFS snapshots. +// A filter argument may be passed to select a snapshot with the matching name, or empty string ("") may be used to select all snapshots. +func Snapshots(ctx context.Context, options ListOptions) ([]Dataset, error) { + options.DatasetType = DatasetSnapshot + return ListDatasets(ctx, options) +} + // GetDataset retrieves a single ZFS dataset by name. // This dataset could be any valid ZFS dataset type, such as a clone, filesystem, snapshot, or volume. func GetDataset(ctx context.Context, name string, extraProperties ...string) (*Dataset, error) { @@ -691,5 +715,15 @@ func (d *Dataset) Rollback(ctx context.Context, options RollbackOptions) error { func (d *Dataset) Children(ctx context.Context, options ListOptions) ([]Dataset, error) { options.ParentDataset = d.Name options.Recursive = true - return ListDatasets(ctx, options) + ds, err := ListDatasets(ctx, options) + if err != nil { + return nil, err + } + + // Filter out the parent dataset. + ds = slices.DeleteFunc(ds, func(dataset Dataset) bool { + return dataset.Name == d.Name + }) + + return ds, nil } diff --git a/zfs_test.go b/zfs_test.go index 4bfd87b..00735b6 100644 --- a/zfs_test.go +++ b/zfs_test.go @@ -86,7 +86,7 @@ func TestDatasetSetInheritProperty(t *testing.T) { func TestSnapshots(t *testing.T) { TestZPool(testZPool, func() { - snapshots, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetSnapshot}) + snapshots, err := Snapshots(context.Background(), ListOptions{}) require.NoError(t, err) for _, snapshot := range snapshots { @@ -102,7 +102,7 @@ func TestFilesystems(t *testing.T) { }) require.NoError(t, err) - filesystems, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetFilesystem}) + filesystems, err := Filesystems(context.Background(), ListOptions{}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -144,7 +144,7 @@ func TestVolumes(t *testing.T) { time.Sleep(time.Second) require.Equal(t, DatasetVolume, v.Type) - volumes, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetVolume}) + volumes, err := Volumes(context.Background(), ListOptions{}) require.NoError(t, err) for _, volume := range volumes { @@ -162,7 +162,7 @@ func TestSnapshot(t *testing.T) { }) require.NoError(t, err) - filesystems, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetFilesystem}) + filesystems, err := Filesystems(context.Background(), ListOptions{}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -184,33 +184,32 @@ func TestListWithProperty(t *testing.T) { TestZPool(testZPool, func() { const prop = "nl.test:bla" - f1, err := CreateFilesystem(context.Background(), testZPool+"/list-test", CreateFilesystemOptions{ + root, err := GetDataset(context.Background(), testZPool) + require.NoError(t, err) + + f1, err := CreateFilesystem(context.Background(), testZPool+"/list-test1", CreateFilesystemOptions{ Properties: noMountProps, }) require.NoError(t, err) require.NoError(t, f1.SetProperty(context.Background(), prop, "123")) - f2, err := CreateFilesystem(context.Background(), testZPool+"/list-2", CreateFilesystemOptions{ + f2, err := CreateFilesystem(context.Background(), testZPool+"/list-test2", CreateFilesystemOptions{ Properties: noMountProps, }) require.NoError(t, err) require.NoError(t, f2.SetProperty(context.Background(), prop, "321")) - ds, err := ListDatasets(context.Background(), ListOptions{ - ParentDataset: testZPool, + ds, err := root.Children(context.Background(), ListOptions{ DatasetType: DatasetFilesystem, ExtraProperties: []string{prop}, - Recursive: true, }) require.NoError(t, err) - require.Len(t, ds, 3) + require.Len(t, ds, 2) - require.Equal(t, testZPool, ds[0].Name) - - require.Equal(t, f1.Name, ds[1].Name) + require.Equal(t, f1.Name, ds[0].Name) require.Equal(t, "123", ds[0].ExtraProps[prop]) - require.Equal(t, f2.Name, ds[2].Name) + require.Equal(t, f2.Name, ds[1].Name) require.Equal(t, "321", ds[1].ExtraProps[prop]) }) } @@ -222,7 +221,7 @@ func TestClone(t *testing.T) { }) require.NoError(t, err) - filesystems, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetFilesystem}) + filesystems, err := Filesystems(context.Background(), ListOptions{}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -253,7 +252,7 @@ func TestSendSnapshot(t *testing.T) { }) require.NoError(t, err) - filesystems, err := ListDatasets(context.Background(), ListOptions{DatasetType: DatasetFilesystem}) + filesystems, err := Filesystems(context.Background(), ListOptions{}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -296,8 +295,7 @@ func TestSendSnapshotResume(t *testing.T) { require.True(t, errors.As(err, &zfsErr)) require.NotEmpty(t, zfsErr.ResumeToken(), zfsErr) - list, err := ListDatasets(context.Background(), ListOptions{ - DatasetType: DatasetFilesystem, + list, err := Filesystems(context.Background(), ListOptions{ ParentDataset: testZPool + "/recv-test", ExtraProperties: []string{PropertyReceiveResumeToken}, }) @@ -324,8 +322,7 @@ func TestSendSnapshotResume(t *testing.T) { }) require.NoError(t, err) - snaps, err := ListDatasets(context.Background(), ListOptions{ - DatasetType: DatasetSnapshot, + snaps, err := Snapshots(context.Background(), ListOptions{ ParentDataset: testZPool + "/recv-test", Recursive: true, }) From e06f2ce4b1671b285e4aaf3984656a413b97f0c2 Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Wed, 20 Mar 2024 17:41:48 +0100 Subject: [PATCH 08/12] More tests and code updates for new api --- http/client_test.go | 2 +- http/http_handlers.go | 11 ++++++-- http/http_handlers_test.go | 34 +++++++++++++++-------- job/filesystem_prune.go | 2 +- job/filesystem_prune_test.go | 4 ++- job/snapshots_create.go | 6 +++- job/snapshots_create_test.go | 2 +- job/snapshots_mark.go | 8 ++++-- job/snapshots_mark_test.go | 4 +-- job/snapshots_prune_test.go | 2 +- job/snapshots_send.go | 16 ++++++++--- job/snapshots_send_test.go | 12 ++++++-- zfs.go | 53 ++++++++++++++++++++++++++---------- zfs_test.go | 30 ++++++++++++++++++-- 14 files changed, 139 insertions(+), 47 deletions(-) diff --git a/http/client_test.go b/http/client_test.go index 356968c..465e9b5 100644 --- a/http/client_test.go +++ b/http/client_test.go @@ -60,7 +60,7 @@ func TestClient_Send(t *testing.T) { ds, err = zfs.GetDataset(context.Background(), fullNewFs) require.NoError(t, err) - snaps, err := ds.Snapshots(context.Background()) + snaps, err := ds.Snapshots(context.Background(), zfs.ListOptions{}) require.NoError(t, err) require.Len(t, snaps, 2) require.Equal(t, fullNewFs+"@lala1", snaps[0].Name) diff --git a/http/http_handlers.go b/http/http_handlers.go index 7108591..9302e5a 100644 --- a/http/http_handlers.go +++ b/http/http_handlers.go @@ -79,7 +79,11 @@ func zfsExtraProperties(req *http.Request) []string { } func (h *HTTP) handleListFilesystems(w http.ResponseWriter, req *http.Request, _ httprouter.Params, logger *slog.Logger) { - list, err := zfs.Filesystems(req.Context(), h.config.ParentDataset, zfsExtraProperties(req)...) + list, err := zfs.Filesystems(req.Context(), zfs.ListOptions{ + ParentDataset: h.config.ParentDataset, + ExtraProperties: zfsExtraProperties(req), + Recursive: true, + }) switch { case errors.Is(err, zfs.ErrDatasetNotFound): logger.Info("zfs.http.handleListFilesystems: Parent dataset not found", "error", err) @@ -178,7 +182,10 @@ func (h *HTTP) handleListSnapshots(w http.ResponseWriter, req *http.Request, ps return } - list, err := zfs.Snapshots(req.Context(), fmt.Sprintf("%s/%s", h.config.ParentDataset, filesystem), zfsExtraProperties(req)...) + list, err := zfs.Snapshots(req.Context(), zfs.ListOptions{ + ParentDataset: fmt.Sprintf("%s/%s", h.config.ParentDataset, filesystem), + ExtraProperties: zfsExtraProperties(req), + }) switch { case errors.Is(err, zfs.ErrDatasetNotFound): logger.Info("zfs.http.handleListSnapshots: Filesystem not found", "error", err, "filesystem", filesystem) diff --git a/http/http_handlers_test.go b/http/http_handlers_test.go index 224a3ae..8ff748b 100644 --- a/http/http_handlers_test.go +++ b/http/http_handlers_test.go @@ -55,14 +55,14 @@ func TestHTTP_handleListFilesystems(t *testing.T) { func TestHTTP_handleSetFilesystemProps(t *testing.T) { httpHandlerTest(t, func(server *httptest.Server) { props := SetProperties{ - Set: map[string]string{"nl.test:blaat": "disk"}, + Set: map[string]string{"nl.test:test": "helloworld", "nl.test:blaat": "disk"}, } data, err := json.Marshal(&props) require.NoError(t, err) req, err := http.NewRequest(http.MethodPatch, fmt.Sprintf("%s/filesystems/%s?%s=%s", server.URL, testFilesystemName, - GETParamExtraProperties, "nl.test:blaat,refquota", + GETParamExtraProperties, "nl.test:blaat,nl.test:test", ), bytes.NewBuffer(data)) require.NoError(t, err) req.Header.Set(HeaderAuthenticationToken, testAuthToken) @@ -77,7 +77,10 @@ func TestHTTP_handleSetFilesystemProps(t *testing.T) { require.NoError(t, err) require.Equal(t, testFilesystem, ds.Name) require.Len(t, ds.ExtraProps, 2) - require.Equal(t, map[string]string{"nl.test:blaat": "disk", "refquota": "0"}, ds.ExtraProps) + require.Equal(t, map[string]string{ + "nl.test:test": "helloworld", + "nl.test:blaat": "disk", + }, ds.ExtraProps) }) } @@ -103,7 +106,9 @@ func TestHTTP_handleMakeSnapshot(t *testing.T) { name := fmt.Sprintf("%s/%s@%s", testZPool, testFilesystemName, snapName) require.Equal(t, name, ds.Name) - snaps, err := zfs.Snapshots(context.Background(), fmt.Sprintf("%s/%s", testZPool, testFilesystemName)) + snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + ParentDataset: fmt.Sprintf("%s/%s", testZPool, testFilesystemName), + }) require.NoError(t, err) require.Len(t, snaps, 1) require.Equal(t, name, snaps[0].Name) @@ -139,7 +144,7 @@ func TestHTTP_handleGetSnapshot(t *testing.T) { require.NoError(t, err) require.Equal(t, testName, ds.Name) - snaps, err := zfs.Snapshots(context.Background(), testName) + snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ParentDataset: testName}) require.NoError(t, err) require.Len(t, snaps, 1) require.Equal(t, fmt.Sprintf("%s/%s@%s", testZPool, "receive", snapName), snaps[0].Name) @@ -196,7 +201,7 @@ func TestHTTP_handleGetSnapshotIncremental(t *testing.T) { require.NoError(t, err) require.Equal(t, newFilesys, ds.Name) - snaps, err := zfs.Snapshots(context.Background(), newFilesys) + snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ParentDataset: newFilesys}) require.NoError(t, err) require.Len(t, snaps, 2) require.Equal(t, fmt.Sprintf("%s@%s", newFilesys, snapName1), snaps[0].Name) @@ -234,7 +239,10 @@ func TestHTTP_handleResumeGetSnapshot(t *testing.T) { var recvErr *zfs.ResumableStreamError require.True(t, errors.As(err, &recvErr)) - fs, err := zfs.Filesystems(context.Background(), testName, zfs.PropertyReceiveResumeToken) + fs, err := zfs.Filesystems(context.Background(), zfs.ListOptions{ + ParentDataset: testName, + ExtraProperties: []string{zfs.PropertyReceiveResumeToken}, + }) require.NoError(t, err) require.Len(t, fs, 1) require.Equal(t, testName, fs[0].Name) @@ -259,7 +267,7 @@ func TestHTTP_handleResumeGetSnapshot(t *testing.T) { }) require.NoError(t, err) - snaps, err := zfs.Snapshots(context.Background(), testName) + snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ParentDataset: testName}) require.NoError(t, err) require.Len(t, snaps, 1) require.Equal(t, fmt.Sprintf("%s/%s@%s", testZPool, "receive", snapName), snaps[0].Name) @@ -302,7 +310,7 @@ func TestHTTP_handleReceiveSnapshot(t *testing.T) { require.NoError(t, err) require.Equal(t, newFs.ExtraProps[testProp], testPropVal) - snaps, err := newFs.Snapshots(context.Background()) + snaps, err := newFs.Snapshots(context.Background(), zfs.ListOptions{}) require.NoError(t, err) require.Len(t, snaps, 1) require.Equal(t, name, snaps[0].Name) @@ -353,7 +361,9 @@ func TestHTTP_handleReceiveSnapshotNoExplicitName(t *testing.T) { require.NoError(t, err) require.Equal(t, fmt.Sprintf("%s/%s", testZPool, newFilesystem), ds.Name) - snaps, err := zfs.Snapshots(context.Background(), fmt.Sprintf("%s/%s", testZPool, newFilesystem)) + snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + ParentDataset: fmt.Sprintf("%s/%s", testZPool, newFilesystem), + }) require.NoError(t, err) require.Len(t, snaps, 1) require.Equal(t, fmt.Sprintf("%s/%s@%s", testZPool, newFilesystem, snapName), snaps[0].Name) @@ -461,7 +471,9 @@ func TestHTTP_handleReceiveSnapshotResume(t *testing.T) { name := fmt.Sprintf("%s/%s@%s", testZPool, newFilesystem, newSnap) require.Equal(t, name, ds.Name) - snaps, err := zfs.Snapshots(context.Background(), fmt.Sprintf("%s/%s", testZPool, newFilesystem)) + snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + ParentDataset: fmt.Sprintf("%s/%s", testZPool, newFilesystem), + }) require.NoError(t, err) require.Len(t, snaps, 1) require.Equal(t, name, snaps[0].Name) diff --git a/job/filesystem_prune.go b/job/filesystem_prune.go index c1ea132..9676d5e 100644 --- a/job/filesystem_prune.go +++ b/job/filesystem_prune.go @@ -55,7 +55,7 @@ func (r *Runner) pruneAgedFilesystem(filesystem string) error { return nil // Not due for removal yet } - children, err := fs.Children(r.ctx, 0) + children, err := fs.Children(r.ctx, zfs.ListOptions{Depth: 1}) if err != nil { return fmt.Errorf("error listing %s children: %w", filesystem, err) } diff --git a/job/filesystem_prune_test.go b/job/filesystem_prune_test.go index 5377928..1db4017 100644 --- a/job/filesystem_prune_test.go +++ b/job/filesystem_prune_test.go @@ -71,7 +71,9 @@ func TestRunner_pruneFilesystems(t *testing.T) { ds, err := zfs.GetDataset(context.Background(), testZPool) require.NoError(t, err) - datasets, err := ds.Children(context.Background(), 0) + datasets, err := ds.Children(context.Background(), zfs.ListOptions{ + Depth: 1, + }) require.NoError(t, err) require.Len(t, datasets, 5) require.Equal(t, fmt.Sprintf("%s/%s", testZPool, fsWithoutDel), datasets[0].Name) diff --git a/job/snapshots_create.go b/job/snapshots_create.go index bf54b71..0ba71b5 100644 --- a/job/snapshots_create.go +++ b/job/snapshots_create.go @@ -57,7 +57,11 @@ func (r *Runner) createDatasetSnapshot(ds *zfs.Dataset) error { } createdProp := r.config.Properties.snapshotCreatedAt() - snapshots, err := zfs.ListByType(r.ctx, zfs.DatasetSnapshot, ds.Name, createdProp) + snapshots, err := zfs.ListDatasets(r.ctx, zfs.ListOptions{ + DatasetType: zfs.DatasetSnapshot, + ParentDataset: ds.Name, + ExtraProperties: []string{createdProp}, + }) if err != nil { return fmt.Errorf("error listing existing snapshots: %w", err) } diff --git a/job/snapshots_create_test.go b/job/snapshots_create_test.go index 8d561f2..fcadf64 100644 --- a/job/snapshots_create_test.go +++ b/job/snapshots_create_test.go @@ -38,7 +38,7 @@ func TestRunner_createSnapshots(t *testing.T) { createTm := arguments[2].(time.Time) require.WithinDuration(t, tm, createTm, time.Second) - snaps, err := ds.Snapshots(context.Background(), createProp) + snaps, err := ds.Snapshots(context.Background(), zfs.ListOptions{ExtraProperties: []string{createProp}}) require.NoError(t, err) require.Len(t, snaps, 1) require.Equal(t, snaps[0].Name, testZPool+"/"+fsName+"@"+name) diff --git a/job/snapshots_mark.go b/job/snapshots_mark.go index 798efb2..e7c6ad9 100644 --- a/job/snapshots_mark.go +++ b/job/snapshots_mark.go @@ -59,7 +59,9 @@ func (r *Runner) markExcessDatasetSnapshots(ds *zfs.Dataset, maxCount int64) err createdProp := r.config.Properties.snapshotCreatedAt() deleteProp := r.config.Properties.deleteAt() - snaps, err := ds.Snapshots(r.ctx, createdProp, deleteProp) + snaps, err := ds.Snapshots(r.ctx, zfs.ListOptions{ + ExtraProperties: []string{createdProp, deleteProp}, + }) if err != nil { return fmt.Errorf("error retrieving snapshots for %s: %w", ds.Name, err) } @@ -133,7 +135,9 @@ func (r *Runner) markAgingDatasetSnapshots(ds *zfs.Dataset, duration time.Durati createdProp := r.config.Properties.snapshotCreatedAt() deleteProp := r.config.Properties.deleteAt() - snaps, err := ds.Snapshots(r.ctx, createdProp, deleteProp) + snaps, err := ds.Snapshots(r.ctx, zfs.ListOptions{ + ExtraProperties: []string{createdProp, deleteProp}, + }) if err != nil { return fmt.Errorf("error retrieving snapshots for %s: %w", ds.Name, err) } diff --git a/job/snapshots_mark_test.go b/job/snapshots_mark_test.go index d7408a7..8dfcfdd 100644 --- a/job/snapshots_mark_test.go +++ b/job/snapshots_mark_test.go @@ -52,7 +52,7 @@ func TestRunner_markPrunableExcessSnapshots(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, events) - snaps, err := ds.Snapshots(context.Background(), deleteProp) + snaps, err := ds.Snapshots(context.Background(), zfs.ListOptions{ExtraProperties: []string{deleteProp}}) require.NoError(t, err) require.Len(t, snaps, 3) @@ -113,7 +113,7 @@ func TestRunner_markPrunableSnapshotsByAge(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, events) - snaps, err := ds.Snapshots(context.Background(), deleteProp) + snaps, err := ds.Snapshots(context.Background(), zfs.ListOptions{ExtraProperties: []string{deleteProp}}) require.NoError(t, err) require.Len(t, snaps, 4) diff --git a/job/snapshots_prune_test.go b/job/snapshots_prune_test.go index d46ef70..9a0d1a4 100644 --- a/job/snapshots_prune_test.go +++ b/job/snapshots_prune_test.go @@ -61,7 +61,7 @@ func TestRunner_pruneSnapshots(t *testing.T) { require.NoError(t, err) require.Equal(t, 2, events) - snaps, err := ds.Snapshots(context.Background()) + snaps, err := ds.Snapshots(context.Background(), zfs.ListOptions{}) require.NoError(t, err) require.Len(t, snaps, 2) require.Equal(t, snaps[0].Name, fmt.Sprintf("%s@%s", testFilesystem, snap3)) diff --git a/job/snapshots_send.go b/job/snapshots_send.go index 3a1def7..9596bc5 100644 --- a/job/snapshots_send.go +++ b/job/snapshots_send.go @@ -17,17 +17,21 @@ var ( func (r *Runner) sendSnapshots(routineID int) error { sendToProp := r.config.Properties.snapshotSendTo() - datasets, err := zfs.ListWithProperty(r.ctx, r.config.DatasetType, r.config.ParentDataset, sendToProp) + datasets, err := zfs.ListDatasets(r.ctx, zfs.ListOptions{ + DatasetType: r.config.DatasetType, + ParentDataset: r.config.ParentDataset, + ExtraProperties: []string{sendToProp}, + }) if err != nil { return fmt.Errorf("error finding snapshottable datasets: %w", err) } - for dataset := range datasets { + for _, dataset := range datasets { if r.ctx.Err() != nil { return nil // context expired, no problem } - ds, err := zfs.GetDataset(r.ctx, dataset, sendToProp) + ds, err := zfs.GetDataset(r.ctx, dataset.Name, sendToProp) if err != nil { return fmt.Errorf("error retrieving snapshottable dataset %s: %w", dataset, err) } @@ -72,7 +76,11 @@ func (r *Runner) sendDatasetSnapshots(routineID int, ds *zfs.Dataset) error { sentProp := r.config.Properties.snapshotSentAt() sendToProp := r.config.Properties.snapshotSendTo() - localSnaps, err := zfs.ListByType(r.ctx, zfs.DatasetSnapshot, ds.Name, createdProp, sentProp) + localSnaps, err := zfs.ListDatasets(r.ctx, zfs.ListOptions{ + DatasetType: zfs.DatasetSnapshot, + ParentDataset: ds.Name, + ExtraProperties: []string{createdProp, sentProp}, + }) if err != nil { return fmt.Errorf("error listing existing snapshots: %w", err) } diff --git a/job/snapshots_send_test.go b/job/snapshots_send_test.go index 7bbcabd..4c32daa 100644 --- a/job/snapshots_send_test.go +++ b/job/snapshots_send_test.go @@ -71,7 +71,9 @@ func TestRunner_sendSnapshots(t *testing.T) { require.Equal(t, 5, sendingCount) require.Equal(t, 5, sentCount) - snaps, err := zfs.Snapshots(context.Background(), testHTTPZPool+"/"+datasetName(testFilesystem, true)) + snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + ParentDataset: testHTTPZPool + "/" + datasetName(testFilesystem, true), + }) require.NoError(t, err) require.Len(t, snaps, 5) @@ -124,7 +126,9 @@ func TestRunner_sendPartialSnapshots(t *testing.T) { require.Equal(t, 4, sendingCount) require.Equal(t, 4, sentCount) - snaps, err := zfs.Snapshots(context.Background(), testHTTPZPool+"/"+datasetName(testFilesystem, true)) + snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + ParentDataset: testHTTPZPool + "/" + datasetName(testFilesystem, true), + }) require.NoError(t, err) require.Len(t, snaps, 5) @@ -177,7 +181,9 @@ func TestRunner_sendWithMissingSnapshots(t *testing.T) { require.Equal(t, 2, sendingCount) require.Equal(t, 2, sentCount) - snaps, err := zfs.Snapshots(context.Background(), testHTTPZPool+"/"+datasetName(testFilesystem, true)) + snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + ParentDataset: testHTTPZPool + "/" + datasetName(testFilesystem, true), + }) require.NoError(t, err) require.Len(t, snaps, 3) diff --git a/zfs.go b/zfs.go index ed030cb..6e02f56 100644 --- a/zfs.go +++ b/zfs.go @@ -30,6 +30,8 @@ type ListOptions struct { // Recursively display any children of the dataset, limiting the recursion to depth. // A depth of 1 will display only the dataset and its direct children. Depth int + // FilterSelf: When true, it will filter out the parent dataset itself from the results + FilterSelf bool } // ListDatasets lists the datasets by type and allows you to fetch extra custom fields @@ -49,8 +51,7 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { } allFields := append(dsPropList, options.ExtraProperties...) // nolint: gocritic - dsPropListOptions := strings.Join(allFields, ",") - args = append(args, dsPropListOptions) + args = append(args, strings.Join(allFields, ",")) if options.ParentDataset != "" { args = append(args, options.ParentDataset) @@ -61,7 +62,40 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { return nil, err } - return readDatasets(out, options.ExtraProperties) + ds, err := readDatasets(out, options.ExtraProperties) + if err != nil { + return nil, err + } + + // Filter out the parent dataset: + if options.FilterSelf { + ds = slices.DeleteFunc(ds, func(dataset Dataset) bool { + return dataset.Name == options.ParentDataset + }) + } + return ds, nil +} + +// ListWithProperty returns a map of dataset names mapped to the properties value for datasets which have the given ZFS property. +func ListWithProperty(ctx context.Context, tp DatasetType, parentDataset, prop string) (map[string]string, error) { + c := command{ + cmd: Binary, + ctx: ctx, + } + lines, err := c.Run("get", "-t", string(tp), "-Hp", "-o", "name,value", "-r", "-s", "local", prop, parentDataset) + if err != nil { + return nil, err + } + result := make(map[string]string, len(lines)) + for _, line := range lines { + switch len(line) { + case 2: + result[line[0]] = line[1] + case 1: + result[line[0]] = PropertyUnset + } + } + return result, nil } // Volumes returns a slice of ZFS volumes. @@ -715,15 +749,6 @@ func (d *Dataset) Rollback(ctx context.Context, options RollbackOptions) error { func (d *Dataset) Children(ctx context.Context, options ListOptions) ([]Dataset, error) { options.ParentDataset = d.Name options.Recursive = true - ds, err := ListDatasets(ctx, options) - if err != nil { - return nil, err - } - - // Filter out the parent dataset. - ds = slices.DeleteFunc(ds, func(dataset Dataset) bool { - return dataset.Name == d.Name - }) - - return ds, nil + options.FilterSelf = true + return ListDatasets(ctx, options) } diff --git a/zfs_test.go b/zfs_test.go index 00735b6..b8fbbe8 100644 --- a/zfs_test.go +++ b/zfs_test.go @@ -32,11 +32,16 @@ func TestDatasets(t *testing.T) { func TestDatasetsWithProps(t *testing.T) { TestZPool(testZPool, func() { - ds, err := GetDataset(context.Background(), testZPool, "xattr", "canmount") + ds, err := GetDataset(context.Background(), testZPool) + require.NoError(t, err) + + require.NoError(t, ds.SetProperty(context.Background(), "nl.test:hello", "world")) + + ds, err = GetDataset(context.Background(), testZPool, "nl.test:hello", "canmount") require.NoError(t, err) require.Len(t, ds.ExtraProps, 2, fmt.Sprintf("%#v", ds.ExtraProps)) - require.Equal(t, "off", ds.ExtraProps["xattr"]) + require.Equal(t, "world", ds.ExtraProps["nl.test:hello"]) require.Equal(t, "off", ds.ExtraProps["canmount"]) }) } @@ -180,7 +185,7 @@ func TestSnapshot(t *testing.T) { }) } -func TestListWithProperty(t *testing.T) { +func TestListingWithProperty(t *testing.T) { TestZPool(testZPool, func() { const prop = "nl.test:bla" @@ -214,6 +219,25 @@ func TestListWithProperty(t *testing.T) { }) } +func TestListWithProperty(t *testing.T) { + TestZPool(testZPool, func() { + const prop = "nl.test:bla" + + f1, err := CreateFilesystem(context.Background(), testZPool+"/list-test1", CreateFilesystemOptions{ + Properties: noMountProps, + }) + require.NoError(t, err) + require.NoError(t, f1.SetProperty(context.Background(), prop, "123")) + + ls, err := ListWithProperty(context.Background(), DatasetFilesystem, testZPool+"/list-test", prop) + require.NoError(t, err) + require.Len(t, ls, 1) + require.Equal(t, map[string]string{ + f1.Name: "123", + }, ls) + }) +} + func TestClone(t *testing.T) { TestZPool(testZPool, func() { f, err := CreateFilesystem(context.Background(), testZPool+"/snapshot-test", CreateFilesystemOptions{ From f9cec5af7fa976b00b9d90486c1aa3eb73a68137 Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Thu, 21 Mar 2024 09:51:31 +0100 Subject: [PATCH 09/12] Refactor list function names + fix multiple extra properties --- dataset.go | 46 +++++++++++++++++++----------------- dataset_test.go | 12 +++++++--- http/http.go | 2 +- http/http_handlers.go | 4 ++-- http/http_handlers_test.go | 14 +++++------ job/snapshots_create.go | 4 ++-- job/snapshots_mark.go | 2 +- job/snapshots_send.go | 11 +++------ job/snapshots_send_test.go | 6 ++--- zfs.go | 48 ++++++++++++++++++++------------------ zfs_test.go | 33 ++++++++++++++------------ 11 files changed, 95 insertions(+), 87 deletions(-) diff --git a/dataset.go b/dataset.go index c1dd00f..be2e813 100644 --- a/dataset.go +++ b/dataset.go @@ -54,7 +54,7 @@ func readDatasets(output [][]string, extraProps []string) ([]Dataset, error) { ) } - count := len(output) / (len(dsPropList) + len(extraProps)) + count := len(output) / multiple curDataset := 0 datasets := make([]Dataset, count) for i, fields := range output { @@ -67,8 +67,11 @@ func readDatasets(output [][]string, extraProps []string) ([]Dataset, error) { } ds := &datasets[curDataset] - ds.ExtraProps = make(map[string]string, len(extraProps)) ds.Name = fields[nameField] + // Init extra props if needed + if ds.ExtraProps == nil { + ds.ExtraProps = make(map[string]string, len(extraProps)) + } prop := fields[propertyField] val := fields[valueField] @@ -80,29 +83,29 @@ func readDatasets(output [][]string, extraProps []string) ([]Dataset, error) { case PropertyType: ds.Type = DatasetType(val) case PropertyOrigin: - setString(&ds.Origin, val) + ds.Origin = setString(val) case PropertyUsed: - setError = setUint(&ds.Used, val) + ds.Used, setError = setUint(val) case PropertyAvailable: - setError = setUint(&ds.Available, val) + ds.Available, setError = setUint(val) case PropertyMountPoint: - setString(&ds.Mountpoint, val) + ds.Mountpoint = setString(val) case PropertyCompression: - setString(&ds.Compression, val) + ds.Compression = setString(val) case PropertyWritten: - setError = setUint(&ds.Written, val) + ds.Written, setError = setUint(val) case PropertyVolSize: - setError = setUint(&ds.Volsize, val) + ds.Volsize, setError = setUint(val) case PropertyLogicalUsed: - setError = setUint(&ds.Logicalused, val) + ds.Logicalused, setError = setUint(val) case PropertyUsedByDataset: - setError = setUint(&ds.Usedbydataset, val) + ds.Usedbydataset, setError = setUint(val) case PropertyQuota: - setError = setUint(&ds.Quota, val) + ds.Quota, setError = setUint(val) case PropertyRefQuota: - setError = setUint(&ds.Refquota, val) + ds.Refquota, setError = setUint(val) case PropertyReferenced: - setError = setUint(&ds.Referenced, val) + ds.Referenced, setError = setUint(val) default: if val == PropertyUnset { ds.ExtraProps[prop] = "" @@ -118,22 +121,21 @@ func readDatasets(output [][]string, extraProps []string) ([]Dataset, error) { return datasets, nil } -func setString(field *string, val string) { +func setString(val string) string { if val == PropertyUnset { - return + return "" } - *field = val + return val } -func setUint(field *uint64, val string) error { +func setUint(val string) (uint64, error) { if val == PropertyUnset { - return nil + return 0, nil } v, err := strconv.ParseUint(val, 10, 64) if err != nil { - return err + return 0, err } - *field = v - return nil + return v, nil } diff --git a/dataset_test.go b/dataset_test.go index 26b518a..03426e5 100644 --- a/dataset_test.go +++ b/dataset_test.go @@ -8,8 +8,10 @@ import ( func Test_readDatasets(t *testing.T) { in := splitOutput(testInput) - const prop = "nl.test:hiephoi" - ds, err := readDatasets(in, []string{prop}) + const prop1 = "nl.test:hiephoi" + const prop2 = "nl.test:eigenschap" + + ds, err := readDatasets(in, []string{prop1, prop2}) require.NoError(t, err) require.Len(t, ds, 3) require.Equal(t, ds[0].Name, "testpool/ds0") @@ -23,7 +25,8 @@ func Test_readDatasets(t *testing.T) { require.NotZero(t, ds[i].Referenced) require.NotZero(t, ds[i].Used) require.NotZero(t, ds[i].Available) - require.Equal(t, "42", ds[i].ExtraProps[prop]) + require.Equal(t, "42", ds[i].ExtraProps[prop1]) + require.Equal(t, "ja", ds[i].ExtraProps[prop2]) } } @@ -42,6 +45,7 @@ testpool/ds0 written 196416 testpool/ds0 logicalused 43520 testpool/ds0 usedbydataset 196416 testpool/ds0 nl.test:hiephoi 42 +testpool/ds0 nl.test:eigenschap ja testpool/ds1 name testpool/ds1 testpool/ds1 type filesystem testpool/ds1 origin - @@ -57,6 +61,7 @@ testpool/ds1 written 196416 testpool/ds1 logicalused 43520 testpool/ds1 usedbydataset 196416 testpool/ds1 nl.test:hiephoi 42 +testpool/ds1 nl.test:eigenschap ja testpool/ds10 name testpool/ds10 testpool/ds10 type filesystem testpool/ds10 origin - @@ -72,4 +77,5 @@ testpool/ds10 written 196416 testpool/ds10 logicalused 43520 testpool/ds10 usedbydataset 196416 testpool/ds10 nl.test:hiephoi 42 +testpool/ds10 nl.test:eigenschap ja ` diff --git a/http/http.go b/http/http.go index 27e8672..5e8eef2 100644 --- a/http/http.go +++ b/http/http.go @@ -53,7 +53,7 @@ func (h *HTTP) init() error { h.logger.Info("zfs.http.init: Serving", "host", h.config.Host, "port", h.config.Port) h.httpServer = &http.Server{ Handler: h.router, - BaseContext: func(listener net.Listener) context.Context { + BaseContext: func(_ net.Listener) context.Context { return h.ctx }, } diff --git a/http/http_handlers.go b/http/http_handlers.go index 9302e5a..c4357ef 100644 --- a/http/http_handlers.go +++ b/http/http_handlers.go @@ -79,7 +79,7 @@ func zfsExtraProperties(req *http.Request) []string { } func (h *HTTP) handleListFilesystems(w http.ResponseWriter, req *http.Request, _ httprouter.Params, logger *slog.Logger) { - list, err := zfs.Filesystems(req.Context(), zfs.ListOptions{ + list, err := zfs.ListFilesystems(req.Context(), zfs.ListOptions{ ParentDataset: h.config.ParentDataset, ExtraProperties: zfsExtraProperties(req), Recursive: true, @@ -182,7 +182,7 @@ func (h *HTTP) handleListSnapshots(w http.ResponseWriter, req *http.Request, ps return } - list, err := zfs.Snapshots(req.Context(), zfs.ListOptions{ + list, err := zfs.ListSnapshots(req.Context(), zfs.ListOptions{ ParentDataset: fmt.Sprintf("%s/%s", h.config.ParentDataset, filesystem), ExtraProperties: zfsExtraProperties(req), }) diff --git a/http/http_handlers_test.go b/http/http_handlers_test.go index 8ff748b..3996726 100644 --- a/http/http_handlers_test.go +++ b/http/http_handlers_test.go @@ -106,7 +106,7 @@ func TestHTTP_handleMakeSnapshot(t *testing.T) { name := fmt.Sprintf("%s/%s@%s", testZPool, testFilesystemName, snapName) require.Equal(t, name, ds.Name) - snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + snaps, err := zfs.ListSnapshots(context.Background(), zfs.ListOptions{ ParentDataset: fmt.Sprintf("%s/%s", testZPool, testFilesystemName), }) require.NoError(t, err) @@ -144,7 +144,7 @@ func TestHTTP_handleGetSnapshot(t *testing.T) { require.NoError(t, err) require.Equal(t, testName, ds.Name) - snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ParentDataset: testName}) + snaps, err := zfs.ListSnapshots(context.Background(), zfs.ListOptions{ParentDataset: testName}) require.NoError(t, err) require.Len(t, snaps, 1) require.Equal(t, fmt.Sprintf("%s/%s@%s", testZPool, "receive", snapName), snaps[0].Name) @@ -201,7 +201,7 @@ func TestHTTP_handleGetSnapshotIncremental(t *testing.T) { require.NoError(t, err) require.Equal(t, newFilesys, ds.Name) - snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ParentDataset: newFilesys}) + snaps, err := zfs.ListSnapshots(context.Background(), zfs.ListOptions{ParentDataset: newFilesys}) require.NoError(t, err) require.Len(t, snaps, 2) require.Equal(t, fmt.Sprintf("%s@%s", newFilesys, snapName1), snaps[0].Name) @@ -239,7 +239,7 @@ func TestHTTP_handleResumeGetSnapshot(t *testing.T) { var recvErr *zfs.ResumableStreamError require.True(t, errors.As(err, &recvErr)) - fs, err := zfs.Filesystems(context.Background(), zfs.ListOptions{ + fs, err := zfs.ListFilesystems(context.Background(), zfs.ListOptions{ ParentDataset: testName, ExtraProperties: []string{zfs.PropertyReceiveResumeToken}, }) @@ -267,7 +267,7 @@ func TestHTTP_handleResumeGetSnapshot(t *testing.T) { }) require.NoError(t, err) - snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ParentDataset: testName}) + snaps, err := zfs.ListSnapshots(context.Background(), zfs.ListOptions{ParentDataset: testName}) require.NoError(t, err) require.Len(t, snaps, 1) require.Equal(t, fmt.Sprintf("%s/%s@%s", testZPool, "receive", snapName), snaps[0].Name) @@ -361,7 +361,7 @@ func TestHTTP_handleReceiveSnapshotNoExplicitName(t *testing.T) { require.NoError(t, err) require.Equal(t, fmt.Sprintf("%s/%s", testZPool, newFilesystem), ds.Name) - snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + snaps, err := zfs.ListSnapshots(context.Background(), zfs.ListOptions{ ParentDataset: fmt.Sprintf("%s/%s", testZPool, newFilesystem), }) require.NoError(t, err) @@ -471,7 +471,7 @@ func TestHTTP_handleReceiveSnapshotResume(t *testing.T) { name := fmt.Sprintf("%s/%s@%s", testZPool, newFilesystem, newSnap) require.Equal(t, name, ds.Name) - snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + snaps, err := zfs.ListSnapshots(context.Background(), zfs.ListOptions{ ParentDataset: fmt.Sprintf("%s/%s", testZPool, newFilesystem), }) require.NoError(t, err) diff --git a/job/snapshots_create.go b/job/snapshots_create.go index 0ba71b5..476a6db 100644 --- a/job/snapshots_create.go +++ b/job/snapshots_create.go @@ -45,6 +45,7 @@ func (r *Runner) createSnapshots() error { func (r *Runner) snapshotName(tm time.Time) string { name := r.config.SnapshotNameTemplate name = strings.ReplaceAll(name, "%UNIXTIME%", strconv.FormatInt(tm.Unix(), 10)) + name = strings.ReplaceAll(name, "%RFC3339%", tm.Format(time.RFC3339)) // TODO: FIXME: Some other constant replacement could be added here? return name } @@ -57,8 +58,7 @@ func (r *Runner) createDatasetSnapshot(ds *zfs.Dataset) error { } createdProp := r.config.Properties.snapshotCreatedAt() - snapshots, err := zfs.ListDatasets(r.ctx, zfs.ListOptions{ - DatasetType: zfs.DatasetSnapshot, + snapshots, err := zfs.ListSnapshots(r.ctx, zfs.ListOptions{ ParentDataset: ds.Name, ExtraProperties: []string{createdProp}, }) diff --git a/job/snapshots_mark.go b/job/snapshots_mark.go index e7c6ad9..2b81a07 100644 --- a/job/snapshots_mark.go +++ b/job/snapshots_mark.go @@ -66,7 +66,7 @@ func (r *Runner) markExcessDatasetSnapshots(ds *zfs.Dataset, maxCount int64) err return fmt.Errorf("error retrieving snapshots for %s: %w", ds.Name, err) } - // Snapshots are always retrieved with the newest last, so reverse the list: + // ListSnapshots are always retrieved with the newest last, so reverse the list: reverseDatasets(snaps) currentFound := int64(0) now := time.Now() diff --git a/job/snapshots_send.go b/job/snapshots_send.go index 9596bc5..b5ff5da 100644 --- a/job/snapshots_send.go +++ b/job/snapshots_send.go @@ -17,11 +17,7 @@ var ( func (r *Runner) sendSnapshots(routineID int) error { sendToProp := r.config.Properties.snapshotSendTo() - datasets, err := zfs.ListDatasets(r.ctx, zfs.ListOptions{ - DatasetType: r.config.DatasetType, - ParentDataset: r.config.ParentDataset, - ExtraProperties: []string{sendToProp}, - }) + datasets, err := zfs.ListWithProperty(r.ctx, r.config.DatasetType, r.config.ParentDataset, sendToProp) if err != nil { return fmt.Errorf("error finding snapshottable datasets: %w", err) } @@ -31,7 +27,7 @@ func (r *Runner) sendSnapshots(routineID int) error { return nil // context expired, no problem } - ds, err := zfs.GetDataset(r.ctx, dataset.Name, sendToProp) + ds, err := zfs.GetDataset(r.ctx, dataset, sendToProp) if err != nil { return fmt.Errorf("error retrieving snapshottable dataset %s: %w", dataset, err) } @@ -76,8 +72,7 @@ func (r *Runner) sendDatasetSnapshots(routineID int, ds *zfs.Dataset) error { sentProp := r.config.Properties.snapshotSentAt() sendToProp := r.config.Properties.snapshotSendTo() - localSnaps, err := zfs.ListDatasets(r.ctx, zfs.ListOptions{ - DatasetType: zfs.DatasetSnapshot, + localSnaps, err := zfs.ListSnapshots(r.ctx, zfs.ListOptions{ ParentDataset: ds.Name, ExtraProperties: []string{createdProp, sentProp}, }) diff --git a/job/snapshots_send_test.go b/job/snapshots_send_test.go index 4c32daa..7e1d069 100644 --- a/job/snapshots_send_test.go +++ b/job/snapshots_send_test.go @@ -71,7 +71,7 @@ func TestRunner_sendSnapshots(t *testing.T) { require.Equal(t, 5, sendingCount) require.Equal(t, 5, sentCount) - snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + snaps, err := zfs.ListSnapshots(context.Background(), zfs.ListOptions{ ParentDataset: testHTTPZPool + "/" + datasetName(testFilesystem, true), }) require.NoError(t, err) @@ -126,7 +126,7 @@ func TestRunner_sendPartialSnapshots(t *testing.T) { require.Equal(t, 4, sendingCount) require.Equal(t, 4, sentCount) - snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + snaps, err := zfs.ListSnapshots(context.Background(), zfs.ListOptions{ ParentDataset: testHTTPZPool + "/" + datasetName(testFilesystem, true), }) require.NoError(t, err) @@ -181,7 +181,7 @@ func TestRunner_sendWithMissingSnapshots(t *testing.T) { require.Equal(t, 2, sendingCount) require.Equal(t, 2, sentCount) - snaps, err := zfs.Snapshots(context.Background(), zfs.ListOptions{ + snaps, err := zfs.ListSnapshots(context.Background(), zfs.ListOptions{ ParentDataset: testHTTPZPool + "/" + datasetName(testFilesystem, true), }) require.NoError(t, err) diff --git a/zfs.go b/zfs.go index 6e02f56..cd89b39 100644 --- a/zfs.go +++ b/zfs.go @@ -57,6 +57,7 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { args = append(args, options.ParentDataset) } + fmt.Println(strings.Join(args, " ")) out, err := zfsOutput(ctx, args...) if err != nil { return nil, err @@ -76,6 +77,27 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { return ds, nil } +// ListVolumes returns a slice of ZFS volumes. +// A filter argument may be passed to select a volume with the matching name, or empty string ("") may be used to select all volumes. +func ListVolumes(ctx context.Context, options ListOptions) ([]Dataset, error) { + options.DatasetType = DatasetVolume + return ListDatasets(ctx, options) +} + +// ListFilesystems returns a slice of ZFS filesystems. +// A filter argument may be passed to select a filesystem with the matching name, or empty string ("") may be used to select all filesystems. +func ListFilesystems(ctx context.Context, options ListOptions) ([]Dataset, error) { + options.DatasetType = DatasetFilesystem + return ListDatasets(ctx, options) +} + +// ListSnapshots returns a slice of ZFS snapshots. +// A filter argument may be passed to select a snapshot with the matching name, or empty string ("") may be used to select all snapshots. +func ListSnapshots(ctx context.Context, options ListOptions) ([]Dataset, error) { + options.DatasetType = DatasetSnapshot + return ListDatasets(ctx, options) +} + // ListWithProperty returns a map of dataset names mapped to the properties value for datasets which have the given ZFS property. func ListWithProperty(ctx context.Context, tp DatasetType, parentDataset, prop string) (map[string]string, error) { c := command{ @@ -98,33 +120,13 @@ func ListWithProperty(ctx context.Context, tp DatasetType, parentDataset, prop s return result, nil } -// Volumes returns a slice of ZFS volumes. -// A filter argument may be passed to select a volume with the matching name, or empty string ("") may be used to select all volumes. -func Volumes(ctx context.Context, options ListOptions) ([]Dataset, error) { - options.DatasetType = DatasetVolume - return ListDatasets(ctx, options) -} - -// Filesystems returns a slice of ZFS filesystems. -// A filter argument may be passed to select a filesystem with the matching name, or empty string ("") may be used to select all filesystems. -func Filesystems(ctx context.Context, options ListOptions) ([]Dataset, error) { - options.DatasetType = DatasetFilesystem - return ListDatasets(ctx, options) -} - -// Snapshots returns a slice of ZFS snapshots. -// A filter argument may be passed to select a snapshot with the matching name, or empty string ("") may be used to select all snapshots. -func Snapshots(ctx context.Context, options ListOptions) ([]Dataset, error) { - options.DatasetType = DatasetSnapshot - return ListDatasets(ctx, options) -} - // GetDataset retrieves a single ZFS dataset by name. // This dataset could be any valid ZFS dataset type, such as a clone, filesystem, snapshot, or volume. func GetDataset(ctx context.Context, name string, extraProperties ...string) (*Dataset, error) { ds, err := ListDatasets(ctx, ListOptions{ ParentDataset: name, Recursive: false, + FilterSelf: false, ExtraProperties: extraProperties, }) if err != nil { @@ -568,7 +570,7 @@ type RenameOptions struct { // according to the mountpoint property inherited from their parent. CreateParent bool - // Recursively rename the snapshots of all descendent datasets. Snapshots are the only dataset that can + // Recursively rename the snapshots of all descendent datasets. ListSnapshots are the only dataset that can // be renamed recursively. Recursive bool @@ -609,7 +611,7 @@ func (d *Dataset) Rename(ctx context.Context, name string, options RenameOptions return GetDataset(ctx, name) } -// Snapshots returns a slice of all ZFS snapshots of a given dataset. +// ListSnapshots returns a slice of all ZFS snapshots of a given dataset. func (d *Dataset) Snapshots(ctx context.Context, options ListOptions) ([]Dataset, error) { options.ParentDataset = d.Name options.DatasetType = DatasetSnapshot diff --git a/zfs_test.go b/zfs_test.go index b8fbbe8..4c94f3b 100644 --- a/zfs_test.go +++ b/zfs_test.go @@ -91,7 +91,7 @@ func TestDatasetSetInheritProperty(t *testing.T) { func TestSnapshots(t *testing.T) { TestZPool(testZPool, func() { - snapshots, err := Snapshots(context.Background(), ListOptions{}) + snapshots, err := ListSnapshots(context.Background(), ListOptions{}) require.NoError(t, err) for _, snapshot := range snapshots { @@ -107,7 +107,7 @@ func TestFilesystems(t *testing.T) { }) require.NoError(t, err) - filesystems, err := Filesystems(context.Background(), ListOptions{}) + filesystems, err := ListFilesystems(context.Background(), ListOptions{}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -129,7 +129,7 @@ func TestCreateFilesystemWithProperties(t *testing.T) { require.NoError(t, err) require.Equal(t, "lz4", f.Compression) - filesystems, err := ListDatasets(context.Background(), ListOptions{}) + filesystems, err := ListFilesystems(context.Background(), ListOptions{}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -149,7 +149,7 @@ func TestVolumes(t *testing.T) { time.Sleep(time.Second) require.Equal(t, DatasetVolume, v.Type) - volumes, err := Volumes(context.Background(), ListOptions{}) + volumes, err := ListVolumes(context.Background(), ListOptions{}) require.NoError(t, err) for _, volume := range volumes { @@ -167,7 +167,7 @@ func TestSnapshot(t *testing.T) { }) require.NoError(t, err) - filesystems, err := Filesystems(context.Background(), ListOptions{}) + filesystems, err := ListFilesystems(context.Background(), ListOptions{}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -187,7 +187,8 @@ func TestSnapshot(t *testing.T) { func TestListingWithProperty(t *testing.T) { TestZPool(testZPool, func() { - const prop = "nl.test:bla" + const prop1 = "nl.test:bla" + const prop2 = "nl.test:hoi" root, err := GetDataset(context.Background(), testZPool) require.NoError(t, err) @@ -196,26 +197,28 @@ func TestListingWithProperty(t *testing.T) { Properties: noMountProps, }) require.NoError(t, err) - require.NoError(t, f1.SetProperty(context.Background(), prop, "123")) + require.NoError(t, f1.SetProperty(context.Background(), prop1, "123")) + require.NoError(t, f1.SetProperty(context.Background(), prop2, "wereld")) f2, err := CreateFilesystem(context.Background(), testZPool+"/list-test2", CreateFilesystemOptions{ Properties: noMountProps, }) require.NoError(t, err) - require.NoError(t, f2.SetProperty(context.Background(), prop, "321")) + require.NoError(t, f2.SetProperty(context.Background(), prop1, "321")) ds, err := root.Children(context.Background(), ListOptions{ DatasetType: DatasetFilesystem, - ExtraProperties: []string{prop}, + ExtraProperties: []string{prop1, prop2}, }) require.NoError(t, err) require.Len(t, ds, 2) require.Equal(t, f1.Name, ds[0].Name) - require.Equal(t, "123", ds[0].ExtraProps[prop]) + require.Equal(t, "123", ds[0].ExtraProps[prop1]) + require.Equal(t, "wereld", ds[0].ExtraProps[prop2]) require.Equal(t, f2.Name, ds[1].Name) - require.Equal(t, "321", ds[1].ExtraProps[prop]) + require.Equal(t, "321", ds[1].ExtraProps[prop1]) }) } @@ -245,7 +248,7 @@ func TestClone(t *testing.T) { }) require.NoError(t, err) - filesystems, err := Filesystems(context.Background(), ListOptions{}) + filesystems, err := ListFilesystems(context.Background(), ListOptions{}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -276,7 +279,7 @@ func TestSendSnapshot(t *testing.T) { }) require.NoError(t, err) - filesystems, err := Filesystems(context.Background(), ListOptions{}) + filesystems, err := ListFilesystems(context.Background(), ListOptions{}) require.NoError(t, err) for _, filesystem := range filesystems { @@ -319,7 +322,7 @@ func TestSendSnapshotResume(t *testing.T) { require.True(t, errors.As(err, &zfsErr)) require.NotEmpty(t, zfsErr.ResumeToken(), zfsErr) - list, err := Filesystems(context.Background(), ListOptions{ + list, err := ListFilesystems(context.Background(), ListOptions{ ParentDataset: testZPool + "/recv-test", ExtraProperties: []string{PropertyReceiveResumeToken}, }) @@ -346,7 +349,7 @@ func TestSendSnapshotResume(t *testing.T) { }) require.NoError(t, err) - snaps, err := Snapshots(context.Background(), ListOptions{ + snaps, err := ListSnapshots(context.Background(), ListOptions{ ParentDataset: testZPool + "/recv-test", Recursive: true, }) From 7342989639e12933f5e180032b43a0bd13c9dd44 Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Thu, 21 Mar 2024 10:17:10 +0100 Subject: [PATCH 10/12] Fix some bugs --- job/filesystem_prune.go | 2 +- job/filesystem_prune_test.go | 4 +--- job/snapshots_mark.go | 12 ++++++++++-- job/snapshots_send.go | 2 +- job/util.go | 6 ------ zfs.go | 1 - zfs_test.go | 4 ++-- 7 files changed, 15 insertions(+), 16 deletions(-) diff --git a/job/filesystem_prune.go b/job/filesystem_prune.go index 9676d5e..fce18cc 100644 --- a/job/filesystem_prune.go +++ b/job/filesystem_prune.go @@ -55,7 +55,7 @@ func (r *Runner) pruneAgedFilesystem(filesystem string) error { return nil // Not due for removal yet } - children, err := fs.Children(r.ctx, zfs.ListOptions{Depth: 1}) + children, err := fs.Children(r.ctx, zfs.ListOptions{}) if err != nil { return fmt.Errorf("error listing %s children: %w", filesystem, err) } diff --git a/job/filesystem_prune_test.go b/job/filesystem_prune_test.go index 1db4017..00bd5d8 100644 --- a/job/filesystem_prune_test.go +++ b/job/filesystem_prune_test.go @@ -71,9 +71,7 @@ func TestRunner_pruneFilesystems(t *testing.T) { ds, err := zfs.GetDataset(context.Background(), testZPool) require.NoError(t, err) - datasets, err := ds.Children(context.Background(), zfs.ListOptions{ - Depth: 1, - }) + datasets, err := ds.Children(context.Background(), zfs.ListOptions{}) require.NoError(t, err) require.Len(t, datasets, 5) require.Equal(t, fmt.Sprintf("%s/%s", testZPool, fsWithoutDel), datasets[0].Name) diff --git a/job/snapshots_mark.go b/job/snapshots_mark.go index 2b81a07..3476f21 100644 --- a/job/snapshots_mark.go +++ b/job/snapshots_mark.go @@ -2,6 +2,7 @@ package job import ( "fmt" + "slices" "time" zfs "github.com/vansante/go-zfsutils" @@ -66,8 +67,15 @@ func (r *Runner) markExcessDatasetSnapshots(ds *zfs.Dataset, maxCount int64) err return fmt.Errorf("error retrieving snapshots for %s: %w", ds.Name, err) } - // ListSnapshots are always retrieved with the newest last, so reverse the list: - reverseDatasets(snaps) + // Sort the list by created, newest first: + snaps, err = orderSnapshotsByCreated(snaps, createdProp) + if err != nil { + return fmt.Errorf("error sorting snapshots for %s: %w", ds.Name, err) + } + + // Snapshots are always retrieved with the newest last, so reverse the list: + slices.Reverse(snaps) + currentFound := int64(0) now := time.Now() for i := range snaps { diff --git a/job/snapshots_send.go b/job/snapshots_send.go index b5ff5da..1106651 100644 --- a/job/snapshots_send.go +++ b/job/snapshots_send.go @@ -22,7 +22,7 @@ func (r *Runner) sendSnapshots(routineID int) error { return fmt.Errorf("error finding snapshottable datasets: %w", err) } - for _, dataset := range datasets { + for dataset := range datasets { if r.ctx.Err() != nil { return nil // context expired, no problem } diff --git a/job/util.go b/job/util.go index 97ba9ab..d8304fb 100644 --- a/job/util.go +++ b/job/util.go @@ -88,12 +88,6 @@ func snapshotsContain(list []zfs.Dataset, dataset, snapshot string) bool { return false } -func reverseDatasets(s []zfs.Dataset) { - for i, j := 0, len(s)-1; i < j; i, j = i+1, j-1 { - s[i], s[j] = s[j], s[i] - } -} - // randomizeDuration adds or removes up to 5% of the duration to randomize background routine wake up times func randomizeDuration(d time.Duration) time.Duration { // nolint:unparam rnd := time.Duration(rand.Int63n(int64(d / 10))) diff --git a/zfs.go b/zfs.go index cd89b39..aad8fa9 100644 --- a/zfs.go +++ b/zfs.go @@ -57,7 +57,6 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { args = append(args, options.ParentDataset) } - fmt.Println(strings.Join(args, " ")) out, err := zfsOutput(ctx, args...) if err != nil { return nil, err diff --git a/zfs_test.go b/zfs_test.go index 4c94f3b..f93d860 100644 --- a/zfs_test.go +++ b/zfs_test.go @@ -42,7 +42,7 @@ func TestDatasetsWithProps(t *testing.T) { require.Len(t, ds.ExtraProps, 2, fmt.Sprintf("%#v", ds.ExtraProps)) require.Equal(t, "world", ds.ExtraProps["nl.test:hello"]) - require.Equal(t, "off", ds.ExtraProps["canmount"]) + require.Equal(t, "on", ds.ExtraProps["canmount"]) }) } @@ -226,7 +226,7 @@ func TestListWithProperty(t *testing.T) { TestZPool(testZPool, func() { const prop = "nl.test:bla" - f1, err := CreateFilesystem(context.Background(), testZPool+"/list-test1", CreateFilesystemOptions{ + f1, err := CreateFilesystem(context.Background(), testZPool+"/list-test", CreateFilesystemOptions{ Properties: noMountProps, }) require.NoError(t, err) From 9a35f81685772de658b8ff5797df4ab11558a6fd Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Thu, 21 Mar 2024 11:12:43 +0100 Subject: [PATCH 11/12] For types, list recursive by default --- zfs.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/zfs.go b/zfs.go index aad8fa9..7bc1337 100644 --- a/zfs.go +++ b/zfs.go @@ -80,6 +80,7 @@ func ListDatasets(ctx context.Context, options ListOptions) ([]Dataset, error) { // A filter argument may be passed to select a volume with the matching name, or empty string ("") may be used to select all volumes. func ListVolumes(ctx context.Context, options ListOptions) ([]Dataset, error) { options.DatasetType = DatasetVolume + options.Recursive = true return ListDatasets(ctx, options) } @@ -87,6 +88,7 @@ func ListVolumes(ctx context.Context, options ListOptions) ([]Dataset, error) { // A filter argument may be passed to select a filesystem with the matching name, or empty string ("") may be used to select all filesystems. func ListFilesystems(ctx context.Context, options ListOptions) ([]Dataset, error) { options.DatasetType = DatasetFilesystem + options.Recursive = true return ListDatasets(ctx, options) } @@ -94,6 +96,7 @@ func ListFilesystems(ctx context.Context, options ListOptions) ([]Dataset, error // A filter argument may be passed to select a snapshot with the matching name, or empty string ("") may be used to select all snapshots. func ListSnapshots(ctx context.Context, options ListOptions) ([]Dataset, error) { options.DatasetType = DatasetSnapshot + options.Recursive = true return ListDatasets(ctx, options) } From 0833adf3ed6bbf3d098b98215cc9c5dc695fb689 Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Thu, 21 Mar 2024 11:51:26 +0100 Subject: [PATCH 12/12] Improve and fix property set detection --- job/snapshots_create.go | 2 +- job/snapshots_mark.go | 10 +++++++--- job/snapshots_mark_test.go | 10 +++++----- job/snapshots_send.go | 10 +++++----- job/util.go | 7 ++++++- zfs.go | 4 ++-- 6 files changed, 26 insertions(+), 17 deletions(-) diff --git a/job/snapshots_create.go b/job/snapshots_create.go index 476a6db..7d72f39 100644 --- a/job/snapshots_create.go +++ b/job/snapshots_create.go @@ -69,7 +69,7 @@ func (r *Runner) createDatasetSnapshot(ds *zfs.Dataset) error { for i := range snapshots { snap := &snapshots[i] - if r.config.IgnoreSnapshotsWithoutCreatedProperty && snap.ExtraProps[createdProp] == zfs.PropertyUnset { + if r.config.IgnoreSnapshotsWithoutCreatedProperty && PropertyIsSet(snap.ExtraProps[createdProp]) { continue } diff --git a/job/snapshots_mark.go b/job/snapshots_mark.go index 3476f21..758ebbb 100644 --- a/job/snapshots_mark.go +++ b/job/snapshots_mark.go @@ -84,11 +84,15 @@ func (r *Runner) markExcessDatasetSnapshots(ds *zfs.Dataset, maxCount int64) err } snap := &snaps[i] - if snap.ExtraProps[createdProp] == zfs.PropertyUnset || snap.ExtraProps[deleteProp] != zfs.PropertyUnset { + if !PropertyIsSet(snap.ExtraProps[createdProp]) { continue // Ignore } - currentFound++ + + if PropertyIsSet(snap.ExtraProps[deleteProp]) { + continue // Already being deleted + } + if currentFound <= maxCount { continue // Not at the max yet } @@ -157,7 +161,7 @@ func (r *Runner) markAgingDatasetSnapshots(ds *zfs.Dataset, duration time.Durati } snap := &snaps[i] - if snap.ExtraProps[createdProp] == zfs.PropertyUnset || snap.ExtraProps[deleteProp] != zfs.PropertyUnset { + if !PropertyIsSet(snap.ExtraProps[createdProp]) || PropertyIsSet(snap.ExtraProps[deleteProp]) { continue // Ignore } diff --git a/job/snapshots_mark_test.go b/job/snapshots_mark_test.go index 8dfcfdd..c78d4f6 100644 --- a/job/snapshots_mark_test.go +++ b/job/snapshots_mark_test.go @@ -62,9 +62,9 @@ func TestRunner_markPrunableExcessSnapshots(t *testing.T) { require.WithinDuration(t, now, tm, time.Second) require.Equal(t, snap2, snapshotName(snaps[1].Name)) - require.Equal(t, zfs.PropertyUnset, snaps[1].ExtraProps[deleteProp]) + require.Equal(t, "", snaps[1].ExtraProps[deleteProp]) require.Equal(t, snap3, snapshotName(snaps[2].Name)) - require.Equal(t, zfs.PropertyUnset, snaps[2].ExtraProps[deleteProp]) + require.Equal(t, "", snaps[2].ExtraProps[deleteProp]) }) } @@ -123,10 +123,10 @@ func TestRunner_markPrunableSnapshotsByAge(t *testing.T) { require.WithinDuration(t, now, tm, time.Second) require.Equal(t, snap2, snapshotName(snaps[1].Name)) - require.Equal(t, zfs.PropertyUnset, snaps[1].ExtraProps[deleteProp]) + require.Equal(t, "", snaps[1].ExtraProps[deleteProp]) require.Equal(t, snap3, snapshotName(snaps[2].Name)) - require.Equal(t, zfs.PropertyUnset, snaps[2].ExtraProps[deleteProp]) + require.Equal(t, "", snaps[2].ExtraProps[deleteProp]) require.Equal(t, snap4, snapshotName(snaps[3].Name)) - require.Equal(t, zfs.PropertyUnset, snaps[3].ExtraProps[deleteProp]) + require.Equal(t, "", snaps[3].ExtraProps[deleteProp]) }) } diff --git a/job/snapshots_send.go b/job/snapshots_send.go index 1106651..52e5c34 100644 --- a/job/snapshots_send.go +++ b/job/snapshots_send.go @@ -33,7 +33,7 @@ func (r *Runner) sendSnapshots(routineID int) error { } server := ds.ExtraProps[sendToProp] - if server == "" || server == zfs.PropertyUnset { + if !PropertyIsSet(server) { continue // Dont know where to send this one ¯\_(ツ)_/¯ } @@ -82,10 +82,10 @@ func (r *Runner) sendDatasetSnapshots(routineID int, ds *zfs.Dataset) error { snapsUnsent := false for _, snap := range localSnaps { - if r.config.IgnoreSnapshotsWithoutCreatedProperty && snap.ExtraProps[createdProp] == zfs.PropertyUnset { + if r.config.IgnoreSnapshotsWithoutCreatedProperty && !PropertyIsSet(snap.ExtraProps[createdProp]) { continue } - if snap.ExtraProps[sentProp] == zfs.PropertyUnset { + if !PropertyIsSet(snap.ExtraProps[sentProp]) { snapsUnsent = true } } @@ -173,7 +173,7 @@ func (r *Runner) reconcileSnapshots(routineID int, local, remote []zfs.Dataset) for i := range local { snap := &local[i] remoteExists := snapshotsContain(remote, datasetName(snap.Name, true), snapshotName(snap.Name)) - localSent := snap.ExtraProps[sentProp] != zfs.PropertyUnset + localSent := PropertyIsSet(snap.ExtraProps[sentProp]) logger := r.logger.With( "routineID", routineID, @@ -218,7 +218,7 @@ func (r *Runner) reconcileSnapshots(routineID int, local, remote []zfs.Dataset) if err != nil { return nil, fmt.Errorf("error getting prop %s copy value for %s: %w", prop, snap.Name, err) } - if val == zfs.PropertyUnset { + if PropertyIsSet(val) { continue } props[prop] = val diff --git a/job/util.go b/job/util.go index d8304fb..89bd744 100644 --- a/job/util.go +++ b/job/util.go @@ -13,6 +13,11 @@ import ( zfs "github.com/vansante/go-zfsutils" ) +// PropertyIsSet returns whether a property is set +func PropertyIsSet(val string) bool { + return val != "" && val != zfs.PropertyUnset +} + func isContextError(err error) bool { return errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) } @@ -53,7 +58,7 @@ func snapshotName(name string) string { func filterSnapshotsWithProp(list []zfs.Dataset, prop string) []zfs.Dataset { nwList := make([]zfs.Dataset, 0, len(list)) for _, snap := range list { - if snap.ExtraProps[prop] == zfs.PropertyUnset { + if !PropertyIsSet(snap.ExtraProps[prop]) { continue } nwList = append(nwList, snap) diff --git a/zfs.go b/zfs.go index 7bc1337..8965661 100644 --- a/zfs.go +++ b/zfs.go @@ -116,7 +116,7 @@ func ListWithProperty(ctx context.Context, tp DatasetType, parentDataset, prop s case 2: result[line[0]] = line[1] case 1: - result[line[0]] = PropertyUnset + result[line[0]] = "" } } return result, nil @@ -613,7 +613,7 @@ func (d *Dataset) Rename(ctx context.Context, name string, options RenameOptions return GetDataset(ctx, name) } -// ListSnapshots returns a slice of all ZFS snapshots of a given dataset. +// Snapshots returns a slice of all ZFS snapshots of a given dataset. func (d *Dataset) Snapshots(ctx context.Context, options ListOptions) ([]Dataset, error) { options.ParentDataset = d.Name options.DatasetType = DatasetSnapshot