Skip to content

Commit 2d92ff0

Browse files
committed
Use feature gate for S3 storage
1 parent 500588e commit 2d92ff0

File tree

2 files changed

+53
-46
lines changed

2 files changed

+53
-46
lines changed

pkg/cloud/services/s3/s3.go

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/pkg/errors"
3434

3535
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
36+
"sigs.k8s.io/cluster-api-provider-aws/v2/feature"
3637
iam "sigs.k8s.io/cluster-api-provider-aws/v2/iam/api/v1beta1"
3738
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
3839
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/userdata"
@@ -97,43 +98,46 @@ func (s *Service) DeleteBucket() error {
9798

9899
log.Info("Deleting S3 Bucket")
99100

100-
// Delete machine pool user data files that did not get deleted
101-
// yet by the lifecycle policy
102-
for {
103-
log.Info("Listing S3 objects of machine pools")
101+
if feature.Gates.Enabled(feature.MachinePool) {
102+
// Delete machine pool user data files that did not get deleted
103+
// yet by the lifecycle policy
104+
for {
105+
log.Info("Listing S3 objects of machine pools")
104106

105-
out, err := s.S3Client.ListObjectsV2(&s3.ListObjectsV2Input{
106-
Bucket: aws.String(bucketName),
107-
Prefix: aws.String("machine-pool/"),
108-
})
109-
if err != nil {
110-
aerr, ok := err.(awserr.Error)
111-
if !ok {
112-
return errors.Wrap(err, "listing S3 bucket")
107+
// TODO Switch to aws-sdk-go-v2 which has NewListObjectsV2Paginator (as part of https://github.com/kubernetes-sigs/cluster-api-provider-aws/issues/2225)
108+
out, err := s.S3Client.ListObjectsV2(&s3.ListObjectsV2Input{
109+
Bucket: aws.String(bucketName),
110+
Prefix: aws.String("machine-pool/"),
111+
})
112+
if err != nil {
113+
aerr, ok := err.(awserr.Error)
114+
if !ok {
115+
return errors.Wrap(err, "listing S3 bucket")
116+
}
117+
118+
switch aerr.Code() {
119+
case s3.ErrCodeNoSuchBucket:
120+
log.Info("Bucket already removed")
121+
return nil
122+
default:
123+
return errors.Wrap(aerr, "listing S3 bucket")
124+
}
113125
}
114126

115-
switch aerr.Code() {
116-
case s3.ErrCodeNoSuchBucket:
117-
log.Info("Bucket already removed")
118-
return nil
119-
default:
120-
return errors.Wrap(aerr, "listing S3 bucket")
127+
// Stop on last page of results
128+
if len(out.Contents) == 0 {
129+
break
121130
}
122-
}
123-
124-
// Stop on last page of results
125-
if len(out.Contents) == 0 {
126-
break
127-
}
128131

129-
log.Info("Deleting S3 objects of machine pools", "count", len(out.Contents))
130-
for _, obj := range out.Contents {
131-
_, err := s.S3Client.DeleteObject(&s3.DeleteObjectInput{
132-
Bucket: aws.String(bucketName),
133-
Key: obj.Key,
134-
})
135-
if err != nil {
136-
return err
132+
log.Info("Deleting S3 objects of machine pools", "count", len(out.Contents))
133+
for _, obj := range out.Contents {
134+
_, err := s.S3Client.DeleteObject(&s3.DeleteObjectInput{
135+
Bucket: aws.String(bucketName),
136+
Key: obj.Key,
137+
})
138+
if err != nil {
139+
return err
140+
}
137141
}
138142
}
139143
}
@@ -342,6 +346,10 @@ func (s *Service) ensureBucketPolicy(bucketName string) error {
342346
}
343347

344348
func (s *Service) ensureBucketLifecycleConfiguration(bucketName string) error {
349+
if !feature.Gates.Enabled(feature.MachinePool) {
350+
return nil
351+
}
352+
345353
input := &s3.PutBucketLifecycleConfigurationInput{
346354
Bucket: aws.String(bucketName),
347355
LifecycleConfiguration: &s3.BucketLifecycleConfiguration{

pkg/cloud/services/s3/s3_test.go

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,11 @@ import (
3232
"github.com/golang/mock/gomock"
3333
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3434
"k8s.io/apimachinery/pkg/runtime"
35+
utilfeature "k8s.io/component-base/featuregate/testing"
3536
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3637

3738
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
39+
"sigs.k8s.io/cluster-api-provider-aws/v2/feature"
3840
iamv1 "sigs.k8s.io/cluster-api-provider-aws/v2/iam/api/v1beta1"
3941
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
4042
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/s3"
@@ -49,8 +51,6 @@ const (
4951
)
5052

5153
func TestReconcileBucket(t *testing.T) {
52-
t.Parallel()
53-
5454
t.Run("does_nothing_when_bucket_management_is_disabled", func(t *testing.T) {
5555
t.Parallel()
5656

@@ -62,7 +62,7 @@ func TestReconcileBucket(t *testing.T) {
6262
})
6363

6464
t.Run("creates_bucket_with_configured_name", func(t *testing.T) {
65-
t.Parallel()
65+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
6666

6767
expectedBucketName := "baz"
6868

@@ -107,7 +107,7 @@ func TestReconcileBucket(t *testing.T) {
107107
})
108108

109109
t.Run("hashes_default_bucket_name_if_name_exceeds_maximum_length", func(t *testing.T) {
110-
t.Parallel()
110+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
111111

112112
mockCtrl := gomock.NewController(t)
113113
s3Mock := mock_s3iface.NewMockS3API(mockCtrl)
@@ -163,7 +163,7 @@ func TestReconcileBucket(t *testing.T) {
163163
})
164164

165165
t.Run("creates_bucket_with_policy_allowing_controlplane_and_worker_nodes_to_read_their_secrets", func(t *testing.T) {
166-
t.Parallel()
166+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
167167

168168
bucketName := "bar"
169169

@@ -216,7 +216,7 @@ func TestReconcileBucket(t *testing.T) {
216216
})
217217

218218
t.Run("is_idempotent", func(t *testing.T) {
219-
t.Parallel()
219+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
220220

221221
svc, s3Mock := testService(t, &infrav1.S3Bucket{})
222222

@@ -235,7 +235,7 @@ func TestReconcileBucket(t *testing.T) {
235235
})
236236

237237
t.Run("ignores_when_bucket_already_exists_but_its_owned_by_the_same_account", func(t *testing.T) {
238-
t.Parallel()
238+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
239239

240240
svc, s3Mock := testService(t, &infrav1.S3Bucket{})
241241

@@ -318,7 +318,7 @@ func TestDeleteBucket(t *testing.T) {
318318
const bucketName = "foo"
319319

320320
t.Run("does_nothing_when_bucket_management_is_disabled", func(t *testing.T) {
321-
t.Parallel()
321+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
322322

323323
svc, _ := testService(t, nil)
324324

@@ -328,7 +328,7 @@ func TestDeleteBucket(t *testing.T) {
328328
})
329329

330330
t.Run("deletes_bucket_with_configured_name", func(t *testing.T) {
331-
t.Parallel()
331+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
332332

333333
svc, s3Mock := testService(t, &infrav1.S3Bucket{
334334
Name: bucketName,
@@ -347,9 +347,8 @@ func TestDeleteBucket(t *testing.T) {
347347
})
348348

349349
t.Run("returns_error_when_bucket_removal_returns", func(t *testing.T) {
350-
t.Parallel()
351350
t.Run("unexpected_error", func(t *testing.T) {
352-
t.Parallel()
351+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
353352

354353
svc, s3Mock := testService(t, &infrav1.S3Bucket{})
355354

@@ -362,7 +361,7 @@ func TestDeleteBucket(t *testing.T) {
362361
})
363362

364363
t.Run("unexpected_AWS_error", func(t *testing.T) {
365-
t.Parallel()
364+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
366365

367366
svc, s3Mock := testService(t, &infrav1.S3Bucket{})
368367

@@ -376,7 +375,7 @@ func TestDeleteBucket(t *testing.T) {
376375
})
377376

378377
t.Run("ignores_when_bucket_has_already_been_removed", func(t *testing.T) {
379-
t.Parallel()
378+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
380379

381380
svc, s3Mock := testService(t, &infrav1.S3Bucket{})
382381

@@ -389,7 +388,7 @@ func TestDeleteBucket(t *testing.T) {
389388
})
390389

391390
t.Run("skips_bucket_removal_when_bucket_is_not_empty", func(t *testing.T) {
392-
t.Parallel()
391+
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)()
393392

394393
svc, s3Mock := testService(t, &infrav1.S3Bucket{})
395394

0 commit comments

Comments
 (0)