From e06f2ce4b1671b285e4aaf3984656a413b97f0c2 Mon Sep 17 00:00:00 2001 From: Paul van Santen Date: Wed, 20 Mar 2024 17:41:48 +0100 Subject: [PATCH] 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{