Skip to content

Commit 53bd65d

Browse files
authored
chore: requeue after CreateBucket call (#202)
Issue [#2730](aws-controllers-k8s/community#2730) Description of changes: Currently, the s3 controller attempts to sync all its fields right after it calls create. In ACK runtime, we set the resource as unmanaged if we receive an AWS error after create, assuming the resource creation failed. With this change, we ensure sdkCreate is only responsible for creating the bucket, and returning a requeue error, that will trigger an update reconciliation loop, ensuring all bucket fields are synced on update By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 08cafc8 commit 53bd65d

6 files changed

Lines changed: 16 additions & 141 deletions

File tree

apis/v1alpha1/ack-generate-metadata.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
ack_generate_info:
2-
build_date: "2026-01-14T10:41:47Z"
3-
build_hash: e743d683160cf0f58a4864e052cdcb0927335ca7
4-
go_version: go1.25.5
5-
version: v0.57.0
6-
api_directory_checksum: 8a79c5d90ef817140c340896677c51d4d61b1aec
2+
build_date: "2026-02-16T23:44:33Z"
3+
build_hash: 917b9ac49daec562bb6504697d5f1b719c9797b4
4+
go_version: go1.25.4
5+
version: v0.57.0-3-g917b9ac
6+
api_directory_checksum: eacd8486c6745fb88c373c09fd904d2268e5538f
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.32.6
99
generator_config_info:

pkg/resource/bucket/hook.go

Lines changed: 0 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -234,135 +234,6 @@ func (rm *resourceManager) customFindBucket(
234234
return &resource{ko}, nil
235235
}
236236

237-
func (rm *resourceManager) createPutFields(
238-
ctx context.Context,
239-
r *resource,
240-
) error {
241-
isDirectoryBucket := r.ko.Spec.Name != nil && IsDirectoryBucketName(*r.ko.Spec.Name)
242-
243-
// Directory buckets only support: Encryption, Lifecycle, Policy, Tagging
244-
// Skip all other operations for directory buckets
245-
246-
if !isDirectoryBucket {
247-
// Other configuration options (Replication) require versioning to be
248-
// enabled before they can be configured
249-
if r.ko.Spec.Versioning != nil {
250-
if err := rm.syncVersioning(ctx, r); err != nil {
251-
return err
252-
}
253-
}
254-
255-
if r.ko.Spec.Accelerate != nil {
256-
if err := rm.syncAccelerate(ctx, r); err != nil {
257-
return errors.Wrapf(err, ErrSyncingPutProperty, "Accelerate")
258-
}
259-
}
260-
if len(r.ko.Spec.Analytics) != 0 {
261-
if err := rm.syncAnalytics(ctx, r, nil); err != nil {
262-
return errors.Wrapf(err, ErrSyncingPutProperty, "Analytics")
263-
}
264-
}
265-
if r.ko.Spec.CORS != nil {
266-
if err := rm.syncCORS(ctx, r); err != nil {
267-
return errors.Wrapf(err, ErrSyncingPutProperty, "CORS")
268-
}
269-
}
270-
}
271-
272-
// Encryption is supported for both bucket types
273-
if r.ko.Spec.Encryption != nil {
274-
if err := rm.syncEncryption(ctx, r); err != nil {
275-
return errors.Wrapf(err, ErrSyncingPutProperty, "Encryption")
276-
}
277-
}
278-
279-
if !isDirectoryBucket {
280-
if len(r.ko.Spec.IntelligentTiering) != 0 {
281-
if err := rm.syncIntelligentTiering(ctx, r, nil); err != nil {
282-
return errors.Wrapf(err, ErrSyncingPutProperty, "IntelligentTiering")
283-
}
284-
}
285-
if len(r.ko.Spec.Inventory) != 0 {
286-
if err := rm.syncInventory(ctx, r, nil); err != nil {
287-
return errors.Wrapf(err, ErrSyncingPutProperty, "Inventory")
288-
}
289-
}
290-
}
291-
292-
// Lifecycle is supported for both bucket types
293-
if r.ko.Spec.Lifecycle != nil {
294-
if err := rm.syncLifecycle(ctx, r); err != nil {
295-
return errors.Wrapf(err, ErrSyncingPutProperty, "Lifecycle")
296-
}
297-
}
298-
299-
if !isDirectoryBucket {
300-
if r.ko.Spec.Logging != nil {
301-
if err := rm.syncLogging(ctx, r); err != nil {
302-
return errors.Wrapf(err, ErrSyncingPutProperty, "Logging")
303-
}
304-
}
305-
if len(r.ko.Spec.Metrics) != 0 {
306-
if err := rm.syncMetrics(ctx, r, nil); err != nil {
307-
return errors.Wrapf(err, ErrSyncingPutProperty, "Metrics")
308-
}
309-
}
310-
if r.ko.Spec.Notification != nil {
311-
if err := rm.syncNotification(ctx, r); err != nil {
312-
return errors.Wrapf(err, ErrSyncingPutProperty, "Notification")
313-
}
314-
}
315-
if r.ko.Spec.OwnershipControls != nil {
316-
if err := rm.syncOwnershipControls(ctx, r); err != nil {
317-
return errors.Wrapf(err, ErrSyncingPutProperty, "OwnershipControls")
318-
}
319-
}
320-
// PublicAccessBlock may need to be set in order to use Policy, so sync it
321-
// first
322-
if r.ko.Spec.PublicAccessBlock != nil {
323-
if err := rm.syncPublicAccessBlock(ctx, r); err != nil {
324-
return errors.Wrapf(err, ErrSyncingPutProperty, "PublicAccessBlock")
325-
}
326-
}
327-
}
328-
329-
// Policy is supported for both bucket types
330-
if r.ko.Spec.Policy != nil {
331-
if err := rm.syncPolicy(ctx, r, isDirectoryBucket); err != nil {
332-
return errors.Wrapf(err, ErrSyncingPutProperty, "Policy")
333-
}
334-
}
335-
336-
if !isDirectoryBucket {
337-
if r.ko.Spec.Replication != nil {
338-
if err := rm.syncReplication(ctx, r); err != nil {
339-
return errors.Wrapf(err, ErrSyncingPutProperty, "Replication")
340-
}
341-
}
342-
if r.ko.Spec.RequestPayment != nil {
343-
if err := rm.syncRequestPayment(ctx, r); err != nil {
344-
return errors.Wrapf(err, ErrSyncingPutProperty, "RequestPayment")
345-
}
346-
}
347-
}
348-
349-
// Tagging is supported for both bucket types (uses S3 Control API for directory buckets)
350-
if r.ko.Spec.Tagging != nil {
351-
if err := rm.syncTagging(ctx, r); err != nil {
352-
return errors.Wrapf(err, ErrSyncingPutProperty, "Tagging")
353-
}
354-
}
355-
356-
if !isDirectoryBucket {
357-
if r.ko.Spec.Website != nil {
358-
if err := rm.syncWebsite(ctx, r); err != nil {
359-
return errors.Wrapf(err, ErrSyncingPutProperty, "Website")
360-
}
361-
}
362-
}
363-
return nil
364-
}
365-
366237
// customUpdateBucket patches each of the resource properties in the backend AWS
367238
// service API and returns a new resource with updated fields.
368239
func (rm *resourceManager) customUpdateBucket(

pkg/resource/bucket/sdk.go

Lines changed: 5 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
if err := rm.createPutFields(ctx, desired); err != nil {
2-
return nil, err
3-
}
1+
ackcondition.SetSynced(&resource{ko}, corev1.ConditionFalse, aws.String("bucket created, requeue for updates"), nil)
2+
err = ackrequeue.NeededAfter(fmt.Errorf("Reconciling to sync additional fields"), time.Second)
3+
return &resource{ko}, err

test/e2e/tests/test_bucket.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ def delete_bucket(bucket: Bucket):
229229
return
230230

231231
# Delete k8s resource
232-
_, deleted = k8s.delete_custom_resource(bucket.ref)
232+
_, deleted = k8s.delete_custom_resource(bucket.ref, DELETE_WAIT_AFTER_SECONDS)
233233
assert deleted is True
234234

235235
time.sleep(DELETE_WAIT_AFTER_SECONDS)
@@ -240,6 +240,7 @@ def basic_bucket(s3_client) -> Generator[Bucket, None, None]:
240240
try:
241241
bucket = create_bucket("bucket")
242242
assert k8s.get_resource_exists(bucket.ref)
243+
k8s.wait_on_condition(bucket.ref, "ACK.ResourceSynced", "True", wait_periods=5)
243244

244245
# assert bucket ARN is present in status
245246
bucket_k8s = bucket.resource_data = k8s.get_resource(bucket.ref)

test/e2e/tests/test_bucket_adoption_policy.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ def test_adopt_or_create_policy(
173173
tags.assert_ack_system_tags(
174174
tags=tagging.tag_set,
175175
)
176+
time.sleep(5)
176177
tags.assert_equal_without_ack_tags(
177178
expected=initial_tags,
178179
actual=tagging.tag_set,

0 commit comments

Comments
 (0)