Skip to content

Commit b72ea21

Browse files
ycohen278ycohen278
authored
Revert "Update bucket attributes in order to disable soft delete (#155)" (#156)
This reverts commit a74b21b.
1 parent a74b21b commit b72ea21

File tree

5 files changed

+12
-77
lines changed

5 files changed

+12
-77
lines changed

‎cli_tools/common/domain/interfaces.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
// StorageClientInterface represents GCS storage client
2626
type StorageClientInterface interface {
2727
CreateBucket(bucketName string, project string, attrs *storage.BucketAttrs) error
28-
UpdateBucket(bucketName string, attrs storage.BucketAttrsToUpdate) error
2928
Buckets(projectID string) *storage.BucketIterator
3029
GetBucketAttrs(bucket string) (*storage.BucketAttrs, error)
3130
GetBucket(bucket string) *storage.BucketHandle

‎cli_tools/common/utils/storage/scratch_bucket_creator.go‎

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,15 @@ func (c *ScratchBucketCreator) getBucketAttrs(fileGcsPath string, project string
7777
// if file is not provided, fallback to input / default zone.
7878
bucketAttrs, err = c.getBucketAttrsOnFallbackZone(project, fallbackZone)
7979
}
80-
if err != nil {
81-
return nil, err
80+
81+
if err == nil {
82+
// Enable Uniform-Bucket-Level-Access by default in image-import/export tools.
83+
bucketAttrs.UniformBucketLevelAccess.Enabled = enableUniformBucketLevelAccess
8284
}
8385

84-
// Enable Uniform-Bucket-Level-Access by default in image-import/export tools.
85-
bucketAttrs.UniformBucketLevelAccess.Enabled = enableUniformBucketLevelAccess
86-
// Disable soft delete.
87-
bucketAttrs.SoftDeletePolicy = &storage.SoftDeletePolicy{RetentionDuration: 0}
86+
if bucketAttrs != nil {
87+
bucketAttrs.SoftDeletePolicy = &storage.SoftDeletePolicy{RetentionDuration: 0}
88+
}
8889

8990
return bucketAttrs, err
9091
}
@@ -133,28 +134,14 @@ func (c *ScratchBucketCreator) createBucketIfNotExisting(project string,
133134
if err != nil {
134135
return "", err
135136
}
136-
137-
if foundBucketAttrs == nil {
138-
log.Printf("Creating scratch bucket `%v` in %v region", bucketAttrs.Name, bucketAttrs.Location)
139-
if err := c.StorageClient.CreateBucket(bucketAttrs.Name, project, bucketAttrs); err != nil {
140-
return "", err
141-
}
142-
}
143-
log.Printf("Updating soft delete property of scratch bucket `%v` in %v region", bucketAttrs.Name, bucketAttrs.Location)
144-
if err = c.removeSoftDeleteFromBucket(bucketAttrs.Name); err != nil {
145-
return "", err
146-
}
147137
if foundBucketAttrs != nil {
148138
return foundBucketAttrs.Location, nil
149139
}
150-
return bucketAttrs.Location, nil
151-
}
152-
153-
func (c *ScratchBucketCreator) removeSoftDeleteFromBucket(bucketName string) error {
154-
bucketAttrToUpdate := storage.BucketAttrsToUpdate{
155-
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
140+
log.Printf("Creating scratch bucket `%v` in %v region", bucketAttrs.Name, bucketAttrs.Location)
141+
if err := c.StorageClient.CreateBucket(bucketAttrs.Name, project, bucketAttrs); err != nil {
142+
return "", err
156143
}
157-
return c.StorageClient.UpdateBucket(bucketName, bucketAttrToUpdate)
144+
return bucketAttrs.Location, nil
158145
}
159146

160147
// IsBucketInProject checks if bucket belongs to a project

‎cli_tools/common/utils/storage/scratch_bucket_creator_test.go‎

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,6 @@ func TestCreateScratchBucketNoSourceFileDefaultBucketCreatedBasedOnDefaultRegion
5858
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
5959
}).Return(nil)
6060

61-
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
62-
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
63-
}).Return(nil)
64-
6561
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
6662
bucket, region, err := c.CreateScratchBucket("", project, "", true)
6763
assert.Equal(t, expectedBucket, bucket)
@@ -87,10 +83,6 @@ func TestCreateScratchBucketNoSourceFileTranslateGoogleDomainDefaultBucketCreate
8783
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
8884
}).Return(nil)
8985

90-
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
91-
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
92-
}).Return(nil)
93-
9486
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
9587
bucket, region, err := c.CreateScratchBucket("", project, "", true)
9688
assert.Equal(t, expectedBucket, bucket)
@@ -116,10 +108,6 @@ func TestCreateScratchBucketNoSourceFileBucketCreatedBasedOnInputZone(t *testing
116108
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
117109
}).Return(nil)
118110

119-
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
120-
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
121-
}).Return(nil)
122-
123111
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
124112
bucket, region, err := c.CreateScratchBucket("", project, "asia-east1-b", true)
125113
assert.Equal(t, expectedBucket, bucket)
@@ -185,9 +173,7 @@ func TestCreateScratchBucketNewBucketCreatedProject(t *testing.T) {
185173
mockStorageClient := mocks.NewMockStorageClientInterface(mockCtrl)
186174
mockStorageClient.EXPECT().GetBucketAttrs(sourceBucketAttrs.Name).Return(sourceBucketAttrs, nil).Times(1)
187175
mockStorageClient.EXPECT().CreateBucket("project1-daisy-bkt-us-west2", project, scratchBucketAttrs).Return(nil).Times(1)
188-
mockStorageClient.EXPECT().UpdateBucket("project1-daisy-bkt-us-west2", storage.BucketAttrsToUpdate{
189-
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
190-
}).Return(nil).Times(1)
176+
191177
mockBucketIterator := mocks.NewMockBucketIteratorInterface(mockCtrl)
192178
first := mockBucketIterator.EXPECT().Next().Return(anotherBucketAttrs, nil)
193179
second := mockBucketIterator.EXPECT().Next().Return(sourceBucketAttrs, nil)
@@ -244,9 +230,6 @@ func TestCreateScratchBucketErrorRetrievingSourceFileBucketMetadataDefaultBucket
244230
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
245231
}).Return(nil)
246232

247-
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
248-
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
249-
}).Return(nil).Times(1)
250233
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
251234
bucket, region, err := c.CreateScratchBucket("gs://sourcebucket/sourcefile", project, "", true)
252235
assert.Equal(t, expectedBucket, bucket)
@@ -273,10 +256,6 @@ func TestCreateScratchBucketErrorRetrievingSourceFileBucketMetadataBucketCreated
273256
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
274257
}).Return(nil)
275258

276-
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
277-
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
278-
}).Return(nil)
279-
280259
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
281260
bucket, region, err := c.CreateScratchBucket("gs://sourcebucket/sourcefile", project, "asia-east1-b", true)
282261
assert.Equal(t, expectedBucket, bucket)
@@ -303,10 +282,6 @@ func TestCreateScratchBucketNilSourceFileBucketMetadataDefaultBucketCreated(t *t
303282
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
304283
}).Return(nil)
305284

306-
mockStorageClient.EXPECT().UpdateBucket(expectedBucket, storage.BucketAttrsToUpdate{
307-
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
308-
}).Return(nil)
309-
310285
c := ScratchBucketCreator{mockStorageClient, ctx, createMockBucketIteratorWithRandomBuckets(mockCtrl, &ctx, mockStorageClient, project)}
311286
bucket, region, err := c.CreateScratchBucket("gs://sourcebucket/sourcefile", project, "", true)
312287
assert.Equal(t, expectedBucket, bucket)
@@ -381,9 +356,6 @@ func TestCreateScratchBucketReturnsExistingScratchBucketNoCreate(t *testing.T) {
381356
Return(mockBucketIterator).
382357
Times(1)
383358

384-
mockStorageClient.EXPECT().UpdateBucket("project1-daisy-bkt-us-west2", storage.BucketAttrsToUpdate{
385-
SoftDeletePolicy: &storage.SoftDeletePolicy{RetentionDuration: 0},
386-
}).Return(nil).Times(1)
387359
c := ScratchBucketCreator{mockStorageClient, ctx, mockBucketIteratorCreator}
388360
bucket, region, err := c.CreateScratchBucket("gs://sourcebucket/sourcefile", projectID, "", true)
389361
assert.Equal(t, "project1-daisy-bkt-us-west2", bucket)

‎cli_tools/common/utils/storage/storage_client.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,6 @@ func (sc *Client) CreateBucket(
7373
return nil
7474
}
7575

76-
// UpdateBucket updates a GCS bucket
77-
func (sc *Client) UpdateBucket(
78-
bucketName string, attrs storage.BucketAttrsToUpdate) error {
79-
if _, err := sc.StorageClient.Bucket(bucketName).Update(sc.Ctx, attrs); err != nil {
80-
return daisy.Errf("Error updating bucket `%v` : %v", bucketName, err)
81-
}
82-
return nil
83-
}
84-
8576
// Buckets returns a bucket iterator for all buckets within a project
8677
func (sc *Client) Buckets(projectID string) *storage.BucketIterator {
8778
return sc.StorageClient.Buckets(sc.Ctx, projectID)

‎cli_tools/mocks/mock_storage_client.go‎

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

0 commit comments

Comments
 (0)