From 24a693a33047fb887d7336cafc289986e10252e8 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Mon, 3 Feb 2025 21:42:54 -0500 Subject: [PATCH 1/9] Add support for writeable snapshots. --- api/v14/api_v14.go | 3 +- api/v14/api_v14_snapshots.go | 62 ++++++++++++++++++++++++++++++++++++ api/v14/api_v14_types.go | 22 +++++++++++++ snapshots.go | 36 +++++++++++++++++++++ snapshots_test.go | 32 +++++++++++++++++++ 5 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 api/v14/api_v14_snapshots.go diff --git a/api/v14/api_v14.go b/api/v14/api_v14.go index 36f68d55..9bb96570 100644 --- a/api/v14/api_v14.go +++ b/api/v14/api_v14.go @@ -17,5 +17,6 @@ limitations under the License. package v14 const ( - clusterAcsPath = "platform/14/cluster/acs" + clusterAcsPath = "platform/14/cluster/acs" + writeableSnapshotPath = "platform/14/snapshot/writable" ) diff --git a/api/v14/api_v14_snapshots.go b/api/v14/api_v14_snapshots.go new file mode 100644 index 00000000..f9f32b1a --- /dev/null +++ b/api/v14/api_v14_snapshots.go @@ -0,0 +1,62 @@ +/* +Copyright (c) 2025 Dell Inc, or its subsidiaries. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v14 + +import ( + "context" + "fmt" + + "github.com/dell/goisilon/api" +) + +// CreateWriteableSnapshot creates a writeable snapshot. +// +// ctx: the context. +// client: the API client. +// sourceSnapshot: the source snapshot name or ID. +// destination: the destination path, must not be nested under the source snapshot. +// +// Returns the response and error. +func CreateWriteableSnapshot( + ctx context.Context, + client api.Client, + sourceSnapshot string, + destination string, +) (resp *IsiWriteableSnapshot, err error) { + // PAPI calls: PUT https://1.2.3.4:8080//platform/14/snapshot/writable + // Body: {"src_snap": sourceSnapshot, "dst_path": destination} + + body := map[string]string{ + "src_snap": sourceSnapshot, + "dst_path": destination, + } + + err = client.Post(ctx, writeableSnapshotPath, "", nil, nil, body, &resp) + if err != nil { + return nil, fmt.Errorf("failed to create writeable snapshot: %v", err) + } + + return resp, err +} + +func RemoveWriteableSnapshot( + ctx context.Context, + client api.Client, + snapshotPath string, +) error { + return client.Delete(ctx, writeableSnapshotPath+snapshotPath, "", nil, nil, nil) +} diff --git a/api/v14/api_v14_types.go b/api/v14/api_v14_types.go index 9d83cc91..019d3861 100644 --- a/api/v14/api_v14_types.go +++ b/api/v14/api_v14_types.go @@ -31,3 +31,25 @@ type IsiClusterAcs struct { // list of unresponsive nodes serial number. UnresponsiveSn []string `json:"unresponsive_sn,omitempty"` } + +// An IsiWriteableSnapshot is a writable snapshot. +type IsiWriteableSnapshot struct { + // The Unix Epoch time the writable snapshot was created. + Created int64 `json:"created"` + // The /ifs path of user supplied source snapshot. This will be null for writable snapshots pending delete. + SrcPath string `json:"src_path"` + // The user supplied /ifs path of writable snapshot. + DstPath string `json:"dst_path"` + // The system ID given to the writable snapshot. + ID int64 `json:"id"` + // The system ID of the user supplied source snapshot. + SrcID int64 `json:"src_id"` + // The user supplied source snapshot name or ID. This will be null for writable snapshots pending delete. + SrcSnap string `json:"src_snap"` + // The sum in bytes of logical size of files in this writable snapshot. + LogSize int64 `json:"log_size"` + // The amount of storage in bytes used to store this writable snapshot. + PhysicalSize int64 `json:"phys_size"` + // Writable Snapshot state. + State string `json:"state"` +} diff --git a/snapshots.go b/snapshots.go index 95d2e2e2..1c36b73b 100644 --- a/snapshots.go +++ b/snapshots.go @@ -25,6 +25,7 @@ import ( "strings" api "github.com/dell/goisilon/api/v1" + apiv14 "github.com/dell/goisilon/api/v14" ) const ( @@ -38,6 +39,9 @@ type SnapshotList []*api.IsiSnapshot // Snapshot represents an Isilon snapshot. type Snapshot *api.IsiSnapshot +// WriteableSnapshot represents an Isilon writeable snapshot. +type WriteableSnapshot *apiv14.IsiWriteableSnapshot + // GetSnapshots returns a list of snapshots from the cluster. func (c *Client) GetSnapshots(ctx context.Context) (SnapshotList, error) { snapshots, err := api.GetIsiSnapshots(ctx, c.API) @@ -259,3 +263,35 @@ func (c *Client) GetSnapshotIsiPath( } return path.Join(zone.Path, snapShot, snapshot.Name, path.Base(snapshot.Path)), nil } + +// CreateWriteableSnapshot creates a writeable snapshot. +// +// ctx: the context. +// sourceSnapshot: the source snapshot name or ID. +// destination: the destination path, must not be nested under the source snapshot. +// +// Returns the snapshot on success and error in case of failure. +func (c *Client) CreateWriteableSnapshot( + ctx context.Context, + sourceSnapshot string, + destination string, +) (WriteableSnapshot, error) { + return apiv14.CreateWriteableSnapshot(ctx, c.API, sourceSnapshot, destination) +} + +// RemoveWriteableSnapshot removes a writeable snapshot. +// +// ctx: the context. +// snapshotPath: the path of the snapshot. +// +// Returns an error on failure. +func (c *Client) RemoveWriteableSnapshot( + ctx context.Context, + snapshotPath string, +) error { + if !strings.HasPrefix(snapshotPath, "/ifs/") { + return fmt.Errorf("invalid snapshot path, must start with /ifs: %s", snapshotPath) + } + + return apiv14.RemoveWriteableSnapshot(ctx, c.API, snapshotPath) +} diff --git a/snapshots_test.go b/snapshots_test.go index 85228d2e..fb5625d9 100644 --- a/snapshots_test.go +++ b/snapshots_test.go @@ -549,3 +549,35 @@ func TestSnapshotSizeGet(_ *testing.T) { panic(fmt.Sprintf("Snapshot folder size %d is not correct.\n", totalSize)) } } + +func TestCreateWriteableSnapshot(_ *testing.T) { + snapshotPath := "test_snapshot_create_volume" + snapshotName := "test_snapshot_create_snapshot" + writeableSnapshotName := "/ifs/data/csi/test_snapshot_create_writeable_snapshot" + + _, err := client.CreateVolume(defaultCtx, snapshotPath) + if err != nil { + panic(err) + } + defer client.DeleteVolume(defaultCtx, snapshotPath) + + snapshot, err := client.GetSnapshot(defaultCtx, -1, snapshotName) + if err == nil && snapshot != nil { + panic(fmt.Sprintf("Snapshot (%s) already exists.\n", snapshotName)) + } + + testSnapshot, err := client.CreateSnapshot( + defaultCtx, snapshotPath, snapshotName) + if err != nil { + panic(err) + } + defer client.RemoveSnapshot(defaultCtx, testSnapshot.ID, snapshotName) + + resp, err := client.CreateWriteableSnapshot(defaultCtx, snapshotName, writeableSnapshotName) + if err != nil { + panic(err) + } + defer client.RemoveWriteableSnapshot(defaultCtx, writeableSnapshotName) + + fmt.Printf("Writeable snapshot created: %s\n", resp.DstPath) +} From f05ccaa3ecb0225930481d59b2282aeaee8a9094 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Wed, 5 Feb 2025 14:56:49 -0500 Subject: [PATCH 2/9] Refactor get calls. --- api/v14/api_v14_snapshots.go | 60 ++++++++++++++++++++++++++++++++++++ api/v14/api_v14_types.go | 10 ++++++ snapshots.go | 33 +++++++++++++++++++- snapshots_test.go | 15 ++++++++- 4 files changed, 116 insertions(+), 2 deletions(-) diff --git a/api/v14/api_v14_snapshots.go b/api/v14/api_v14_snapshots.go index f9f32b1a..dc767b42 100644 --- a/api/v14/api_v14_snapshots.go +++ b/api/v14/api_v14_snapshots.go @@ -23,6 +23,66 @@ import ( "github.com/dell/goisilon/api" ) +// GetIsiWriteableSnapshots retrieves a list of writeable snapshots. +// +// ctx: the context. +// client: the API client. +// Returns a list of writeable snapshots and an error in case of failure. +func GetIsiWriteableSnapshots( + ctx context.Context, + client api.Client, +) ([]*IsiWriteableSnapshot, error) { + var resp *IsiWriteableSnapshotQueryResponse + err := client.Get(ctx, writeableSnapshotPath, "", nil, nil, &resp) + if err != nil { + return nil, fmt.Errorf("failed to get writeable snapshots from array: %v", err) + } + + result := make([]*IsiWriteableSnapshot, 0, resp.Total) + result = append(result, resp.Writeable...) + + for { + if resp.Resume == "" { + break + } + + query := api.OrderedValues{ + {[]byte("resume"), []byte(resp.Resume)}, + } + + resp = nil + err = client.Get(ctx, writeableSnapshotPath, "", query, nil, &resp) + if err != nil { + return nil, fmt.Errorf("failed to get writeable snapshots (query mode) from array: %v", err) + } + + result = append(result, resp.Writeable...) + } + + return result, err +} + +// GetIsiWriteableSnapshot retrieves a writeable snapshot. +// +// ctx: the context. +// client: the API client. +// snapshotPath: the path of the snapshot. +// +// Returns the snapshot on success and error in case of failure. +func GetIsiWriteableSnapshot( + ctx context.Context, + client api.Client, + snapshotPath string, +) (*IsiWriteableSnapshot, error) { + var resp *IsiWriteableSnapshotQueryResponse + err := client.Get(ctx, writeableSnapshotPath+snapshotPath, "", nil, nil, &resp) + if err != nil { + return nil, fmt.Errorf("failed to create writeable snapshot: %v", err) + } + + return resp.Writeable[0], nil +} + // CreateWriteableSnapshot creates a writeable snapshot. // // ctx: the context. diff --git a/api/v14/api_v14_types.go b/api/v14/api_v14_types.go index 019d3861..5946c308 100644 --- a/api/v14/api_v14_types.go +++ b/api/v14/api_v14_types.go @@ -53,3 +53,13 @@ type IsiWriteableSnapshot struct { // Writable Snapshot state. State string `json:"state"` } + +// IsiWriteableSnapshotQueryResponse is the response to a writable snapshot query. +type IsiWriteableSnapshotQueryResponse struct { + // Total number of items available. + Total int64 `json:"total,omitempty"` + // Used to continue a query. This is null for the last page. + Resume string `json:"resume,omitempty"` + // List of writable snapshots. + Writeable []*IsiWriteableSnapshot `json:"writable"` +} diff --git a/snapshots.go b/snapshots.go index 1c36b73b..ebf9b458 100644 --- a/snapshots.go +++ b/snapshots.go @@ -42,6 +42,9 @@ type Snapshot *api.IsiSnapshot // WriteableSnapshot represents an Isilon writeable snapshot. type WriteableSnapshot *apiv14.IsiWriteableSnapshot +// WriteableSnapshotList represents a list of Isilon writeable snapshots. +type WriteableSnapshotList []*apiv14.IsiWriteableSnapshot + // GetSnapshots returns a list of snapshots from the cluster. func (c *Client) GetSnapshots(ctx context.Context) (SnapshotList, error) { snapshots, err := api.GetIsiSnapshots(ctx, c.API) @@ -290,8 +293,36 @@ func (c *Client) RemoveWriteableSnapshot( snapshotPath string, ) error { if !strings.HasPrefix(snapshotPath, "/ifs/") { - return fmt.Errorf("invalid snapshot path, must start with /ifs: %s", snapshotPath) + return fmt.Errorf("invalid snapshot path, must start with /ifs/: %s", snapshotPath) } return apiv14.RemoveWriteableSnapshot(ctx, c.API, snapshotPath) } + +// GetWriteableSnapshot retrieves a writeable snapshot. +// +// ctx: the context. +// snapshotPath: the path of the snapshot. +// +// Returns the snapshot on success and error in case of failure. +func (c *Client) GetWriteableSnapshot( + ctx context.Context, + snapshotPath string, +) (WriteableSnapshot, error) { + if !strings.HasPrefix(snapshotPath, "/ifs/") { + return nil, fmt.Errorf("invalid snapshot path, must start with /ifs/: %s", snapshotPath) + } + + return apiv14.GetIsiWriteableSnapshot(ctx, c.API, snapshotPath) +} + +// GetWriteableSnapshots retrieves a list of all writeable snapshots. +// +// ctx: the context. +// +// Returns the list of writeable snapshots and an error in case of failure. +func (c *Client) GetWriteableSnapshots( + ctx context.Context, +) (WriteableSnapshotList, error) { + return apiv14.GetIsiWriteableSnapshots(ctx, c.API) +} diff --git a/snapshots_test.go b/snapshots_test.go index fb5625d9..b030d202 100644 --- a/snapshots_test.go +++ b/snapshots_test.go @@ -578,6 +578,19 @@ func TestCreateWriteableSnapshot(_ *testing.T) { panic(err) } defer client.RemoveWriteableSnapshot(defaultCtx, writeableSnapshotName) + fmt.Printf("Writeable snapshot created: %v\n", resp) - fmt.Printf("Writeable snapshot created: %s\n", resp.DstPath) + resp2, err2 := client.GetWriteableSnapshot(defaultCtx, writeableSnapshotName) + if err2 != nil { + panic(err) + } + fmt.Printf("GetWriteableSnapshot returned: %v\n", resp2) + + // Unrelated to the above but test ListWriteableSnapshots + resp3, err3 := client.GetWriteableSnapshots(defaultCtx) + if err3 != nil { + panic(err3) + } + + fmt.Printf("ListWriteableSnapshots returned %d items\n", len(resp3)) } From 6b4256507783b97c6ecf6fad492011179ffe3e48 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Fri, 14 Feb 2025 06:35:34 -0500 Subject: [PATCH 3/9] Correct error message. --- api/v14/api_v14_snapshots.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v14/api_v14_snapshots.go b/api/v14/api_v14_snapshots.go index dc767b42..14f7ab14 100644 --- a/api/v14/api_v14_snapshots.go +++ b/api/v14/api_v14_snapshots.go @@ -77,7 +77,7 @@ func GetIsiWriteableSnapshot( var resp *IsiWriteableSnapshotQueryResponse err := client.Get(ctx, writeableSnapshotPath+snapshotPath, "", nil, nil, &resp) if err != nil { - return nil, fmt.Errorf("failed to create writeable snapshot: %v", err) + return nil, fmt.Errorf("failed to get writeable snapshot: %v", err) } return resp.Writeable[0], nil From 00a9888ffd9c99a471c8f5ae4c1e862cfb4f2973 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Fri, 14 Feb 2025 06:54:44 -0500 Subject: [PATCH 4/9] Fix test. --- snapshots_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/snapshots_test.go b/snapshots_test.go index b030d202..00674de7 100644 --- a/snapshots_test.go +++ b/snapshots_test.go @@ -582,7 +582,7 @@ func TestCreateWriteableSnapshot(_ *testing.T) { resp2, err2 := client.GetWriteableSnapshot(defaultCtx, writeableSnapshotName) if err2 != nil { - panic(err) + panic(err2) } fmt.Printf("GetWriteableSnapshot returned: %v\n", resp2) @@ -593,4 +593,7 @@ func TestCreateWriteableSnapshot(_ *testing.T) { } fmt.Printf("ListWriteableSnapshots returned %d items\n", len(resp3)) + for _, v := range resp3 { + fmt.Fprintf(os.Stderr, "%v\n", v) + } } From e34db273e1849df6f5a37eddf779ee80e98c1d1a Mon Sep 17 00:00:00 2001 From: Don Khan Date: Sat, 15 Feb 2025 22:20:33 -0500 Subject: [PATCH 5/9] Use more common spelling - writable. --- api/v14/api_v14.go | 4 +-- api/v14/api_v14_snapshots.go | 52 ++++++++++++++++++------------------ api/v14/api_v14_types.go | 17 +++++++----- snapshots.go | 40 +++++++++++++-------------- snapshots_test.go | 20 +++++++------- 5 files changed, 69 insertions(+), 64 deletions(-) diff --git a/api/v14/api_v14.go b/api/v14/api_v14.go index 9bb96570..30d21167 100644 --- a/api/v14/api_v14.go +++ b/api/v14/api_v14.go @@ -17,6 +17,6 @@ limitations under the License. package v14 const ( - clusterAcsPath = "platform/14/cluster/acs" - writeableSnapshotPath = "platform/14/snapshot/writable" + clusterAcsPath = "platform/14/cluster/acs" + writableSnapshotPath = "platform/14/snapshot/writable" ) diff --git a/api/v14/api_v14_snapshots.go b/api/v14/api_v14_snapshots.go index 14f7ab14..bef4b28c 100644 --- a/api/v14/api_v14_snapshots.go +++ b/api/v14/api_v14_snapshots.go @@ -23,23 +23,23 @@ import ( "github.com/dell/goisilon/api" ) -// GetIsiWriteableSnapshots retrieves a list of writeable snapshots. +// GetIsiWritableSnapshots retrieves a list of writable snapshots. // // ctx: the context. // client: the API client. -// Returns a list of writeable snapshots and an error in case of failure. -func GetIsiWriteableSnapshots( +// Returns a list of writable snapshots and an error in case of failure. +func GetIsiWritableSnapshots( ctx context.Context, client api.Client, -) ([]*IsiWriteableSnapshot, error) { - var resp *IsiWriteableSnapshotQueryResponse - err := client.Get(ctx, writeableSnapshotPath, "", nil, nil, &resp) +) ([]*IsiWritableSnapshot, error) { + var resp *IsiWritableSnapshotQueryResponse + err := client.Get(ctx, writableSnapshotPath, "", nil, nil, &resp) if err != nil { - return nil, fmt.Errorf("failed to get writeable snapshots from array: %v", err) + return nil, fmt.Errorf("failed to get writable snapshots from array: %v", err) } - result := make([]*IsiWriteableSnapshot, 0, resp.Total) - result = append(result, resp.Writeable...) + result := make([]*IsiWritableSnapshot, 0, resp.Total) + result = append(result, resp.Writable...) for { if resp.Resume == "" { @@ -51,39 +51,39 @@ func GetIsiWriteableSnapshots( } resp = nil - err = client.Get(ctx, writeableSnapshotPath, "", query, nil, &resp) + err = client.Get(ctx, writableSnapshotPath, "", query, nil, &resp) if err != nil { - return nil, fmt.Errorf("failed to get writeable snapshots (query mode) from array: %v", err) + return nil, fmt.Errorf("failed to get writable snapshots (query mode) from array: %v", err) } - result = append(result, resp.Writeable...) + result = append(result, resp.Writable...) } return result, err } -// GetIsiWriteableSnapshot retrieves a writeable snapshot. +// GetIsiWritableSnapshot retrieves a writable snapshot. // // ctx: the context. // client: the API client. // snapshotPath: the path of the snapshot. // // Returns the snapshot on success and error in case of failure. -func GetIsiWriteableSnapshot( +func GetIsiWritableSnapshot( ctx context.Context, client api.Client, snapshotPath string, -) (*IsiWriteableSnapshot, error) { - var resp *IsiWriteableSnapshotQueryResponse - err := client.Get(ctx, writeableSnapshotPath+snapshotPath, "", nil, nil, &resp) +) (*IsiWritableSnapshot, error) { + var resp *IsiWritableSnapshotQueryResponse + err := client.Get(ctx, writableSnapshotPath+snapshotPath, "", nil, nil, &resp) if err != nil { - return nil, fmt.Errorf("failed to get writeable snapshot: %v", err) + return nil, fmt.Errorf("failed to get writable snapshot: %v", err) } - return resp.Writeable[0], nil + return resp.Writable[0], nil } -// CreateWriteableSnapshot creates a writeable snapshot. +// CreateWritableSnapshot creates a writable snapshot. // // ctx: the context. // client: the API client. @@ -91,12 +91,12 @@ func GetIsiWriteableSnapshot( // destination: the destination path, must not be nested under the source snapshot. // // Returns the response and error. -func CreateWriteableSnapshot( +func CreateWritableSnapshot( ctx context.Context, client api.Client, sourceSnapshot string, destination string, -) (resp *IsiWriteableSnapshot, err error) { +) (resp *IsiWritableSnapshot, err error) { // PAPI calls: PUT https://1.2.3.4:8080//platform/14/snapshot/writable // Body: {"src_snap": sourceSnapshot, "dst_path": destination} @@ -105,18 +105,18 @@ func CreateWriteableSnapshot( "dst_path": destination, } - err = client.Post(ctx, writeableSnapshotPath, "", nil, nil, body, &resp) + err = client.Post(ctx, writableSnapshotPath, "", nil, nil, body, &resp) if err != nil { - return nil, fmt.Errorf("failed to create writeable snapshot: %v", err) + return nil, fmt.Errorf("failed to create writable snapshot: %v", err) } return resp, err } -func RemoveWriteableSnapshot( +func RemoveWritableSnapshot( ctx context.Context, client api.Client, snapshotPath string, ) error { - return client.Delete(ctx, writeableSnapshotPath+snapshotPath, "", nil, nil, nil) + return client.Delete(ctx, writableSnapshotPath+snapshotPath, "", nil, nil, nil) } diff --git a/api/v14/api_v14_types.go b/api/v14/api_v14_types.go index 5946c308..b6988fd2 100644 --- a/api/v14/api_v14_types.go +++ b/api/v14/api_v14_types.go @@ -32,8 +32,13 @@ type IsiClusterAcs struct { UnresponsiveSn []string `json:"unresponsive_sn,omitempty"` } -// An IsiWriteableSnapshot is a writable snapshot. -type IsiWriteableSnapshot struct { +const ( + WritableSnapshotStateActive = "active" + WritableSnapshotStateDeleting = "deleting" +) + +// An IsiWritableSnapshot is a writable snapshot. +type IsiWritableSnapshot struct { // The Unix Epoch time the writable snapshot was created. Created int64 `json:"created"` // The /ifs path of user supplied source snapshot. This will be null for writable snapshots pending delete. @@ -50,16 +55,16 @@ type IsiWriteableSnapshot struct { LogSize int64 `json:"log_size"` // The amount of storage in bytes used to store this writable snapshot. PhysicalSize int64 `json:"phys_size"` - // Writable Snapshot state. + // Writable Snapshot state. [WritableSnapshotStateActive, WritableSnapshotStateDeleting] State string `json:"state"` } -// IsiWriteableSnapshotQueryResponse is the response to a writable snapshot query. -type IsiWriteableSnapshotQueryResponse struct { +// IsiWritableSnapshotQueryResponse is the response to a writable snapshot query. +type IsiWritableSnapshotQueryResponse struct { // Total number of items available. Total int64 `json:"total,omitempty"` // Used to continue a query. This is null for the last page. Resume string `json:"resume,omitempty"` // List of writable snapshots. - Writeable []*IsiWriteableSnapshot `json:"writable"` + Writable []*IsiWritableSnapshot `json:"writable"` } diff --git a/snapshots.go b/snapshots.go index ebf9b458..3c300e86 100644 --- a/snapshots.go +++ b/snapshots.go @@ -39,11 +39,11 @@ type SnapshotList []*api.IsiSnapshot // Snapshot represents an Isilon snapshot. type Snapshot *api.IsiSnapshot -// WriteableSnapshot represents an Isilon writeable snapshot. -type WriteableSnapshot *apiv14.IsiWriteableSnapshot +// WritableSnapshot represents an Isilon writable snapshot. +type WritableSnapshot *apiv14.IsiWritableSnapshot -// WriteableSnapshotList represents a list of Isilon writeable snapshots. -type WriteableSnapshotList []*apiv14.IsiWriteableSnapshot +// WritableSnapshotList represents a list of Isilon writable snapshots. +type WritableSnapshotList []*apiv14.IsiWritableSnapshot // GetSnapshots returns a list of snapshots from the cluster. func (c *Client) GetSnapshots(ctx context.Context) (SnapshotList, error) { @@ -267,28 +267,28 @@ func (c *Client) GetSnapshotIsiPath( return path.Join(zone.Path, snapShot, snapshot.Name, path.Base(snapshot.Path)), nil } -// CreateWriteableSnapshot creates a writeable snapshot. +// CreateWritableSnapshot creates a writable snapshot. // // ctx: the context. // sourceSnapshot: the source snapshot name or ID. // destination: the destination path, must not be nested under the source snapshot. // // Returns the snapshot on success and error in case of failure. -func (c *Client) CreateWriteableSnapshot( +func (c *Client) CreateWritableSnapshot( ctx context.Context, sourceSnapshot string, destination string, -) (WriteableSnapshot, error) { - return apiv14.CreateWriteableSnapshot(ctx, c.API, sourceSnapshot, destination) +) (WritableSnapshot, error) { + return apiv14.CreateWritableSnapshot(ctx, c.API, sourceSnapshot, destination) } -// RemoveWriteableSnapshot removes a writeable snapshot. +// RemoveWritableSnapshot removes a writable snapshot. // // ctx: the context. // snapshotPath: the path of the snapshot. // // Returns an error on failure. -func (c *Client) RemoveWriteableSnapshot( +func (c *Client) RemoveWritableSnapshot( ctx context.Context, snapshotPath string, ) error { @@ -296,33 +296,33 @@ func (c *Client) RemoveWriteableSnapshot( return fmt.Errorf("invalid snapshot path, must start with /ifs/: %s", snapshotPath) } - return apiv14.RemoveWriteableSnapshot(ctx, c.API, snapshotPath) + return apiv14.RemoveWritableSnapshot(ctx, c.API, snapshotPath) } -// GetWriteableSnapshot retrieves a writeable snapshot. +// GetWritableSnapshot retrieves a writable snapshot. // // ctx: the context. // snapshotPath: the path of the snapshot. // // Returns the snapshot on success and error in case of failure. -func (c *Client) GetWriteableSnapshot( +func (c *Client) GetWritableSnapshot( ctx context.Context, snapshotPath string, -) (WriteableSnapshot, error) { +) (WritableSnapshot, error) { if !strings.HasPrefix(snapshotPath, "/ifs/") { return nil, fmt.Errorf("invalid snapshot path, must start with /ifs/: %s", snapshotPath) } - return apiv14.GetIsiWriteableSnapshot(ctx, c.API, snapshotPath) + return apiv14.GetIsiWritableSnapshot(ctx, c.API, snapshotPath) } -// GetWriteableSnapshots retrieves a list of all writeable snapshots. +// GetWritableSnapshots retrieves a list of all writable snapshots. // // ctx: the context. // -// Returns the list of writeable snapshots and an error in case of failure. -func (c *Client) GetWriteableSnapshots( +// Returns the list of writable snapshots and an error in case of failure. +func (c *Client) GetWritableSnapshots( ctx context.Context, -) (WriteableSnapshotList, error) { - return apiv14.GetIsiWriteableSnapshots(ctx, c.API) +) (WritableSnapshotList, error) { + return apiv14.GetIsiWritableSnapshots(ctx, c.API) } diff --git a/snapshots_test.go b/snapshots_test.go index 00674de7..669ef8ea 100644 --- a/snapshots_test.go +++ b/snapshots_test.go @@ -550,10 +550,10 @@ func TestSnapshotSizeGet(_ *testing.T) { } } -func TestCreateWriteableSnapshot(_ *testing.T) { +func TestCreateWritableSnapshot(_ *testing.T) { snapshotPath := "test_snapshot_create_volume" snapshotName := "test_snapshot_create_snapshot" - writeableSnapshotName := "/ifs/data/csi/test_snapshot_create_writeable_snapshot" + writableSnapshotName := "/ifs/data/csi/test_snapshot_create_writable_snapshot" _, err := client.CreateVolume(defaultCtx, snapshotPath) if err != nil { @@ -573,26 +573,26 @@ func TestCreateWriteableSnapshot(_ *testing.T) { } defer client.RemoveSnapshot(defaultCtx, testSnapshot.ID, snapshotName) - resp, err := client.CreateWriteableSnapshot(defaultCtx, snapshotName, writeableSnapshotName) + resp, err := client.CreateWritableSnapshot(defaultCtx, snapshotName, writableSnapshotName) if err != nil { panic(err) } - defer client.RemoveWriteableSnapshot(defaultCtx, writeableSnapshotName) - fmt.Printf("Writeable snapshot created: %v\n", resp) + defer client.RemoveWritableSnapshot(defaultCtx, writableSnapshotName) + fmt.Printf("Writable snapshot created: %v\n", resp) - resp2, err2 := client.GetWriteableSnapshot(defaultCtx, writeableSnapshotName) + resp2, err2 := client.GetWritableSnapshot(defaultCtx, writableSnapshotName) if err2 != nil { panic(err2) } - fmt.Printf("GetWriteableSnapshot returned: %v\n", resp2) + fmt.Printf("GetWritableSnapshot returned: %v\n", resp2) - // Unrelated to the above but test ListWriteableSnapshots - resp3, err3 := client.GetWriteableSnapshots(defaultCtx) + // Unrelated to the above but test ListWritableSnapshots + resp3, err3 := client.GetWritableSnapshots(defaultCtx) if err3 != nil { panic(err3) } - fmt.Printf("ListWriteableSnapshots returned %d items\n", len(resp3)) + fmt.Printf("ListWritableSnapshots returned %d items\n", len(resp3)) for _, v := range resp3 { fmt.Fprintf(os.Stderr, "%v\n", v) } From f64c1974074f6b4a8149d70f53e0c463a55d4fd3 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Mon, 17 Feb 2025 08:02:58 -0500 Subject: [PATCH 6/9] Include snapshots in quotas. --- api/v1/api_v1_quotas.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/v1/api_v1_quotas.go b/api/v1/api_v1_quotas.go index 737d2cc7..ca1f3519 100644 --- a/api/v1/api_v1_quotas.go +++ b/api/v1/api_v1_quotas.go @@ -126,7 +126,7 @@ func CreateIsiQuota( ) (string, error) { // PAPI call: POST https://1.2.3.4:8080/platform/1/quota/quotas // { "enforced" : true, - // "include_snapshots" : false, + // "include_snapshots" : true, // "path" : "/ifs/volumes/volume_name", // "container" : true, // "thresholds_include_overhead" : false, @@ -136,7 +136,7 @@ func CreateIsiQuota( // "soft" : null // } // } - // body={'path': '/ifs/data/quotatest', 'thresholds': {'soft_grace': 86400L, 'soft': 1048576L}, 'include_snapshots': False, 'force': False, 'type': 'directory'} + // body={'path': '/ifs/data/quotatest', 'thresholds': {'soft_grace': 86400L, 'soft': 1048576L}, 'include_snapshots': True, 'force': False, 'type': 'directory'} // softGrace := 86400U thresholds := isiThresholdsReq{Advisory: advisoryLimit, Hard: size, Soft: softLimit, SoftGrace: softGracePrd} if advisoryLimit == 0 { @@ -150,7 +150,7 @@ func CreateIsiQuota( } data := &IsiQuotaReq{ Enforced: true, - IncludeSnapshots: false, + IncludeSnapshots: true, Path: path, Container: container, ThresholdsIncludeOverhead: false, From fa35ad0ee2938daca990e7fe14fee482d7bb6fd3 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Mon, 17 Feb 2025 08:13:37 -0500 Subject: [PATCH 7/9] Expose include_snapshot to public API. --- api/v1/api_v1_quotas.go | 5 +++-- quota.go | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/api/v1/api_v1_quotas.go b/api/v1/api_v1_quotas.go index ca1f3519..34e0ed03 100644 --- a/api/v1/api_v1_quotas.go +++ b/api/v1/api_v1_quotas.go @@ -123,6 +123,7 @@ func CreateIsiQuota( ctx context.Context, client api.Client, path string, container bool, size, softLimit, advisoryLimit, softGracePrd int64, + includeSnapshots bool, ) (string, error) { // PAPI call: POST https://1.2.3.4:8080/platform/1/quota/quotas // { "enforced" : true, @@ -150,7 +151,7 @@ func CreateIsiQuota( } data := &IsiQuotaReq{ Enforced: true, - IncludeSnapshots: true, + IncludeSnapshots: includeSnapshots, Path: path, Container: container, ThresholdsIncludeOverhead: false, @@ -170,7 +171,7 @@ func SetIsiQuotaHardThreshold( client api.Client, path string, size, softLimit, advisoryLimit, softGracePrd int64, ) (string, error) { - return CreateIsiQuota(ctx, client, path, false, size, softLimit, advisoryLimit, softGracePrd) + return CreateIsiQuota(ctx, client, path, false, size, softLimit, advisoryLimit, softGracePrd, true) } // UpdateIsiQuotaHardThreshold modifies the hard threshold of a quota for a directory diff --git a/quota.go b/quota.go index 9f6635d8..63d3992e 100644 --- a/quota.go +++ b/quota.go @@ -86,19 +86,19 @@ func (c *Client) GetQuotaWithPath(ctx context.Context, path string) (Quota, erro // CreateQuota creates a new hard directory quota with the specified size // and container option func (c *Client) CreateQuota( - ctx context.Context, name string, container bool, size, softLimit, advisoryLimit, softGracePrd int64, + ctx context.Context, name string, container bool, size, softLimit, advisoryLimit, softGracePrd int64, includeSnapshots bool, ) (string, error) { return api.CreateIsiQuota( - ctx, c.API, c.API.VolumePath(name), container, size, softLimit, advisoryLimit, softGracePrd) + ctx, c.API, c.API.VolumePath(name), container, size, softLimit, advisoryLimit, softGracePrd, includeSnapshots) } // CreateQuotaWithPath creates a new hard directory quota with the specified size // and container option func (c *Client) CreateQuotaWithPath( - ctx context.Context, path string, container bool, size, softLimit, advisoryLimit, softGracePrd int64, + ctx context.Context, path string, container bool, size, softLimit, advisoryLimit, softGracePrd int64, includeSnapshots bool, ) (string, error) { return api.CreateIsiQuota( - ctx, c.API, path, container, size, softLimit, advisoryLimit, softGracePrd) + ctx, c.API, path, container, size, softLimit, advisoryLimit, softGracePrd, includeSnapshots) } // SetQuotaSize sets the max size (hard threshold) of a quota for a volume From 2318c2764cdb1b455c17bfdbbf6ea8360f769c75 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Tue, 18 Feb 2025 01:34:09 -0500 Subject: [PATCH 8/9] Add unit tests for writable snapshots. --- snapshots_test.go | 136 +++++++++++++++++++++++++++++++++------------- 1 file changed, 97 insertions(+), 39 deletions(-) diff --git a/snapshots_test.go b/snapshots_test.go index 669ef8ea..0023b15a 100644 --- a/snapshots_test.go +++ b/snapshots_test.go @@ -16,11 +16,16 @@ limitations under the License. package goisilon import ( + "context" + "errors" "fmt" "os" "testing" apiv1 "github.com/dell/goisilon/api/v1" + "github.com/dell/gounity/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func TestSnapshotsGet(_ *testing.T) { @@ -550,50 +555,103 @@ func TestSnapshotSizeGet(_ *testing.T) { } } -func TestCreateWritableSnapshot(_ *testing.T) { - snapshotPath := "test_snapshot_create_volume" - snapshotName := "test_snapshot_create_snapshot" - writableSnapshotName := "/ifs/data/csi/test_snapshot_create_writable_snapshot" +func TestGetWritableSnapshots(t *testing.T) { + client.API.(*mocks.Client).ExpectedCalls = nil + + client.API.(*mocks.Client).On("Get", anyArgs[0:6]...).Return(nil).Run(func(args mock.Arguments) { + resp := args.Get(5).(**apiv14.IsiWritableSnapshotQueryResponse) + *resp = &apiv14.IsiWritableSnapshotQueryResponse{ + Writable: []*apiv14.IsiWritableSnapshot{ + { + ID: 100, + }, + }, + } + }).Once() - _, err := client.CreateVolume(defaultCtx, snapshotPath) - if err != nil { - panic(err) - } - defer client.DeleteVolume(defaultCtx, snapshotPath) + result, err := client.GetWritableSnapshots(context.Background()) + assert.Nil(t, err) + assert.Len(t, result, 1) + assert.Equal(t, int64(100), result[0].ID) +} - snapshot, err := client.GetSnapshot(defaultCtx, -1, snapshotName) - if err == nil && snapshot != nil { - panic(fmt.Sprintf("Snapshot (%s) already exists.\n", snapshotName)) - } +func TestGetWritableSnapshot(t *testing.T) { + client.API.(*mocks.Client).ExpectedCalls = nil + + client.API.(*mocks.Client).On("Get", anyArgs[0:6]...).Return(nil).Run(func(args mock.Arguments) { + resp := args.Get(5).(**apiv14.IsiWritableSnapshotQueryResponse) + *resp = &apiv14.IsiWritableSnapshotQueryResponse{ + Writable: []*apiv14.IsiWritableSnapshot{ + { + ID: 100, + }, + }, + } + }).Once() - testSnapshot, err := client.CreateSnapshot( - defaultCtx, snapshotPath, snapshotName) - if err != nil { - panic(err) - } - defer client.RemoveSnapshot(defaultCtx, testSnapshot.ID, snapshotName) + result, err := client.GetWritableSnapshot(context.Background(), "/ifs/data1") + assert.Nil(t, err) + assert.Equal(t, int64(100), result.ID) - resp, err := client.CreateWritableSnapshot(defaultCtx, snapshotName, writableSnapshotName) - if err != nil { - panic(err) - } - defer client.RemoveWritableSnapshot(defaultCtx, writableSnapshotName) - fmt.Printf("Writable snapshot created: %v\n", resp) + result, err = client.GetWritableSnapshot(context.Background(), "/data1") + assert.NotNil(t, err) + assert.ErrorContains(t, err, "invalid snapshot path, must start with /ifs/: /data1") +} - resp2, err2 := client.GetWritableSnapshot(defaultCtx, writableSnapshotName) - if err2 != nil { - panic(err2) - } - fmt.Printf("GetWritableSnapshot returned: %v\n", resp2) +func TestCreateWritableSnapshot(t *testing.T) { + client.API.(*mocks.Client).ExpectedCalls = nil + + snapshotPath := "/ifs/data1" + sourceSnapshot := "snapshot_source_1" + destinationPath := "/ifs/data2" + + client.API.(*mocks.Client).On("Post", anyArgs[0:7]...).Return(nil).Run(func(args mock.Arguments) { + body := args.Get(5).(map[string]string) + assert.Equal(t, sourceSnapshot, body["src_snap"]) + assert.Equal(t, destinationPath, body["dst_path"]) + + resp := args.Get(6).(**apiv14.IsiWritableSnapshot) + *resp = &apiv14.IsiWritableSnapshot{ + ID: 100, + SrcPath: snapshotPath, + DstPath: destinationPath, + SrcSnap: sourceSnapshot, + State: apiv14.WritableSnapshotStateActive, + LogSize: 100, + PhysicalSize: 200, + } + }).Once() + + result, err := client.CreateWritableSnapshot(context.Background(), sourceSnapshot, destinationPath) + assert.Nil(t, err) + assert.Equal(t, int64(100), result.ID) + assert.Equal(t, snapshotPath, result.SrcPath) + assert.Equal(t, destinationPath, result.DstPath) + assert.Equal(t, sourceSnapshot, result.SrcSnap) + assert.Equal(t, apiv14.WritableSnapshotStateActive, result.State) + assert.Equal(t, int64(100), result.LogSize) + assert.Equal(t, int64(200), result.PhysicalSize) + + // Test case: error in API call + client.API.(*mocks.Client).On("Post", anyArgs[0:7]...).Return(errors.New("API call failed")).Once() + + result, err = client.CreateWritableSnapshot(context.Background(), sourceSnapshot, destinationPath) + assert.ErrorContains(t, err, "API call failed") + assert.Nil(t, result) +} - // Unrelated to the above but test ListWritableSnapshots - resp3, err3 := client.GetWritableSnapshots(defaultCtx) - if err3 != nil { - panic(err3) - } +func TestRemoveWritableSnapshot(t *testing.T) { + client.API.(*mocks.Client).ExpectedCalls = nil - fmt.Printf("ListWritableSnapshots returned %d items\n", len(resp3)) - for _, v := range resp3 { - fmt.Fprintf(os.Stderr, "%v\n", v) - } + client.API.(*mocks.Client).On("Delete", anyArgs[0:6]...).Return(nil).Run(func(args mock.Arguments) { + tgt := args.Get(1).(string) + assert.Equal(t, "platform/14/snapshot/writable/ifs/data1", tgt) + }).Once() + + err := client.RemoveWritableSnapshot(context.Background(), "/ifs/data1") + assert.Nil(t, err) + + // Test case: error in API call + err = client.RemoveWritableSnapshot(context.Background(), "/data1") + assert.ErrorContains(t, err, "invalid snapshot path, must start with /ifs/: /data1") } From d254f9a026128bfa6632bd406e9b065e1bc3b7f4 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Wed, 19 Feb 2025 19:08:18 -0500 Subject: [PATCH 9/9] Add unit tests for writable snapshots. --- api/v14/api_v14_snapshots_test.go | 173 ++++++++++++++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 api/v14/api_v14_snapshots_test.go diff --git a/api/v14/api_v14_snapshots_test.go b/api/v14/api_v14_snapshots_test.go new file mode 100644 index 00000000..40539792 --- /dev/null +++ b/api/v14/api_v14_snapshots_test.go @@ -0,0 +1,173 @@ +/* +Copyright (c) 2025 Dell Inc, or its subsidiaries. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v14 + +import ( + "context" + "errors" + "testing" + + "github.com/dell/goisilon/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +func TestGetWritableSnapshots_ErrorCase1(t *testing.T) { + ctx := context.Background() + client := &mocks.Client{} + client.On("Get", anyArgs...).Return(errors.New("unauthorized")).Once() + _, err := GetIsiWritableSnapshots(ctx, client) + assert.NotNil(t, err) +} + +func TestGetWritableSnapshots_ErrorCase2(t *testing.T) { + ctx := context.Background() + client := &mocks.Client{} + + client.On("Get", anyArgs[0:6]...).Return(nil).Run(func(args mock.Arguments) { + resp := args.Get(5).(**IsiWritableSnapshotQueryResponse) + *resp = &IsiWritableSnapshotQueryResponse{ + Writable: []*IsiWritableSnapshot{ + { + ID: 100, + }, + }, + Total: 2048, + Resume: "resume", + } + }).Once() + + client.On("Get", anyArgs...).Return(errors.New("invalid")).Once() + _, err := GetIsiWritableSnapshots(ctx, client) + assert.NotNil(t, err) +} + +func TestGetWritableSnapshots(t *testing.T) { + ctx := context.Background() + client := &mocks.Client{} + + client.On("Get", anyArgs[0:6]...).Return(nil).Run(func(args mock.Arguments) { + resp := args.Get(5).(**IsiWritableSnapshotQueryResponse) + *resp = &IsiWritableSnapshotQueryResponse{ + Writable: []*IsiWritableSnapshot{ + { + ID: 100, + }, + }, + Total: 2048, + Resume: "resume", + } + }).Once() + + client.On("Get", anyArgs[0:6]...).Return(nil).Run(func(args mock.Arguments) { + resp := args.Get(5).(**IsiWritableSnapshotQueryResponse) + *resp = &IsiWritableSnapshotQueryResponse{ + Writable: []*IsiWritableSnapshot{ + { + ID: 1000000, + }, + }, + Total: 1, + Resume: "", + } + }).Once() + + result, err := GetIsiWritableSnapshots(ctx, client) + assert.Nil(t, err) + assert.Len(t, result, 2) + assert.Equal(t, int64(100), result[0].ID) +} + +func TestGetWritableSnapshot(t *testing.T) { + ctx := context.Background() + client := &mocks.Client{} + + client.On("Get", anyArgs[0:6]...).Return(nil).Run(func(args mock.Arguments) { + resp := args.Get(5).(**IsiWritableSnapshotQueryResponse) + *resp = &IsiWritableSnapshotQueryResponse{ + Writable: []*IsiWritableSnapshot{ + { + ID: 100, + }, + }, + } + }).Once() + + result, err := GetIsiWritableSnapshot(ctx, client, "/ifs/data1") + assert.Nil(t, err) + assert.Equal(t, int64(100), result.ID) + + client.On("Get", anyArgs...).Return(errors.New("not found")).Once() + result, err = GetIsiWritableSnapshot(ctx, client, "/ifs/data1") + assert.NotNil(t, err) +} + +func TestCreateWritableSnapshot(t *testing.T) { + ctx := context.Background() + client := &mocks.Client{} + + snapshotPath := "/ifs/data1" + sourceSnapshot := "snapshot_source_1" + destinationPath := "/ifs/data2" + + client.On("Post", anyArgs[0:7]...).Return(nil).Run(func(args mock.Arguments) { + body := args.Get(5).(map[string]string) + assert.Equal(t, sourceSnapshot, body["src_snap"]) + assert.Equal(t, destinationPath, body["dst_path"]) + + resp := args.Get(6).(**IsiWritableSnapshot) + *resp = &IsiWritableSnapshot{ + ID: 100, + SrcPath: snapshotPath, + DstPath: destinationPath, + SrcSnap: sourceSnapshot, + State: WritableSnapshotStateActive, + LogSize: 100, + PhysicalSize: 200, + } + }).Once() + + result, err := CreateWritableSnapshot(ctx, client, sourceSnapshot, destinationPath) + assert.Nil(t, err) + assert.Equal(t, int64(100), result.ID) + assert.Equal(t, snapshotPath, result.SrcPath) + assert.Equal(t, destinationPath, result.DstPath) + assert.Equal(t, sourceSnapshot, result.SrcSnap) + assert.Equal(t, WritableSnapshotStateActive, result.State) + assert.Equal(t, int64(100), result.LogSize) + assert.Equal(t, int64(200), result.PhysicalSize) + + // Test case: error in API call + client.On("Post", anyArgs[0:7]...).Return(errors.New("API call failed")).Once() + + result, err = CreateWritableSnapshot(ctx, client, sourceSnapshot, destinationPath) + assert.ErrorContains(t, err, "API call failed") + assert.Nil(t, result) +} + +func TestRemoveWritableSnapshot(t *testing.T) { + ctx := context.Background() + client := &mocks.Client{} + + client.On("Delete", anyArgs[0:6]...).Return(nil).Run(func(args mock.Arguments) { + tgt := args.Get(1).(string) + assert.Equal(t, "platform/14/snapshot/writable/ifs/data1", tgt) + }).Once() + + err := RemoveWritableSnapshot(ctx, client, "/ifs/data1") + assert.Nil(t, err) +}