Skip to content

Commit 8b07d49

Browse files
committed
Update to fetch snapshots using pagination
Modified ListSnapshots to use pagination, ensuring all snapshots are fetched regardless of the total count. This should, in theory, resolve the issue where having too many old snapshots prevents the latest ones from being seen, which was resulting in unnecessary creation of new snapshots.
1 parent 033a1fd commit 8b07d49

File tree

3 files changed

+108
-96
lines changed

3 files changed

+108
-96
lines changed

snapshot/client.go

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7-
log "github.com/sirupsen/logrus"
87
"net/http"
98
"strings"
109
"time"
1110

11+
log "github.com/sirupsen/logrus"
12+
1213
"github.com/pkg/errors"
1314
"golang.org/x/oauth2/google"
1415
compute "google.golang.org/api/compute/v1"
@@ -33,8 +34,8 @@ type GCPSnapClient struct {
3334
type GCPSnapClientInterface interface {
3435
GetDisksFromLabel(label *models.Label) ([]compute.Disk, error)
3536
GetDisksFromDescription(label *models.Description) ([]compute.Disk, error)
36-
ListSnapshots(diskSelfLink string) ([]compute.Snapshot, error)
37-
ListClientCreatedSnapshots(diskSelfLink string) ([]compute.Snapshot, error)
37+
ListSnapshots(diskSelfLink string) ([]*compute.Snapshot, error)
38+
ListClientCreatedSnapshots(diskSelfLink string) ([]*compute.Snapshot, error)
3839
CreateSnapshot(diskName, zone string) (string, error)
3940
DeleteSnapshot(snapName string) (string, error)
4041
GetZonalOperationStatus(operation, zone string) (string, error)
@@ -43,7 +44,6 @@ type GCPSnapClientInterface interface {
4344

4445
// Basic Init function for the snapshotter
4546
func CreateGCPSnapClient(project, snapPrefix string, zones []string) *GCPSnapClient {
46-
4747
ctx := context.Background()
4848
googleClient, err := google.DefaultClient(ctx, compute.ComputeScope)
4949
if err != nil {
@@ -65,7 +65,6 @@ func CreateGCPSnapClient(project, snapPrefix string, zones []string) *GCPSnapCli
6565

6666
// In case of a gcp link it returns the target (final part after /)
6767
func formatLinkString(in string) string {
68-
6968
if strings.ContainsAny(in, "/") {
7069
elems := strings.Split(in, "/")
7170
return elems[len(elems)-1]
@@ -75,7 +74,6 @@ func formatLinkString(in string) string {
7574

7675
// GetDiskList: Returns a list of disks that contain one of the given labels
7776
func (gsc *GCPSnapClient) GetDisksFromLabel(label *models.Label) ([]compute.Disk, error) {
78-
7977
disks := []compute.Disk{}
8078

8179
for _, zone := range gsc.Zones {
@@ -97,7 +95,6 @@ func (gsc *GCPSnapClient) GetDisksFromLabel(label *models.Label) ([]compute.Disk
9795
}
9896

9997
func (gsc *GCPSnapClient) GetDisksFromDescription(desc *models.Description) ([]compute.Disk, error) {
100-
10198
disks := []compute.Disk{}
10299

103100
for _, zone := range gsc.Zones {
@@ -124,42 +121,47 @@ func (gsc *GCPSnapClient) GetDisksFromDescription(desc *models.Description) ([]c
124121
}
125122

126123
// ListSnapshots: Lists Snapshots for a given disk
127-
func (gsc *GCPSnapClient) ListSnapshots(diskSelfLink string) ([]compute.Snapshot, error) {
124+
func (gsc *GCPSnapClient) ListSnapshots(diskSelfLink string) ([]*compute.Snapshot, error) {
125+
var snapshots []*compute.Snapshot
128126

129-
snapshots := []compute.Snapshot{}
127+
req := gsc.ComputeService.Snapshots.List(gsc.Project)
130128

131-
resp, err := gsc.ComputeService.Snapshots.List(gsc.Project).Do()
132-
if err != nil {
133-
return snapshots, errors.Wrap(err, "error requesting snapshots list:")
134-
}
135-
for _, snap := range resp.Items {
136-
// If not created by the snapshotter just ignore
137-
if val, ok := snap.Labels[SnapshotterLabel]; ok {
138-
if val != SnapshotterLabelValue {
129+
err := req.Pages(context.Background(), func(page *compute.SnapshotList) error {
130+
for _, snap := range page.Items {
131+
// If not created by the snapshotter just ignore
132+
if val, ok := snap.Labels[SnapshotterLabel]; ok {
133+
if val != SnapshotterLabelValue {
134+
continue
135+
}
136+
} else {
139137
continue
140138
}
141-
} else {
142-
continue
143-
}
144139

145-
// If it's a snapshot of the input disk add to the list
146-
if snap.SourceDisk == diskSelfLink {
147-
snapshots = append(snapshots, *snap)
140+
// Skip if snapshot is not from the requested disk
141+
if snap.SourceDisk != diskSelfLink {
142+
continue
143+
}
144+
145+
snapshots = append(snapshots, snap)
148146
}
147+
return nil
148+
})
149+
if err != nil {
150+
return nil, errors.Wrap(err, "error requesting snapshots list:")
149151
}
152+
150153
return snapshots, nil
151154
}
152155

153-
// ListClientCreatedSnapshots: Lists snapshots for a given disk that were create by the client,
156+
// ListClientCreatedSnapshots: Lists snapshots for a given disk that were created by the client,
154157
// meaning that they have the SnapshotterLabel
155-
func (gsc *GCPSnapClient) ListClientCreatedSnapshots(diskSelfLink string) ([]compute.Snapshot, error) {
156-
158+
func (gsc *GCPSnapClient) ListClientCreatedSnapshots(diskSelfLink string) ([]*compute.Snapshot, error) {
157159
snaps, err := gsc.ListSnapshots(diskSelfLink)
158160
if err != nil {
159-
return []compute.Snapshot{}, err
161+
return nil, err
160162
}
161163

162-
res := []compute.Snapshot{}
164+
res := []*compute.Snapshot{}
163165
for _, snap := range snaps {
164166
if val, ok := snap.Labels[SnapshotterLabel]; ok {
165167
if SnapshotterLabelValue == val {
@@ -175,7 +177,6 @@ func (gsc *GCPSnapClient) ListClientCreatedSnapshots(diskSelfLink string) ([]com
175177
// CreateSnapshot: Gets a disk name and a zone, issues a create snapshot command to api
176178
// and returns a link to the create snapshot operation
177179
func (gsc *GCPSnapClient) CreateSnapshot(diskName, zone string) (string, error) {
178-
179180
// format zone if link
180181
zn := formatLinkString(zone)
181182

@@ -205,7 +206,6 @@ func (gsc *GCPSnapClient) CreateSnapshot(diskName, zone string) (string, error)
205206

206207
// DeleteSnapshot: Gets a snapshot name and issues a delete. Returns a link to the delete operation
207208
func (gsc *GCPSnapClient) DeleteSnapshot(snapName string) (string, error) {
208-
209209
resp, err := gsc.ComputeService.Snapshots.Delete(gsc.Project, snapName).Do()
210210
if err != nil {
211211
return "", errors.Wrap(err, "error deleting snapshot:")
@@ -215,7 +215,6 @@ func (gsc *GCPSnapClient) DeleteSnapshot(snapName string) (string, error) {
215215
}
216216

217217
func parseOperationOut(operation *compute.Operation) (string, error) {
218-
219218
// Get status (Possible values: "DONE", "PENDING", "RUNNING") and errors
220219
status := operation.Status
221220
if operation.Error != nil {
@@ -226,11 +225,9 @@ func parseOperationOut(operation *compute.Operation) (string, error) {
226225
return status, errors.New(strings.Join(err_msgs, ","))
227226
}
228227
return status, nil
229-
230228
}
231229

232230
func (gsc *GCPSnapClient) GetZonalOperationStatus(operation, zone string) (string, error) {
233-
234231
// Format in case of link
235232
operation = formatLinkString(operation)
236233
zone = formatLinkString(zone)
@@ -244,7 +241,6 @@ func (gsc *GCPSnapClient) GetZonalOperationStatus(operation, zone string) (strin
244241
}
245242

246243
func (gsc *GCPSnapClient) GetGlobalOperationStatus(operation string) (string, error) {
247-
248244
// Format in case of link
249245
operation = formatLinkString(operation)
250246

snapshot/mock_client.go

Lines changed: 77 additions & 60 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

watch/watch.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ type WatcherInterface interface {
2929
}
3030

3131
func (w *Watcher) Watch(sc *models.SnapshotConfigs) {
32-
3332
for t := time.Tick(time.Second * time.Duration(w.WatchInterval)); ; <-t {
3433

3534
// Check Labels
@@ -85,7 +84,7 @@ func (w *Watcher) CheckAndSnapDisks(disks []compute.Disk, retentionStart, lastAc
8584

8685
// If created before retention start time we need to delete
8786
if snapTime.Before(retentionStart) {
88-
snapsToDelete = append(snapsToDelete, snap)
87+
snapsToDelete = append(snapsToDelete, *snap)
8988
}
9089

9190
// If a snap was taken after last accepted creation time we do not need a new one

0 commit comments

Comments
 (0)