Skip to content

Commit

Permalink
More tests and code updates for new api
Browse files Browse the repository at this point in the history
  • Loading branch information
vansante committed Mar 21, 2024
1 parent 63cf9af commit e06f2ce
Show file tree
Hide file tree
Showing 14 changed files with 139 additions and 47 deletions.
2 changes: 1 addition & 1 deletion http/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions http/http_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
34 changes: 23 additions & 11 deletions http/http_handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
})
}

Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion job/filesystem_prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 3 additions & 1 deletion job/filesystem_prune_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion job/snapshots_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion job/snapshots_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions job/snapshots_mark.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions job/snapshots_mark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion job/snapshots_prune_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
16 changes: 12 additions & 4 deletions job/snapshots_send.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Check failure on line 36 in job/snapshots_send.go

View workflow job for this annotation

GitHub Actions / test (1.22.x, ubuntu-latest)

fmt.Errorf format %s has arg dataset of wrong type github.com/vansante/go-zfsutils.Dataset

Check failure on line 36 in job/snapshots_send.go

View workflow job for this annotation

GitHub Actions / lint

printf: fmt.Errorf format %s has arg dataset of wrong type github.com/vansante/go-zfsutils.Dataset (govet)

Check failure on line 36 in job/snapshots_send.go

View workflow job for this annotation

GitHub Actions / test (1.21.x, ubuntu-latest)

fmt.Errorf format %s has arg dataset of wrong type github.com/vansante/go-zfsutils.Dataset

Check failure on line 36 in job/snapshots_send.go

View workflow job for this annotation

GitHub Actions / test (1.22.x, ubuntu-latest)

fmt.Errorf format %s has arg dataset of wrong type github.com/vansante/go-zfsutils.Dataset
}
Expand Down Expand Up @@ -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)
}
Expand Down
12 changes: 9 additions & 3 deletions job/snapshots_send_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
53 changes: 39 additions & 14 deletions zfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -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)
}
Loading

0 comments on commit e06f2ce

Please sign in to comment.