Skip to content

Commit c6facdd

Browse files
committed
delete fixes
1 parent 534418a commit c6facdd

File tree

8 files changed

+280
-360
lines changed

8 files changed

+280
-360
lines changed

Diff for: apis/v1alpha1/ack-generate-metadata.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
ack_generate_info:
2-
build_date: "2025-03-23T21:30:39Z"
2+
build_date: "2025-03-24T15:20:40Z"
33
build_hash: 3722729cebe6d3c03c7e442655ef0846f91566a2
44
go_version: go1.24.0
55
version: v0.43.2-7-g3722729

Diff for: pkg/resource/table/hooks.go

-4
Original file line numberDiff line numberDiff line change
@@ -239,10 +239,6 @@ func (rm *resourceManager) customUpdateTable(
239239
rlog.Debug(msg)
240240
return nil, ackerr.NewTerminalError(errors.New(msg))
241241
}
242-
243-
if !canUpdateTableReplicas(latest) {
244-
return nil, requeueWaitReplicasActive
245-
}
246242
if err := rm.syncReplicas(ctx, latest, desired); err != nil {
247243
return nil, err
248244
}

Diff for: pkg/resource/table/hooks_replica_updates.go

+69-23
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@ import (
1818
"strings"
1919
"time"
2020

21-
ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
2221
ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors"
2322
ackrequeue "github.com/aws-controllers-k8s/runtime/pkg/requeue"
23+
ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
2424
"github.com/aws/aws-sdk-go-v2/aws"
2525
svcsdk "github.com/aws/aws-sdk-go-v2/service/dynamodb"
2626
svcsdktypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
2727

2828
"github.com/aws-controllers-k8s/dynamodb-controller/apis/v1alpha1"
29+
svcapitypes "github.com/aws-controllers-k8s/dynamodb-controller/apis/v1alpha1"
2930
)
3031

3132
// equalCreateReplicationGroupMemberActions compares two CreateReplicationGroupMemberAction objects
@@ -262,25 +263,6 @@ func deleteReplicaUpdate(regionName string) svcsdktypes.ReplicationGroupUpdate {
262263
}
263264
}
264265

265-
// canUpdateTableReplicas returns true if it's possible to update table replicas.
266-
// We can only modify replicas when they are in ACTIVE state.
267-
func canUpdateTableReplicas(r *resource) bool {
268-
// Check if Status or Replicas is nil
269-
// needed when called by sdkdelete
270-
if r == nil || r.ko == nil || r.ko.Status.Replicas == nil {
271-
return true // If no replicas exist, we can proceed with updates
272-
}
273-
// Check if any replica is not in ACTIVE state
274-
for _, replicaDesc := range r.ko.Status.Replicas {
275-
if replicaDesc.RegionName != nil && replicaDesc.ReplicaStatus != nil {
276-
if *replicaDesc.ReplicaStatus != string(svcsdktypes.ReplicaStatusActive) {
277-
return false
278-
}
279-
}
280-
}
281-
return true
282-
}
283-
284266
// hasStreamSpecificationWithNewAndOldImages checks if the table has DynamoDB Streams enabled
285267
// with the stream containing both the new and the old images of the item.
286268
func hasStreamSpecificationWithNewAndOldImages(r *resource) bool {
@@ -317,8 +299,8 @@ func (rm *resourceManager) syncReplicas(
317299
// Handle specific errors
318300
if awsErr, ok := ackerr.AWSError(err); ok {
319301
// Handle ValidationException - when replicas are not part of the global table
320-
if awsErr.ErrorCode() == "ValidationException" &&
321-
strings.Contains(awsErr.ErrorMessage(), "not part of the global table") {
302+
if awsErr.ErrorCode() == "ValidationException" &&
303+
strings.Contains(awsErr.ErrorMessage(), "not part of the global table") {
322304
// A replica was already deleted
323305
rlog.Debug("replica already deleted from global table",
324306
"table", *latest.ko.Spec.TableName,
@@ -328,7 +310,7 @@ func (rm *resourceManager) syncReplicas(
328310
30*time.Second,
329311
)
330312
}
331-
313+
332314
// Handle ResourceInUseException - when the table is being updated
333315
if awsErr.ErrorCode() == "ResourceInUseException" {
334316
rlog.Debug("table is currently in use, will retry",
@@ -382,20 +364,29 @@ func (rm *resourceManager) newUpdateTableReplicaUpdatesOneAtATimePayload(
382364

383365
if len(createReplicas) > 0 {
384366
replica := *createReplicas[0]
367+
if checkReplicaStatus(latest.ko.Status.Replicas, *replica.RegionName) {
368+
return nil, 0, requeueWaitReplicasActive
369+
}
385370
rlog.Debug("creating replica in region", "table", *desired.ko.Spec.TableName, "region", *replica.RegionName)
386371
input.ReplicaUpdates = append(input.ReplicaUpdates, createReplicaUpdate(createReplicas[0]))
387372
return input, replicasInQueue, nil
388373
}
389374

390375
if len(updateReplicas) > 0 {
391376
replica := *updateReplicas[0]
377+
if checkReplicaStatus(latest.ko.Status.Replicas, *replica.RegionName) {
378+
return nil, 0, requeueWaitReplicasActive
379+
}
392380
rlog.Debug("updating replica in region", "table", *desired.ko.Spec.TableName, "region", *replica.RegionName)
393381
input.ReplicaUpdates = append(input.ReplicaUpdates, updateReplicaUpdate(updateReplicas[0]))
394382
return input, replicasInQueue, nil
395383
}
396384

397385
if len(deleteRegions) > 0 {
398386
replica := deleteRegions[0]
387+
if checkReplicaStatus(latest.ko.Status.Replicas, replica) {
388+
return nil, 0, requeueWaitReplicasActive
389+
}
399390
rlog.Debug("deleting replica in region", "table", *desired.ko.Spec.TableName, "region", replica)
400391
input.ReplicaUpdates = append(input.ReplicaUpdates, deleteReplicaUpdate(deleteRegions[0]))
401392
return input, replicasInQueue, nil
@@ -451,3 +442,58 @@ func calculateReplicaUpdates(
451442

452443
return createReplicas, updateReplicas, deleteRegions
453444
}
445+
446+
func setTableReplicas(ko *svcapitypes.Table, replicas []svcsdktypes.ReplicaDescription) {
447+
if len(replicas) > 0 {
448+
tableReplicas := []*v1alpha1.CreateReplicationGroupMemberAction{}
449+
for _, replica := range replicas {
450+
replicaElem := &v1alpha1.CreateReplicationGroupMemberAction{}
451+
if replica.RegionName != nil {
452+
replicaElem.RegionName = replica.RegionName
453+
}
454+
if replica.KMSMasterKeyId != nil {
455+
replicaElem.KMSMasterKeyID = replica.KMSMasterKeyId
456+
}
457+
if replica.ProvisionedThroughputOverride != nil {
458+
replicaElem.ProvisionedThroughputOverride = &v1alpha1.ProvisionedThroughputOverride{
459+
ReadCapacityUnits: replica.ProvisionedThroughputOverride.ReadCapacityUnits,
460+
}
461+
}
462+
if replica.GlobalSecondaryIndexes != nil {
463+
gsiList := []*v1alpha1.ReplicaGlobalSecondaryIndex{}
464+
for _, gsi := range replica.GlobalSecondaryIndexes {
465+
gsiElem := &v1alpha1.ReplicaGlobalSecondaryIndex{
466+
IndexName: gsi.IndexName,
467+
}
468+
if gsi.ProvisionedThroughputOverride != nil {
469+
gsiElem.ProvisionedThroughputOverride = &v1alpha1.ProvisionedThroughputOverride{
470+
ReadCapacityUnits: gsi.ProvisionedThroughputOverride.ReadCapacityUnits,
471+
}
472+
}
473+
gsiList = append(gsiList, gsiElem)
474+
}
475+
replicaElem.GlobalSecondaryIndexes = gsiList
476+
}
477+
if replica.ReplicaTableClassSummary != nil && replica.ReplicaTableClassSummary.TableClass != "" {
478+
replicaElem.TableClassOverride = aws.String(string(replica.ReplicaTableClassSummary.TableClass))
479+
}
480+
tableReplicas = append(tableReplicas, replicaElem)
481+
}
482+
ko.Spec.TableReplicas = tableReplicas
483+
} else {
484+
ko.Spec.TableReplicas = nil
485+
}
486+
}
487+
488+
func checkReplicaStatus(ReplicaDescription []*svcapitypes.ReplicaDescription, regionName string) bool {
489+
for _, replica := range ReplicaDescription {
490+
if *replica.RegionName == regionName {
491+
replicaStatus := replica.ReplicaStatus
492+
if *replicaStatus == string(svcsdktypes.ReplicaStatusCreating) || *replicaStatus == string(svcsdktypes.ReplicaStatusDeleting) || *replicaStatus == string(svcsdktypes.ReplicaStatusUpdating) {
493+
return true
494+
}
495+
}
496+
}
497+
498+
return false
499+
}

Diff for: pkg/resource/table/sdk.go

+1-42
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: templates/hooks/table/sdk_delete_pre_build_request.go.tpl

-3
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@
66
}
77

88
// If there are replicas, we need to remove them before deleting the table
9-
if !canUpdateTableReplicas(latest) {
10-
return nil, requeueWaitReplicasActive
11-
}
129
if len(r.ko.Spec.TableReplicas) > 0 {
1310
desired := &resource{
1411
ko: r.ko.DeepCopy(),

Diff for: templates/hooks/table/sdk_read_one_post_set_output.go.tpl

+1-39
Original file line numberDiff line numberDiff line change
@@ -53,45 +53,7 @@
5353
} else {
5454
ko.Spec.BillingMode = aws.String("PROVISIONED")
5555
}
56-
if len(resp.Table.Replicas) > 0 {
57-
tableReplicas := []*svcapitypes.CreateReplicationGroupMemberAction{}
58-
for _, replica := range resp.Table.Replicas {
59-
replicaElem := &svcapitypes.CreateReplicationGroupMemberAction{}
60-
if replica.RegionName != nil {
61-
replicaElem.RegionName = replica.RegionName
62-
}
63-
if replica.KMSMasterKeyId != nil {
64-
replicaElem.KMSMasterKeyID = replica.KMSMasterKeyId
65-
}
66-
if replica.ProvisionedThroughputOverride != nil {
67-
replicaElem.ProvisionedThroughputOverride = &svcapitypes.ProvisionedThroughputOverride{
68-
ReadCapacityUnits: replica.ProvisionedThroughputOverride.ReadCapacityUnits,
69-
}
70-
}
71-
if replica.GlobalSecondaryIndexes != nil {
72-
gsiList := []*svcapitypes.ReplicaGlobalSecondaryIndex{}
73-
for _, gsi := range replica.GlobalSecondaryIndexes {
74-
gsiElem := &svcapitypes.ReplicaGlobalSecondaryIndex{
75-
IndexName: gsi.IndexName,
76-
}
77-
if gsi.ProvisionedThroughputOverride != nil {
78-
gsiElem.ProvisionedThroughputOverride = &svcapitypes.ProvisionedThroughputOverride{
79-
ReadCapacityUnits: gsi.ProvisionedThroughputOverride.ReadCapacityUnits,
80-
}
81-
}
82-
gsiList = append(gsiList, gsiElem)
83-
}
84-
replicaElem.GlobalSecondaryIndexes = gsiList
85-
}
86-
if replica.ReplicaTableClassSummary != nil && replica.ReplicaTableClassSummary.TableClass != "" {
87-
replicaElem.TableClassOverride = aws.String(string(replica.ReplicaTableClassSummary.TableClass))
88-
}
89-
tableReplicas = append(tableReplicas, replicaElem)
90-
}
91-
ko.Spec.TableReplicas = tableReplicas
92-
} else {
93-
ko.Spec.TableReplicas = nil
94-
}
56+
setTableReplicas(r, resp.Table.Replicas)
9557
if isTableCreating(&resource{ko}) {
9658
return &resource{ko}, requeueWaitWhileCreating
9759
}

Diff for: test/e2e/resources/table_with_gsi_and_replicas.yaml

+6-10
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ spec:
1414
attributeType: S
1515
- attributeName: GSI1SK
1616
attributeType: S
17-
- attributeName: GSI2PK
18-
attributeType: S
1917
keySchema:
2018
- attributeName: PK
2119
keyType: HASH
@@ -25,9 +23,13 @@ spec:
2523
streamSpecification:
2624
streamEnabled: true
2725
streamViewType: "NEW_AND_OLD_IMAGES"
28-
replicationGroup:
26+
tableReplicas:
2927
- regionName: $REPLICA_REGION_1
28+
globalSecondaryIndexes:
29+
- indexName: GSI1
3030
- regionName: $REPLICA_REGION_2
31+
globalSecondaryIndexes:
32+
- indexName: GSI1
3133
globalSecondaryIndexes:
3234
- indexName: GSI1
3335
keySchema:
@@ -36,10 +38,4 @@ spec:
3638
- attributeName: GSI1SK
3739
keyType: RANGE
3840
projection:
39-
projectionType: ALL
40-
- indexName: GSI2
41-
keySchema:
42-
- attributeName: GSI2PK
43-
keyType: HASH
44-
projection:
45-
projectionType: KEYS_ONLY
41+
projectionType: ALL

0 commit comments

Comments
 (0)