From 9a206661298882b348209b2010f42c2b276492ed Mon Sep 17 00:00:00 2001 From: hardikl Date: Mon, 13 Apr 2026 17:50:21 +0530 Subject: [PATCH 1/4] feat: adding add/update/remove schedule in snapshot policy --- descriptions/descriptions.go | 5 + integration/test/snapshotPolicy_test.go | 201 +++++++++++++++++++ integration/test/snapshot_test.go | 101 ---------- ontap/ontap.go | 36 ++-- rest/snapshotpolicy.go | 229 ++++++++++++++++++---- server/Snapshotpolicy.go | 249 ++++++++++++++++++++++-- server/server.go | 4 + tool/tool.go | 12 ++ 8 files changed, 664 insertions(+), 173 deletions(-) create mode 100644 integration/test/snapshotPolicy_test.go delete mode 100644 integration/test/snapshot_test.go diff --git a/descriptions/descriptions.go b/descriptions/descriptions.go index a26cc49..7e76caf 100644 --- a/descriptions/descriptions.go +++ b/descriptions/descriptions.go @@ -37,9 +37,14 @@ const DeleteVolume = `Delete a volume on a cluster by cluster name.` const UpdateVolume = `Update volume name, size, state, nfs export policy of volume on a cluster by cluster name.` const CreateSnapshotPolicy = `Create a snapshot policy on a cluster by cluster name.` +const UpdateSnapshotPolicy = `Update a snapshot policy on a cluster by cluster name.` const DeleteSnapshotPolicy = `Delete a snapshot policy on a cluster by cluster name.` const CreateSchedule = `Create a cron schedule on a cluster by cluster name. Ex: 5 1 * * *, this cron expression indicates schedule would be triggered at 01:05 AM for every day` +const AddScheduleInSnapshotPolicy = `Add a schedule entry to an existing snapshot policy on a cluster by cluster name.` +const UpdateScheduleInSnapshotPolicy = `Update a schedule entry within an existing snapshot policy on a cluster by cluster name. At least one of count, prefix, or snapmirror_label must be provided.` +const RemoveScheduleInSnapshotPolicy = `Remove a schedule entry from an existing snapshot policy on a cluster by cluster name.` + const ListQoSPolicies = `List QoS policies from an ONTAP cluster — includes both SVM-scoped and cluster-scoped (admin SVM) policies. The response is split into two sections: diff --git a/integration/test/snapshotPolicy_test.go b/integration/test/snapshotPolicy_test.go new file mode 100644 index 0000000..663714b --- /dev/null +++ b/integration/test/snapshotPolicy_test.go @@ -0,0 +1,201 @@ +package main + +import ( + "context" + "crypto/tls" + "github.com/carlmjohnson/requests" + "github.com/netapp/ontap-mcp/ontap" + "log/slog" + "net/http" + "strings" + "testing" + "time" + + "github.com/netapp/ontap-mcp/config" +) + +func TestSnapshot(t *testing.T) { + SkipIfMissing(t, CheckTools) + + tests := []struct { + name string + input string + expectedOntapErr string + verifyAPI ontapVerifier + mustContain []string + }{ + { + name: "Clean SVM", + input: ClusterStr + "delete " + rn("marketing") + " svm", + expectedOntapErr: "because it does not exist", + verifyAPI: ontapVerifier{api: "api/svm/svms?name=" + rn("marketing"), validationFunc: deleteObject}, + }, + { + name: "Create SVM", + input: ClusterStr + "create " + rn("marketing") + " svm", + expectedOntapErr: "", + verifyAPI: ontapVerifier{api: "api/svm/svms?name=" + rn("marketing"), validationFunc: createObject}, + }, + { + name: "Clean snapshot policy every4hours", + input: ClusterStr + "delete " + rn("every4hours") + " snapshot policy in " + rn("marketing") + " svm", + expectedOntapErr: "because it does not exist", + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: deleteObject}, + }, + { + name: "Clean snapshot policy every5min", + input: ClusterStr + "Delete " + rn("every5min") + " snapshot policy in " + rn("marketing") + " svm", + expectedOntapErr: "because it does not exist", + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every5min"), validationFunc: deleteObject}, + }, + { + name: "Create snapshot policy every4hours", + input: ClusterStr + "create a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM. The schedule is 4hours and keeps the last 5 snapshots", + expectedOntapErr: "", + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: createObject}, + }, + { + name: "Create snapshot policy every5min", + input: ClusterStr + "create a snapshot policy named " + rn("every5min") + " on the " + rn("marketing") + " SVM. The schedule is 5minutes and keeps the last 2 snapshots", + expectedOntapErr: "", + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every5min"), validationFunc: createObject}, + }, + { + name: "Add schedule to snapshot policy every4hours", + input: ClusterStr + "add schedule 2hours in a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM and keeps the last 6 snapshots", + expectedOntapErr: "", + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: verifySchedule(true, "2hours", "", "", 6)}, + }, + { + name: "Update schedule in snapshot policy every4hours", + input: ClusterStr + "update schedule named 2hours in a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM with snapshot name prefix as `2h` and snapmirror label as `sm2`", + expectedOntapErr: "", + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: verifySchedule(true, "2hours", "2h", "sm2", 6)}, + }, + { + name: "Remove schedule from snapshot policy every4hours", + input: ClusterStr + "remove schedule named 2hours from a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM", + expectedOntapErr: "", + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: verifySchedule(false, "2hours", "", "", 0)}, + }, + { + name: "Update snapshot policy every4hours", + input: ClusterStr + "disable a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM with comment as `4 hour policy`", + expectedOntapErr: "", + verifyAPI: ontapVerifier{}, + }, + { + name: "Clean snapshot policy every4hours", + input: ClusterStr + "delete " + rn("every4hours") + " snapshot policy in " + rn("marketing") + " svm", + expectedOntapErr: "because it does not exist", + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: deleteObject}, + }, + { + name: "Clean snapshot policy every5min", + input: ClusterStr + "Delete " + rn("every5min") + " snapshot policy in " + rn("marketing") + " svm", + expectedOntapErr: "because it does not exist", + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every5min"), validationFunc: deleteObject}, + }, + { + name: "Clean SVM", + input: ClusterStr + "delete " + rn("marketing") + " svm", + expectedOntapErr: "because it does not exist", + verifyAPI: ontapVerifier{api: "api/svm/svms?name=" + rn("marketing"), validationFunc: deleteObject}, + }, + } + + cfg, err := config.ReadConfig(ConfigFile) + if err != nil { + t.Fatalf("Error parsing the config: %v", err) + } + + poller := cfg.Pollers[Cluster] + transport := &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: poller.UseInsecureTLS, // #nosec G402 + }, + } + client := &http.Client{Transport: transport, Timeout: 10 * time.Second} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + slog.Debug("", slog.String("Input", tt.input)) + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + response, err := testAgent.ChatWithResponse(ctx, t, tt.input, tt.expectedOntapErr) + if err != nil { + t.Fatalf("Error processing input %q: %v", tt.input, err) + } + if tt.verifyAPI.api != "" && !tt.verifyAPI.validationFunc(t, tt.verifyAPI.api, poller, client) { + t.Errorf("Error while accessing the object via prompt %q", tt.input) + } + lower := strings.ToLower(response) + for _, want := range tt.mustContain { + if !strings.Contains(lower, strings.ToLower(want)) { + t.Errorf("response missing expected text %q \nfull response: %s", want, response) + } + } + }) + } +} + +func verifySchedule(exist bool, scheduleName string, expectedPrefix string, expectedSMLabel string, expectedCount int) func(t *testing.T, api string, poller *config.Poller, client *http.Client) bool { + return func(t *testing.T, api string, poller *config.Poller, client *http.Client) bool { + type SnapshotPolicySchedule struct { + Count int `json:"count,omitzero" jsonschema:"number of snapshots to keep for this schedule"` + Schedule ontap.NameAndUUID `json:"schedule,omitzero" jsonschema:"name of the schedule"` + Prefix string `json:"prefix,omitzero" jsonschema:"snapshot name prefix for this schedule"` + SnapmirrorLabel string `json:"snapmirror_label,omitzero" jsonschema:"SnapMirror label for this schedule"` + } + type Copy struct { + Count int `json:"count"` + Schedule SnapshotPolicySchedule `json:"schedule"` + } + type SnapshotPolicy struct { + Copies []Copy `json:"copies,omitzero" jsonschema:"snapshot copies"` + } + type response struct { + NumRecords int `json:"num_records"` + Records []SnapshotPolicy `json:"records"` + } + + var data response + err := requests.URL("https://"+poller.Addr+"/"+api). + BasicAuth(poller.Username, poller.Password). + Client(client). + ToJSON(&data). + Fetch(context.Background()) + if err != nil { + t.Errorf("verifySchedule: request failed: %v", err) + return false + } + if data.NumRecords != 1 { + t.Errorf("verifySchedule: expected 1 record, got %d", data.NumRecords) + return false + } + + for _, ssCopy := range data.Records[0].Copies { + if ssCopy.Schedule.Schedule.Name == scheduleName { + if !exist { + t.Errorf("verifySchedule: schedule should not be exist") + return false + } + + if expectedPrefix != ssCopy.Schedule.Prefix { + t.Errorf("verifySchedule: got = %s, want %s", ssCopy.Schedule.Prefix, expectedPrefix) + return false + } + if expectedSMLabel != ssCopy.Schedule.SnapmirrorLabel { + t.Errorf("verifySchedule: got = %s, want %s", ssCopy.Schedule.SnapmirrorLabel, expectedSMLabel) + return false + } + if expectedCount != ssCopy.Schedule.Count { + t.Errorf("verifySchedule: got = %d, want %d", ssCopy.Schedule.Count, expectedCount) + return false + } + } + } + + return true + } +} diff --git a/integration/test/snapshot_test.go b/integration/test/snapshot_test.go deleted file mode 100644 index b1ca541..0000000 --- a/integration/test/snapshot_test.go +++ /dev/null @@ -1,101 +0,0 @@ -package main - -import ( - "context" - "crypto/tls" - "log/slog" - "net/http" - "strings" - "testing" - "time" - - "github.com/netapp/ontap-mcp/config" -) - -func TestSnapshot(t *testing.T) { - SkipIfMissing(t, CheckTools) - - tests := []struct { - name string - input string - expectedOntapErr string - verifyAPI ontapVerifier - mustContain []string - }{ - { - name: "Clean snapshot policy every4hours", - input: ClusterStr + "delete " + rn("every4hours") + " snapshot policy in marketing svm", - expectedOntapErr: "because it does not exist", - verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: deleteObject}, - }, - { - name: "Clean snapshot policy every5min", - input: ClusterStr + "Delete " + rn("every5min") + " snapshot policy in marketing svm", - expectedOntapErr: "because it does not exist", - verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every5min"), validationFunc: deleteObject}, - }, - { - name: "Create snapshot policy every4hours", - input: ClusterStr + "create a snapshot policy named " + rn("every4hours") + " on the marketing SVM. The schedule is 4hours and keeps the last 5 snapshots", - expectedOntapErr: "", - verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: createObject}, - }, - { - name: "Create snapshot policy every5min", - input: ClusterStr + "create a snapshot policy named " + rn("every5min") + " on the marketing SVM. The schedule is 5minutes and keeps the last 2 snapshots", - expectedOntapErr: "", - verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every5min"), validationFunc: createObject}, - }, - { - name: "Clean snapshot policy every4hours", - input: ClusterStr + "delete " + rn("every4hours") + " snapshot policy in marketing svm", - expectedOntapErr: "because it does not exist", - verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: deleteObject}, - }, - { - name: "Clean snapshot policy every5min", - input: ClusterStr + "Delete " + rn("every5min") + " snapshot policy in marketing svm", - expectedOntapErr: "because it does not exist", - verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every5min"), validationFunc: deleteObject}, - }, - { - name: "List snapshots on a volume", - input: ClusterStr + "list snapshots on volume harvest_root on svm harvest", - mustContain: []string{"snapshot"}, - }, - } - - cfg, err := config.ReadConfig(ConfigFile) - if err != nil { - t.Fatalf("Error parsing the config: %v", err) - } - - poller := cfg.Pollers[Cluster] - transport := &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: poller.UseInsecureTLS, // #nosec G402 - }, - } - client := &http.Client{Transport: transport, Timeout: 10 * time.Second} - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - slog.Debug("", slog.String("Input", tt.input)) - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) - defer cancel() - response, err := testAgent.ChatWithResponse(ctx, t, tt.input, tt.expectedOntapErr) - if err != nil { - t.Fatalf("Error processing input %q: %v", tt.input, err) - } - if tt.verifyAPI.api != "" && !tt.verifyAPI.validationFunc(t, tt.verifyAPI.api, poller, client) { - t.Errorf("Error while accessing the object via prompt %q", tt.input) - } - lower := strings.ToLower(response) - for _, want := range tt.mustContain { - if !strings.Contains(lower, strings.ToLower(want)) { - t.Errorf("response missing expected text %q \nfull response: %s", want, response) - } - } - }) - } -} diff --git a/ontap/ontap.go b/ontap/ontap.go index 41d7dc8..1222ab8 100644 --- a/ontap/ontap.go +++ b/ontap/ontap.go @@ -46,16 +46,17 @@ type PostJob struct { type GetData struct { Records []struct { - ID int `json:"id,omitzero"` - UUID string `json:"uuid,omitzero"` - Index int `json:"index,omitzero"` - Name string `json:"name,omitzero"` - Svm NameAndUUID `json:"svm,omitzero"` - Volume NameAndUUID `json:"volume,omitzero"` - RoRule []string `json:"ro_rule,omitzero"` - RwRule []string `json:"rw_rule,omitzero"` - Clients []ClientData `json:"clients,omitzero"` - Nas NAS `json:"nas,omitzero"` + ID int `json:"id,omitzero"` + UUID string `json:"uuid,omitzero"` + Index int `json:"index,omitzero"` + Name string `json:"name,omitzero"` + Svm NameAndUUID `json:"svm,omitzero"` + Volume NameAndUUID `json:"volume,omitzero"` + RoRule []string `json:"ro_rule,omitzero"` + RwRule []string `json:"rw_rule,omitzero"` + Clients []ClientData `json:"clients,omitzero"` + Nas NAS `json:"nas,omitzero"` + Schedule NameAndUUID `json:"schedule,omitzero"` } `json:"records"` NumRecords int `json:"num_records"` } @@ -131,9 +132,11 @@ type NameAndSVM struct { } type SnapshotPolicy struct { - SVM NameAndUUID `json:"svm,omitzero" jsonschema:"svm name"` - Name string `json:"name,omitzero" jsonschema:"snapshot policy name"` - Copies []Copy `json:"copies,omitzero" jsonschema:"snapshot copies"` + SVM NameAndUUID `json:"svm,omitzero" jsonschema:"svm name"` + Name string `json:"name,omitzero" jsonschema:"snapshot policy name"` + Copies []Copy `json:"copies,omitzero" jsonschema:"snapshot copies"` + Enabled string `json:"enabled,omitzero" jsonschema:"the state of snapshot policy"` + Comment string `json:"comment,omitzero" jsonschema:"comment associated with the snapshot policy"` } type Copy struct { @@ -146,6 +149,13 @@ type Schedule struct { Cron Cron `json:"cron,omitzero"` } +type SnapshotPolicySchedule struct { + Count int `json:"count,omitzero" jsonschema:"number of snapshots to keep for this schedule"` + Schedule NameAndUUID `json:"schedule,omitzero" jsonschema:"name of the schedule"` + Prefix string `json:"prefix,omitzero" jsonschema:"snapshot name prefix for this schedule"` + SnapmirrorLabel string `json:"snapmirror_label,omitzero" jsonschema:"SnapMirror label for this schedule"` +} + type Cron struct { Days []int `json:"days,omitzero"` Hours []int `json:"hours,omitzero"` diff --git a/rest/snapshotpolicy.go b/rest/snapshotpolicy.go index 820c2f1..de5b203 100644 --- a/rest/snapshotpolicy.go +++ b/rest/snapshotpolicy.go @@ -1,7 +1,6 @@ package rest import ( - "bytes" "context" "fmt" "github.com/netapp/ontap-mcp/ontap" @@ -9,16 +8,60 @@ import ( "net/url" ) -func (c *Client) DeleteSnapshotPolicy(ctx context.Context, snapshotPolicy ontap.SnapshotPolicy) error { +func (c *Client) CreateSnapshotPolicy(ctx context.Context, snapshotPolicy ontap.SnapshotPolicy) error { + var ( + statusCode int + oc ontap.OnlyCount + err error + ) + responseHeaders := http.Header{} + + // If schedule is exist then use it else create new + scheduleName := snapshotPolicy.Copies[0].Schedule.Name + if scheduleName == "" { + return fmt.Errorf("no schedule exist with %s name", scheduleName) + } + params := url.Values{} + params.Set("return_records", "false") + params.Set("fields", "name") + params.Set("name", scheduleName) + + builder := c.baseRequestBuilder(`/api/cluster/schedules`, &statusCode, responseHeaders). + ToJSON(&oc). + Params(params) + + err = c.buildAndExecuteRequest(ctx, builder) + + if err != nil { + return err + } + + if oc.NumRecords == 0 { + return fmt.Errorf("no schedule %s found", scheduleName) + } else if oc.NumRecords != 1 { + return fmt.Errorf("failed to create snapshotPolicy=%s on svm=%s with given schedule name=%s because there are %d matching schedules", + snapshotPolicy.Name, snapshotPolicy.SVM.Name, scheduleName, oc.NumRecords) + } + + builder2 := c.baseRequestBuilder(`/api/storage/snapshot-policies`, &statusCode, responseHeaders). + BodyJSON(snapshotPolicy) + + if err := c.buildAndExecuteRequest(ctx, builder2); err != nil { + return err + } + + return c.checkStatus(statusCode) +} + +func (c *Client) UpdateSnapshotPolicy(ctx context.Context, snapshotPolicy ontap.SnapshotPolicy, snapshotPolicyName string, svmName string) error { var ( - buf bytes.Buffer statusCode int ssPolicy ontap.GetData ) responseHeaders := http.Header{} params := url.Values{} params.Set("fields", "uuid") - params.Set("name", snapshotPolicy.Name) + params.Set("name", snapshotPolicyName) builder := c.baseRequestBuilder(`/api/storage/snapshot-policies`, nil, responseHeaders). Params(params). @@ -31,66 +74,53 @@ func (c *Client) DeleteSnapshotPolicy(ctx context.Context, snapshotPolicy ontap. } if ssPolicy.NumRecords == 0 { - return fmt.Errorf("failed to delete snapshotPolicy=%s on svm=%s because it does not exist", snapshotPolicy.Name, snapshotPolicy.SVM.Name) + return fmt.Errorf("failed to get snapshotPolicy=%s on svm=%s because it does not exist", snapshotPolicyName, svmName) } if ssPolicy.NumRecords != 1 { - return fmt.Errorf("failed to delete snapshotPolicy=%s on svm=%s because there are %d matching records", - snapshotPolicy.Name, snapshotPolicy.SVM.Name, ssPolicy.NumRecords) + return fmt.Errorf("failed to get snapshotPolicy=%s on svm=%s because there are %d matching records", + snapshotPolicyName, svmName, ssPolicy.NumRecords) } - builder2 := c.baseRequestBuilder(`/api/storage/snapshot-policies/`+ssPolicy.Records[0].UUID, &statusCode, responseHeaders). - Delete(). - ToBytesBuffer(&buf) - - err = c.buildAndExecuteRequest(ctx, builder2) + builder2 := c.baseRequestBuilder(`/api/storage/snapshot-policies`, &statusCode, responseHeaders). + BodyJSON(snapshotPolicy) - if err != nil { + if err := c.buildAndExecuteRequest(ctx, builder2); err != nil { return err } - return c.handleJob(ctx, statusCode, buf) + return c.checkStatus(statusCode) } -func (c *Client) CreateSnapshotPolicy(ctx context.Context, snapshotPolicy ontap.SnapshotPolicy) error { +func (c *Client) DeleteSnapshotPolicy(ctx context.Context, snapshotPolicy ontap.SnapshotPolicy) error { var ( - buf bytes.Buffer statusCode int - oc ontap.OnlyCount - err error + ssPolicy ontap.GetData ) responseHeaders := http.Header{} - - // If schedule is exist then use it else create new - scheduleName := snapshotPolicy.Copies[0].Schedule.Name - if scheduleName == "" { - return fmt.Errorf("no schedule exist with %s name", scheduleName) - } params := url.Values{} - params.Set("return_records", "false") - params.Set("fields", "name") - params.Set("name", scheduleName) + params.Set("fields", "uuid") + params.Set("name", snapshotPolicy.Name) - builder := c.baseRequestBuilder(`/api/cluster/schedules`, &statusCode, responseHeaders). - ToBytesBuffer(&buf). - ToJSON(&oc). - Params(params) + builder := c.baseRequestBuilder(`/api/storage/snapshot-policies`, nil, responseHeaders). + Params(params). + ToJSON(&ssPolicy) - err = c.buildAndExecuteRequest(ctx, builder) + err := c.buildAndExecuteRequest(ctx, builder) if err != nil { return err } - if oc.NumRecords == 0 { - return fmt.Errorf("no schedule %s found", scheduleName) - } else if oc.NumRecords != 1 { - return fmt.Errorf("failed to create snapshotPolicy=%s on svm=%s with given schedule name=%s because there are %d matching schedules", - snapshotPolicy.Name, snapshotPolicy.SVM.Name, scheduleName, oc.NumRecords) + if ssPolicy.NumRecords == 0 { + return fmt.Errorf("failed to get snapshotPolicy=%s on svm=%s because it does not exist", snapshotPolicy.Name, snapshotPolicy.SVM.Name) + } + if ssPolicy.NumRecords != 1 { + return fmt.Errorf("failed to get snapshotPolicy=%s on svm=%s because there are %d matching records", + snapshotPolicy.Name, snapshotPolicy.SVM.Name, ssPolicy.NumRecords) } - builder2 := c.baseRequestBuilder(`/api/storage/snapshot-policies`, &statusCode, responseHeaders). - BodyJSON(snapshotPolicy). - ToBytesBuffer(&buf) + builder2 := c.baseRequestBuilder(`/api/storage/snapshot-policies/`+ssPolicy.Records[0].UUID, &statusCode, responseHeaders). + Delete() if err := c.buildAndExecuteRequest(ctx, builder2); err != nil { return err @@ -111,3 +141,122 @@ func (c *Client) CreateSchedule(ctx context.Context, schedule ontap.Schedule) er return c.checkStatus(statusCode) } + +func (c *Client) getSnapshotPolicyUUID(ctx context.Context, policyName, svmName string) (string, error) { + var ssPolicy ontap.GetData + responseHeaders := http.Header{} + params := url.Values{} + params.Set("fields", "uuid") + params.Set("name", policyName) + params.Set("svm.name", svmName) + + builder := c.baseRequestBuilder(`/api/storage/snapshot-policies`, nil, responseHeaders). + Params(params). + ToJSON(&ssPolicy) + + if err := c.buildAndExecuteRequest(ctx, builder); err != nil { + return "", err + } + + if ssPolicy.NumRecords == 0 { + return "", fmt.Errorf("failed to get snapshot policy=%s on svm=%s because it does not exist", policyName, svmName) + } + if ssPolicy.NumRecords != 1 { + return "", fmt.Errorf("failed to get snapshot policy=%s on svm=%s because there are %d matching records", policyName, svmName, ssPolicy.NumRecords) + } + + return ssPolicy.Records[0].UUID, nil +} + +func (c *Client) getSnapshotPolicyScheduleUUID(ctx context.Context, policyUUID, scheduleName string) (string, error) { + var scheduleRecords ontap.GetData + responseHeaders := http.Header{} + params := url.Values{} + params.Set("fields", "schedule.uuid,schedule.name") + params.Set("schedule.name", scheduleName) + + builder := c.baseRequestBuilder(`/api/storage/snapshot-policies/`+policyUUID+`/schedules`, nil, responseHeaders). + Params(params). + ToJSON(&scheduleRecords) + + if err := c.buildAndExecuteRequest(ctx, builder); err != nil { + return "", err + } + + if scheduleRecords.NumRecords == 0 { + return "", fmt.Errorf("schedule=%s does not exist in snapshot policy uuid=%s", scheduleName, policyUUID) + } + if scheduleRecords.NumRecords != 1 { + return "", fmt.Errorf("found %d schedules matching name=%s in snapshot policy uuid=%s", scheduleRecords.NumRecords, scheduleName, policyUUID) + } + + return scheduleRecords.Records[0].Schedule.UUID, nil +} + +func (c *Client) AddScheduleInSnapshotPolicy(ctx context.Context, policyName, svmName string, schedule ontap.SnapshotPolicySchedule) error { + var statusCode int + responseHeaders := http.Header{} + + policyUUID, err := c.getSnapshotPolicyUUID(ctx, policyName, svmName) + if err != nil { + return err + } + + builder := c.baseRequestBuilder(`/api/storage/snapshot-policies/`+policyUUID+`/schedules`, &statusCode, responseHeaders). + BodyJSON(schedule) + + if err := c.buildAndExecuteRequest(ctx, builder); err != nil { + return err + } + + return c.checkStatus(statusCode) +} + +func (c *Client) UpdateScheduleInSnapshotPolicy(ctx context.Context, policyName, svmName, scheduleName string, schedule ontap.SnapshotPolicySchedule) error { + var statusCode int + responseHeaders := http.Header{} + + policyUUID, err := c.getSnapshotPolicyUUID(ctx, policyName, svmName) + if err != nil { + return err + } + + scheduleUUID, err := c.getSnapshotPolicyScheduleUUID(ctx, policyUUID, scheduleName) + if err != nil { + return err + } + + builder := c.baseRequestBuilder(`/api/storage/snapshot-policies/`+policyUUID+`/schedules/`+scheduleUUID, &statusCode, responseHeaders). + BodyJSON(schedule). + Patch() + + if err := c.buildAndExecuteRequest(ctx, builder); err != nil { + return err + } + + return c.checkStatus(statusCode) +} + +func (c *Client) RemoveScheduleInSnapshotPolicy(ctx context.Context, policyName, svmName, scheduleName string) error { + var statusCode int + responseHeaders := http.Header{} + + policyUUID, err := c.getSnapshotPolicyUUID(ctx, policyName, svmName) + if err != nil { + return err + } + + scheduleUUID, err := c.getSnapshotPolicyScheduleUUID(ctx, policyUUID, scheduleName) + if err != nil { + return err + } + + builder := c.baseRequestBuilder(`/api/storage/snapshot-policies/`+policyUUID+`/schedules/`+scheduleUUID, &statusCode, responseHeaders). + Delete() + + if err := c.buildAndExecuteRequest(ctx, builder); err != nil { + return err + } + + return c.checkStatus(statusCode) +} diff --git a/server/Snapshotpolicy.go b/server/Snapshotpolicy.go index b932d3f..f975049 100644 --- a/server/Snapshotpolicy.go +++ b/server/Snapshotpolicy.go @@ -10,13 +10,13 @@ import ( "strings" ) -func (a *App) DeleteSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, parameters tool.SnapshotPolicy) (*mcp.CallToolResult, any, error) { +func (a *App) CreateSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, parameters tool.SnapshotPolicy) (*mcp.CallToolResult, any, error) { if !a.locks.TryLock(parameters.Cluster) { return errorResult(fmt.Errorf("another write operation is in progress on cluster %s, please try again", parameters.Cluster)), nil, nil } defer a.locks.Unlock(parameters.Cluster) - snapshotPolicyDelete, err := newDeleteSnapshotPolicy(parameters) + snapshotPolicyCreate, err := newCreateSnapshotPolicy(parameters) if err != nil { return nil, nil, err } @@ -25,13 +25,13 @@ func (a *App) DeleteSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, if err != nil { return errorResult(err), nil, err } - err = client.DeleteSnapshotPolicy(ctx, snapshotPolicyDelete) + err = client.CreateSnapshotPolicy(ctx, snapshotPolicyCreate) if err != nil { return errorResult(err), nil, err } - responseText := "Snapshot policy deleted successfully" + responseText := "Snapshot policy created successfully" return &mcp.CallToolResult{ Content: []mcp.Content{ @@ -40,13 +40,13 @@ func (a *App) DeleteSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, }, nil, nil } -func (a *App) CreateSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, parameters tool.SnapshotPolicy) (*mcp.CallToolResult, any, error) { +func (a *App) UpdateSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, parameters tool.SnapshotPolicy) (*mcp.CallToolResult, any, error) { if !a.locks.TryLock(parameters.Cluster) { return errorResult(fmt.Errorf("another write operation is in progress on cluster %s, please try again", parameters.Cluster)), nil, nil } defer a.locks.Unlock(parameters.Cluster) - snapshotPolicyCreate, err := newCreateSnapshotPolicy(parameters) + snapshotPolicyUpdate, err := newUpdateSnapshotPolicy(parameters) if err != nil { return nil, nil, err } @@ -55,13 +55,13 @@ func (a *App) CreateSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, if err != nil { return errorResult(err), nil, err } - err = client.CreateSnapshotPolicy(ctx, snapshotPolicyCreate) + err = client.UpdateSnapshotPolicy(ctx, snapshotPolicyUpdate, parameters.Name, parameters.SVM) if err != nil { return errorResult(err), nil, err } - responseText := "Snapshot policy created successfully" + responseText := "Snapshot policy updated successfully" return &mcp.CallToolResult{ Content: []mcp.Content{ @@ -70,21 +70,34 @@ func (a *App) CreateSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, }, nil, nil } -// newDeleteSnapshotPolicy validates the customer provided arguments and converts them into -// the corresponding ONTAP object ready to use via the REST API -func newDeleteSnapshotPolicy(in tool.SnapshotPolicy) (ontap.SnapshotPolicy, error) { - out := ontap.SnapshotPolicy{} - if in.SVM == "" { - return out, errors.New("SVM name is required") +func (a *App) DeleteSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, parameters tool.SnapshotPolicy) (*mcp.CallToolResult, any, error) { + if !a.locks.TryLock(parameters.Cluster) { + return errorResult(fmt.Errorf("another write operation is in progress on cluster %s, please try again", parameters.Cluster)), nil, nil } - if in.Name == "" { - return out, errors.New("snapshot policy name is required") + defer a.locks.Unlock(parameters.Cluster) + + snapshotPolicyDelete, err := newDeleteSnapshotPolicy(parameters) + if err != nil { + return nil, nil, err } - out.SVM = ontap.NameAndUUID{Name: in.SVM} - out.Name = in.Name + client, err := a.getClient(parameters.Cluster) + if err != nil { + return errorResult(err), nil, err + } + err = client.DeleteSnapshotPolicy(ctx, snapshotPolicyDelete) - return out, nil + if err != nil { + return errorResult(err), nil, err + } + + responseText := "Snapshot policy deleted successfully" + + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: responseText}, + }, + }, nil, nil } // newCreateSnapshotPolicy validates the customer provided arguments and converts them into @@ -116,6 +129,48 @@ func newCreateSnapshotPolicy(in tool.SnapshotPolicy) (ontap.SnapshotPolicy, erro return out, nil } +// newUpdateSnapshotPolicy validates the customer provided arguments and converts them into +// the corresponding ONTAP object ready to use via the REST API +func newUpdateSnapshotPolicy(in tool.SnapshotPolicy) (ontap.SnapshotPolicy, error) { + out := ontap.SnapshotPolicy{} + if in.SVM == "" { + return out, errors.New("SVM name is required") + } + if in.Name == "" { + return out, errors.New("snapshot policy name is required") + } + + if in.Comment != "" { + out.Comment = in.Comment + } + if in.Enabled != "" { + out.Enabled = in.Enabled + } + + if out.Enabled == "" && out.Comment == "" { + return out, errors.New("at least one supported update field must be provided; enabled and comment are supported for update") + } + + return out, nil +} + +// newDeleteSnapshotPolicy validates the customer provided arguments and converts them into +// the corresponding ONTAP object ready to use via the REST API +func newDeleteSnapshotPolicy(in tool.SnapshotPolicy) (ontap.SnapshotPolicy, error) { + out := ontap.SnapshotPolicy{} + if in.SVM == "" { + return out, errors.New("SVM name is required") + } + if in.Name == "" { + return out, errors.New("snapshot policy name is required") + } + + out.SVM = ontap.NameAndUUID{Name: in.SVM} + out.Name = in.Name + + return out, nil +} + func (a *App) CreateSchedule(ctx context.Context, _ *mcp.CallToolRequest, parameters tool.Schedule) (*mcp.CallToolResult, any, error) { if !a.locks.TryLock(parameters.Cluster) { return errorResult(fmt.Errorf("another write operation is in progress on cluster %s, please try again", parameters.Cluster)), nil, nil @@ -174,6 +229,7 @@ func newCreateSchedule(in tool.Schedule) (ontap.Schedule, error) { return out, nil } + func readRanges(minRange int, maxRange int, r string, out *[]int) { if r != "*" { for rng := range strings.SplitSeq(r, ",") { @@ -233,3 +289,158 @@ func convertCron(cronStr string, out *ontap.Schedule) error { } return nil } + +func (a *App) AddScheduleInSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, parameters tool.SnapshotPolicySchedule) (*mcp.CallToolResult, any, error) { + if !a.locks.TryLock(parameters.Cluster) { + return errorResult(fmt.Errorf("another write operation is in progress on cluster %s, please try again", parameters.Cluster)), nil, nil + } + defer a.locks.Unlock(parameters.Cluster) + + scheduleEntry, err := newAddScheduleInSnapshotPolicy(parameters) + if err != nil { + return nil, nil, err + } + + client, err := a.getClient(parameters.Cluster) + if err != nil { + return errorResult(err), nil, err + } + + err = client.AddScheduleInSnapshotPolicy(ctx, parameters.PolicyName, parameters.SVM, scheduleEntry) + if err != nil { + return errorResult(err), nil, err + } + + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: "Schedule added to snapshot policy successfully"}, + }, + }, nil, nil +} + +func (a *App) UpdateScheduleInSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, parameters tool.SnapshotPolicySchedule) (*mcp.CallToolResult, any, error) { + if !a.locks.TryLock(parameters.Cluster) { + return errorResult(fmt.Errorf("another write operation is in progress on cluster %s, please try again", parameters.Cluster)), nil, nil + } + defer a.locks.Unlock(parameters.Cluster) + + scheduleEntry, err := newUpdateScheduleInSnapshotPolicy(parameters) + if err != nil { + return nil, nil, err + } + + client, err := a.getClient(parameters.Cluster) + if err != nil { + return errorResult(err), nil, err + } + + err = client.UpdateScheduleInSnapshotPolicy(ctx, parameters.PolicyName, parameters.SVM, parameters.ScheduleName, scheduleEntry) + if err != nil { + return errorResult(err), nil, err + } + + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: "Schedule in snapshot policy updated successfully"}, + }, + }, nil, nil +} + +func (a *App) RemoveScheduleInSnapshotPolicy(ctx context.Context, _ *mcp.CallToolRequest, parameters tool.SnapshotPolicySchedule) (*mcp.CallToolResult, any, error) { + if !a.locks.TryLock(parameters.Cluster) { + return errorResult(fmt.Errorf("another write operation is in progress on cluster %s, please try again", parameters.Cluster)), nil, nil + } + defer a.locks.Unlock(parameters.Cluster) + + if err := validateDeleteScheduleInSnapshotPolicy(parameters); err != nil { + return nil, nil, err + } + + client, err := a.getClient(parameters.Cluster) + if err != nil { + return errorResult(err), nil, err + } + + err = client.RemoveScheduleInSnapshotPolicy(ctx, parameters.PolicyName, parameters.SVM, parameters.ScheduleName) + if err != nil { + return errorResult(err), nil, err + } + + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: "Schedule removed from snapshot policy successfully"}, + }, + }, nil, nil +} + +// newAddScheduleInSnapshotPolicy validates and converts input for adding a schedule to a snapshot policy +func newAddScheduleInSnapshotPolicy(in tool.SnapshotPolicySchedule) (ontap.SnapshotPolicySchedule, error) { + out := ontap.SnapshotPolicySchedule{} + if in.SVM == "" { + return out, errors.New("SVM name is required") + } + if in.PolicyName == "" { + return out, errors.New("snapshot policy name is required") + } + if in.ScheduleName == "" { + return out, errors.New("schedule name is required") + } + if in.Count == 0 { + return out, errors.New("snapshot copies count is required") + } + + out.Schedule = ontap.NameAndUUID{Name: in.ScheduleName} + out.Count = in.Count + + if in.Prefix != "" { + out.Prefix = in.Prefix + } + if in.SnapmirrorLabel != "" { + out.SnapmirrorLabel = in.SnapmirrorLabel + } + + return out, nil +} + +// newUpdateScheduleInSnapshotPolicy validates and converts input for updating a schedule in a snapshot policy +func newUpdateScheduleInSnapshotPolicy(in tool.SnapshotPolicySchedule) (ontap.SnapshotPolicySchedule, error) { + out := ontap.SnapshotPolicySchedule{} + if in.SVM == "" { + return out, errors.New("SVM name is required") + } + if in.PolicyName == "" { + return out, errors.New("snapshot policy name is required") + } + if in.ScheduleName == "" { + return out, errors.New("schedule name is required") + } + if in.Count == 0 && in.Prefix == "" && in.SnapmirrorLabel == "" { + return out, errors.New("at least one supported update field must be provided; count, prefix, and snapmirror_label are supported for update") + } + + if in.Prefix != "" { + out.Prefix = in.Prefix + } + if in.SnapmirrorLabel != "" { + out.SnapmirrorLabel = in.SnapmirrorLabel + } + if in.Count > 0 { + out.Count = in.Count + } + + return out, nil +} + +// validateDeleteScheduleInSnapshotPolicy validates input for removing a schedule from a snapshot policy +func validateDeleteScheduleInSnapshotPolicy(in tool.SnapshotPolicySchedule) error { + if in.SVM == "" { + return errors.New("SVM name is required") + } + if in.PolicyName == "" { + return errors.New("snapshot policy name is required") + } + if in.ScheduleName == "" { + return errors.New("schedule name is required") + } + return nil +} diff --git a/server/server.go b/server/server.go index 1e5b4b0..52d2648 100644 --- a/server/server.go +++ b/server/server.go @@ -99,8 +99,12 @@ func (a *App) createMCPServer() *mcp.Server { // operation on Snapshot Policy object addTool(a, server, "create_snapshot_policy", descriptions.CreateSnapshotPolicy, createAnnotation, a.CreateSnapshotPolicy) + addTool(a, server, "update_snapshot_policy", descriptions.UpdateSnapshotPolicy, updateAnnotation, a.UpdateSnapshotPolicy) addTool(a, server, "delete_snapshot_policy", descriptions.DeleteSnapshotPolicy, deleteAnnotation, a.DeleteSnapshotPolicy) addTool(a, server, "create_schedule", descriptions.CreateSchedule, createAnnotation, a.CreateSchedule) + addTool(a, server, "add_schedule_in_snapshot_policy", descriptions.AddScheduleInSnapshotPolicy, createAnnotation, a.AddScheduleInSnapshotPolicy) + addTool(a, server, "update_schedule_in_snapshot_policy", descriptions.UpdateScheduleInSnapshotPolicy, updateAnnotation, a.UpdateScheduleInSnapshotPolicy) + addTool(a, server, "remove_schedule_in_snapshot_policy", descriptions.RemoveScheduleInSnapshotPolicy, deleteAnnotation, a.RemoveScheduleInSnapshotPolicy) // operation on QoS Policy object addTool(a, server, "list_qos_policies", descriptions.ListQoSPolicies, readOnlyAnnotation, a.ListQoSPolicies) diff --git a/tool/tool.go b/tool/tool.go index a472fc5..3ca9afb 100644 --- a/tool/tool.go +++ b/tool/tool.go @@ -44,6 +44,8 @@ type SnapshotPolicy struct { Name string `json:"name,omitzero" jsonschema:"snapshot policy name"` Schedule string `json:"schedule,omitzero" jsonschema:"schedule of snapshot policy"` Count int `json:"count,omitzero" jsonschema:"number of snapshots"` + Enabled string `json:"enabled,omitzero" jsonschema:"the state of snapshot policy"` + Comment string `json:"comment,omitzero" jsonschema:"comment associated with the snapshot policy"` } type Schedule struct { @@ -52,6 +54,16 @@ type Schedule struct { CronExpression string `json:"cron_expression" jsonschema:"cron_expression"` } +type SnapshotPolicySchedule struct { + Cluster string `json:"cluster_name" jsonschema:"cluster name"` + SVM string `json:"svm_name" jsonschema:"SVM name"` + PolicyName string `json:"policy_name" jsonschema:"snapshot policy name"` + ScheduleName string `json:"schedule_name" jsonschema:"name of the schedule"` + Count int `json:"count,omitzero" jsonschema:"number of snapshots to keep for this schedule"` + Prefix string `json:"prefix,omitzero" jsonschema:"snapshot name prefix for this schedule"` + SnapmirrorLabel string `json:"snapmirror_label,omitzero" jsonschema:"SnapMirror label for this schedule"` +} + type Cron struct { Days string `json:"days,omitzero"` Hours string `json:"hours,omitzero"` From 5ff112bd2c0a4830a0e5bf0603977fb2fead3d6b Mon Sep 17 00:00:00 2001 From: hardikl Date: Tue, 14 Apr 2026 21:09:15 +0530 Subject: [PATCH 2/4] feat: update tests --- integration/test/snapshotPolicy_test.go | 55 +++++++++++----------- ontap/ontap.go | 1 - rest/snapshotpolicy.go | 61 ++++++------------------- server/Snapshotpolicy.go | 10 +--- tool/tool.go | 1 - 5 files changed, 43 insertions(+), 85 deletions(-) diff --git a/integration/test/snapshotPolicy_test.go b/integration/test/snapshotPolicy_test.go index 663714b..9a4fbc8 100644 --- a/integration/test/snapshotPolicy_test.go +++ b/integration/test/snapshotPolicy_test.go @@ -64,19 +64,19 @@ func TestSnapshot(t *testing.T) { name: "Add schedule to snapshot policy every4hours", input: ClusterStr + "add schedule 2hours in a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM and keeps the last 6 snapshots", expectedOntapErr: "", - verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: verifySchedule(true, "2hours", "", "", 6)}, + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours") + "&svm.name=" + rn("marketing") + "&fields=copies", validationFunc: verifySchedule(true, "2hours", "-", 6)}, }, { name: "Update schedule in snapshot policy every4hours", - input: ClusterStr + "update schedule named 2hours in a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM with snapshot name prefix as `2h` and snapmirror label as `sm2`", + input: ClusterStr + "update schedule named 2hours in a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM with snapmirror label as `sm2`", expectedOntapErr: "", - verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: verifySchedule(true, "2hours", "2h", "sm2", 6)}, + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours") + "&svm.name=" + rn("marketing") + "&fields=copies", validationFunc: verifySchedule(true, "2hours", "sm2", 6)}, }, { name: "Remove schedule from snapshot policy every4hours", input: ClusterStr + "remove schedule named 2hours from a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM", expectedOntapErr: "", - verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: verifySchedule(false, "2hours", "", "", 0)}, + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours") + "&svm.name=" + rn("marketing") + "&fields=copies", validationFunc: verifySchedule(false, "2hours", "", 0)}, }, { name: "Update snapshot policy every4hours", @@ -139,18 +139,13 @@ func TestSnapshot(t *testing.T) { } } -func verifySchedule(exist bool, scheduleName string, expectedPrefix string, expectedSMLabel string, expectedCount int) func(t *testing.T, api string, poller *config.Poller, client *http.Client) bool { +func verifySchedule(exist bool, scheduleName string, expectedSMLabel string, expectedCount int) func(t *testing.T, api string, poller *config.Poller, client *http.Client) bool { return func(t *testing.T, api string, poller *config.Poller, client *http.Client) bool { - type SnapshotPolicySchedule struct { + type Copy struct { Count int `json:"count,omitzero" jsonschema:"number of snapshots to keep for this schedule"` Schedule ontap.NameAndUUID `json:"schedule,omitzero" jsonschema:"name of the schedule"` - Prefix string `json:"prefix,omitzero" jsonschema:"snapshot name prefix for this schedule"` SnapmirrorLabel string `json:"snapmirror_label,omitzero" jsonschema:"SnapMirror label for this schedule"` } - type Copy struct { - Count int `json:"count"` - Schedule SnapshotPolicySchedule `json:"schedule"` - } type SnapshotPolicy struct { Copies []Copy `json:"copies,omitzero" jsonschema:"snapshot copies"` } @@ -160,6 +155,7 @@ func verifySchedule(exist bool, scheduleName string, expectedPrefix string, expe } var data response + var scheduleFound bool err := requests.URL("https://"+poller.Addr+"/"+api). BasicAuth(poller.Username, poller.Password). Client(client). @@ -175,27 +171,30 @@ func verifySchedule(exist bool, scheduleName string, expectedPrefix string, expe } for _, ssCopy := range data.Records[0].Copies { - if ssCopy.Schedule.Schedule.Name == scheduleName { - if !exist { - t.Errorf("verifySchedule: schedule should not be exist") - return false - } + if ssCopy.Schedule.Name != scheduleName { + continue + } + if !exist { + t.Errorf("verifySchedule: schedule should not be exist") + return false + } + scheduleFound = true - if expectedPrefix != ssCopy.Schedule.Prefix { - t.Errorf("verifySchedule: got = %s, want %s", ssCopy.Schedule.Prefix, expectedPrefix) - return false - } - if expectedSMLabel != ssCopy.Schedule.SnapmirrorLabel { - t.Errorf("verifySchedule: got = %s, want %s", ssCopy.Schedule.SnapmirrorLabel, expectedSMLabel) - return false - } - if expectedCount != ssCopy.Schedule.Count { - t.Errorf("verifySchedule: got = %d, want %d", ssCopy.Schedule.Count, expectedCount) - return false - } + if expectedSMLabel != ssCopy.SnapmirrorLabel { + t.Errorf("verifySchedule: got = %s, want %s", ssCopy.SnapmirrorLabel, expectedSMLabel) + return false + } + if expectedCount != ssCopy.Count { + t.Errorf("verifySchedule: got = %d, want %d", ssCopy.Count, expectedCount) + return false } } + if !scheduleFound && exist { + t.Errorf("verifySchedule: schedule must be exist") + return false + } + return true } } diff --git a/ontap/ontap.go b/ontap/ontap.go index 8ce67e0..5d72e28 100644 --- a/ontap/ontap.go +++ b/ontap/ontap.go @@ -152,7 +152,6 @@ type Schedule struct { type SnapshotPolicySchedule struct { Count int `json:"count,omitzero" jsonschema:"number of snapshots to keep for this schedule"` Schedule NameAndUUID `json:"schedule,omitzero" jsonschema:"name of the schedule"` - Prefix string `json:"prefix,omitzero" jsonschema:"snapshot name prefix for this schedule"` SnapmirrorLabel string `json:"snapmirror_label,omitzero" jsonschema:"SnapMirror label for this schedule"` } diff --git a/rest/snapshotpolicy.go b/rest/snapshotpolicy.go index de5b203..6a70f5e 100644 --- a/rest/snapshotpolicy.go +++ b/rest/snapshotpolicy.go @@ -10,16 +10,19 @@ import ( func (c *Client) CreateSnapshotPolicy(ctx context.Context, snapshotPolicy ontap.SnapshotPolicy) error { var ( - statusCode int - oc ontap.OnlyCount - err error + statusCode int + scheduleName string + oc ontap.OnlyCount + err error ) responseHeaders := http.Header{} // If schedule is exist then use it else create new - scheduleName := snapshotPolicy.Copies[0].Schedule.Name + if len(snapshotPolicy.Copies) > 0 { + scheduleName = snapshotPolicy.Copies[0].Schedule.Name + } if scheduleName == "" { - return fmt.Errorf("no schedule exist with %s name", scheduleName) + return fmt.Errorf("schedule name must be required in snapshot policy %s", snapshotPolicy.Name) } params := url.Values{} params.Set("return_records", "false") @@ -56,35 +59,17 @@ func (c *Client) CreateSnapshotPolicy(ctx context.Context, snapshotPolicy ontap. func (c *Client) UpdateSnapshotPolicy(ctx context.Context, snapshotPolicy ontap.SnapshotPolicy, snapshotPolicyName string, svmName string) error { var ( statusCode int - ssPolicy ontap.GetData ) responseHeaders := http.Header{} - params := url.Values{} - params.Set("fields", "uuid") - params.Set("name", snapshotPolicyName) - - builder := c.baseRequestBuilder(`/api/storage/snapshot-policies`, nil, responseHeaders). - Params(params). - ToJSON(&ssPolicy) - - err := c.buildAndExecuteRequest(ctx, builder) - + policyUUID, err := c.getSnapshotPolicyUUID(ctx, snapshotPolicyName, svmName) if err != nil { return err } - if ssPolicy.NumRecords == 0 { - return fmt.Errorf("failed to get snapshotPolicy=%s on svm=%s because it does not exist", snapshotPolicyName, svmName) - } - if ssPolicy.NumRecords != 1 { - return fmt.Errorf("failed to get snapshotPolicy=%s on svm=%s because there are %d matching records", - snapshotPolicyName, svmName, ssPolicy.NumRecords) - } - - builder2 := c.baseRequestBuilder(`/api/storage/snapshot-policies`, &statusCode, responseHeaders). + builder := c.baseRequestBuilder(`/api/storage/snapshot-policies`+policyUUID, &statusCode, responseHeaders). BodyJSON(snapshotPolicy) - if err := c.buildAndExecuteRequest(ctx, builder2); err != nil { + if err := c.buildAndExecuteRequest(ctx, builder); err != nil { return err } @@ -94,35 +79,17 @@ func (c *Client) UpdateSnapshotPolicy(ctx context.Context, snapshotPolicy ontap. func (c *Client) DeleteSnapshotPolicy(ctx context.Context, snapshotPolicy ontap.SnapshotPolicy) error { var ( statusCode int - ssPolicy ontap.GetData ) responseHeaders := http.Header{} - params := url.Values{} - params.Set("fields", "uuid") - params.Set("name", snapshotPolicy.Name) - - builder := c.baseRequestBuilder(`/api/storage/snapshot-policies`, nil, responseHeaders). - Params(params). - ToJSON(&ssPolicy) - - err := c.buildAndExecuteRequest(ctx, builder) - + policyUUID, err := c.getSnapshotPolicyUUID(ctx, snapshotPolicy.Name, snapshotPolicy.SVM.Name) if err != nil { return err } - if ssPolicy.NumRecords == 0 { - return fmt.Errorf("failed to get snapshotPolicy=%s on svm=%s because it does not exist", snapshotPolicy.Name, snapshotPolicy.SVM.Name) - } - if ssPolicy.NumRecords != 1 { - return fmt.Errorf("failed to get snapshotPolicy=%s on svm=%s because there are %d matching records", - snapshotPolicy.Name, snapshotPolicy.SVM.Name, ssPolicy.NumRecords) - } - - builder2 := c.baseRequestBuilder(`/api/storage/snapshot-policies/`+ssPolicy.Records[0].UUID, &statusCode, responseHeaders). + builder := c.baseRequestBuilder(`/api/storage/snapshot-policies/`+policyUUID, &statusCode, responseHeaders). Delete() - if err := c.buildAndExecuteRequest(ctx, builder2); err != nil { + if err := c.buildAndExecuteRequest(ctx, builder); err != nil { return err } diff --git a/server/Snapshotpolicy.go b/server/Snapshotpolicy.go index f975049..0a82352 100644 --- a/server/Snapshotpolicy.go +++ b/server/Snapshotpolicy.go @@ -392,9 +392,6 @@ func newAddScheduleInSnapshotPolicy(in tool.SnapshotPolicySchedule) (ontap.Snaps out.Schedule = ontap.NameAndUUID{Name: in.ScheduleName} out.Count = in.Count - if in.Prefix != "" { - out.Prefix = in.Prefix - } if in.SnapmirrorLabel != "" { out.SnapmirrorLabel = in.SnapmirrorLabel } @@ -414,13 +411,10 @@ func newUpdateScheduleInSnapshotPolicy(in tool.SnapshotPolicySchedule) (ontap.Sn if in.ScheduleName == "" { return out, errors.New("schedule name is required") } - if in.Count == 0 && in.Prefix == "" && in.SnapmirrorLabel == "" { - return out, errors.New("at least one supported update field must be provided; count, prefix, and snapmirror_label are supported for update") + if in.Count == 0 && in.SnapmirrorLabel == "" { + return out, errors.New("at least one supported update field must be provided; count and snapmirror_label are supported for update") } - if in.Prefix != "" { - out.Prefix = in.Prefix - } if in.SnapmirrorLabel != "" { out.SnapmirrorLabel = in.SnapmirrorLabel } diff --git a/tool/tool.go b/tool/tool.go index e007fca..1156e79 100644 --- a/tool/tool.go +++ b/tool/tool.go @@ -60,7 +60,6 @@ type SnapshotPolicySchedule struct { PolicyName string `json:"policy_name" jsonschema:"snapshot policy name"` ScheduleName string `json:"schedule_name" jsonschema:"name of the schedule"` Count int `json:"count,omitzero" jsonschema:"number of snapshots to keep for this schedule"` - Prefix string `json:"prefix,omitzero" jsonschema:"snapshot name prefix for this schedule"` SnapmirrorLabel string `json:"snapmirror_label,omitzero" jsonschema:"SnapMirror label for this schedule"` } From d550b9571e73d200e4f860e3bd451f66900eb731 Mon Sep 17 00:00:00 2001 From: hardikl Date: Wed, 15 Apr 2026 20:34:12 +0530 Subject: [PATCH 3/4] feat: handled review comments --- integration/test/snapshotPolicy_test.go | 50 ++++++++++++++++++++++--- rest/snapshotpolicy.go | 5 ++- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/integration/test/snapshotPolicy_test.go b/integration/test/snapshotPolicy_test.go index 9a4fbc8..7d90b57 100644 --- a/integration/test/snapshotPolicy_test.go +++ b/integration/test/snapshotPolicy_test.go @@ -80,26 +80,26 @@ func TestSnapshot(t *testing.T) { }, { name: "Update snapshot policy every4hours", - input: ClusterStr + "disable a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM with comment as `4 hour policy`", + input: ClusterStr + "disable a snapshot policy named " + rn("every4hours") + " on the " + rn("marketing") + " SVM with comment as `4_hour_policy`", expectedOntapErr: "", - verifyAPI: ontapVerifier{}, + verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours") + "&svm.name=" + rn("marketing") + "&fields=enabled,comment", validationFunc: verifySnapshotPolicy(false, "4_hour_policy")}, }, { name: "Clean snapshot policy every4hours", input: ClusterStr + "delete " + rn("every4hours") + " snapshot policy in " + rn("marketing") + " svm", - expectedOntapErr: "because it does not exist", + expectedOntapErr: "", verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every4hours"), validationFunc: deleteObject}, }, { name: "Clean snapshot policy every5min", input: ClusterStr + "Delete " + rn("every5min") + " snapshot policy in " + rn("marketing") + " svm", - expectedOntapErr: "because it does not exist", + expectedOntapErr: "", verifyAPI: ontapVerifier{api: "api/storage/snapshot-policies?name=" + rn("every5min"), validationFunc: deleteObject}, }, { name: "Clean SVM", input: ClusterStr + "delete " + rn("marketing") + " svm", - expectedOntapErr: "because it does not exist", + expectedOntapErr: "", verifyAPI: ontapVerifier{api: "api/svm/svms?name=" + rn("marketing"), validationFunc: deleteObject}, }, } @@ -198,3 +198,43 @@ func verifySchedule(exist bool, scheduleName string, expectedSMLabel string, exp return true } } + +func verifySnapshotPolicy(expectedState bool, expectedComment string) func(t *testing.T, api string, poller *config.Poller, client *http.Client) bool { + return func(t *testing.T, api string, poller *config.Poller, client *http.Client) bool { + type SnapshotPolicy struct { + Enabled bool `json:"enabled"` + Comment string `json:"comment"` + } + type response struct { + NumRecords int `json:"num_records"` + Records []SnapshotPolicy `json:"records"` + } + + var data response + err := requests.URL("https://"+poller.Addr+"/"+api). + BasicAuth(poller.Username, poller.Password). + Client(client). + ToJSON(&data). + Fetch(context.Background()) + if err != nil { + t.Errorf("verifySnapshotPolicy: request failed: %v", err) + return false + } + if data.NumRecords != 1 { + t.Errorf("verifySnapshotPolicy: expected 1 record, got %d", data.NumRecords) + return false + } + + gotSPolicy := data.Records[0] + if expectedState != gotSPolicy.Enabled { + t.Errorf("verifySnapshotPolicy: got = %v, want %v", gotSPolicy.Enabled, expectedState) + return false + } + if expectedComment != gotSPolicy.Comment { + t.Errorf("verifySnapshotPolicy: got = %s, want %s", gotSPolicy.Comment, expectedComment) + return false + } + + return true + } +} diff --git a/rest/snapshotpolicy.go b/rest/snapshotpolicy.go index 6a70f5e..100b5b5 100644 --- a/rest/snapshotpolicy.go +++ b/rest/snapshotpolicy.go @@ -66,8 +66,9 @@ func (c *Client) UpdateSnapshotPolicy(ctx context.Context, snapshotPolicy ontap. return err } - builder := c.baseRequestBuilder(`/api/storage/snapshot-policies`+policyUUID, &statusCode, responseHeaders). - BodyJSON(snapshotPolicy) + builder := c.baseRequestBuilder(`/api/storage/snapshot-policies/`+policyUUID, &statusCode, responseHeaders). + BodyJSON(snapshotPolicy). + Patch() if err := c.buildAndExecuteRequest(ctx, builder); err != nil { return err From a3d5902d289e19353862f991b08d255c24d606dd Mon Sep 17 00:00:00 2001 From: hardikl Date: Thu, 16 Apr 2026 14:49:12 +0530 Subject: [PATCH 4/4] feat: handle comments --- descriptions/descriptions.go | 2 +- rest/snapshotpolicy.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/descriptions/descriptions.go b/descriptions/descriptions.go index 3b6ae94..c5839b9 100644 --- a/descriptions/descriptions.go +++ b/descriptions/descriptions.go @@ -42,7 +42,7 @@ const DeleteSnapshotPolicy = `Delete a snapshot policy on a cluster by cluster n const CreateSchedule = `Create a cron schedule on a cluster by cluster name. Ex: 5 1 * * *, this cron expression indicates schedule would be triggered at 01:05 AM for every day` const AddScheduleInSnapshotPolicy = `Add a schedule entry to an existing snapshot policy on a cluster by cluster name.` -const UpdateScheduleInSnapshotPolicy = `Update a schedule entry within an existing snapshot policy on a cluster by cluster name. At least one of count, prefix, or snapmirror_label must be provided.` +const UpdateScheduleInSnapshotPolicy = `Update a schedule entry within an existing snapshot policy on a cluster by cluster name. At least one of count or snapmirror_label must be provided.` const RemoveScheduleInSnapshotPolicy = `Remove a schedule entry from an existing snapshot policy on a cluster by cluster name.` const ListQoSPolicies = `List QoS policies from an ONTAP cluster — includes both SVM-scoped and cluster-scoped (admin SVM) policies. diff --git a/rest/snapshotpolicy.go b/rest/snapshotpolicy.go index 100b5b5..5b23998 100644 --- a/rest/snapshotpolicy.go +++ b/rest/snapshotpolicy.go @@ -22,7 +22,7 @@ func (c *Client) CreateSnapshotPolicy(ctx context.Context, snapshotPolicy ontap. scheduleName = snapshotPolicy.Copies[0].Schedule.Name } if scheduleName == "" { - return fmt.Errorf("schedule name must be required in snapshot policy %s", snapshotPolicy.Name) + return fmt.Errorf("schedule name is required in snapshot policy %s", snapshotPolicy.Name) } params := url.Values{} params.Set("return_records", "false")