Skip to content

Commit f42a7c9

Browse files
rushmash91rushmash91
committed
add tests
1 parent 3cc1a09 commit f42a7c9

19 files changed

+674
-199
lines changed

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

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
ack_generate_info:
2-
build_date: "2025-03-12T23:25:20Z"
3-
build_hash: 5645e51ed3c413f616e1b929f1527a5139c44198
2+
build_date: "2025-03-21T05:42:15Z"
3+
build_hash: 3722729cebe6d3c03c7e442655ef0846f91566a2
44
go_version: go1.24.0
5-
version: v0.43.2-3-g5645e51
6-
api_directory_checksum: 0d5b95bdbe63c6cfc495149b7a86440c4a5fb33a
5+
version: v0.43.2-7-g3722729
6+
api_directory_checksum: a3ce13b1988591541d32ac56e55b416ff3b6bfc2
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.32.6
99
generator_config_info:
10-
file_checksum: 48323d41b9cb7ade08c8a87367846cccdd1f6409
10+
file_checksum: c8721c6d81ac66a3ea6c2b96b784c3da48b798b5
1111
original_file_name: generator.yaml
1212
last_modification:
1313
reason: API generation

Diff for: ‎apis/v1alpha1/generator.yaml

+3-6
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,11 @@ operations:
2525
resources:
2626
Table:
2727
fields:
28-
Replicas:
28+
ReplicationGroup:
2929
custom_field:
3030
list_of: CreateReplicationGroupMemberAction
3131
compare:
3232
is_ignored: true
33-
ReplicasDescriptions:
34-
custom_field:
35-
list_of: ReplicaDescription
36-
is_read_only: true
3733
GlobalSecondaryIndexesDescriptions:
3834
custom_field:
3935
list_of: GlobalSecondaryIndexDescription
@@ -73,13 +69,14 @@ resources:
7369
references:
7470
service_name: kms
7571
resource: Key
76-
path: Status.ACKResourceMetadata.ARN
72+
path: Status.ACKResourceMetadata.ARN
7773
exceptions:
7874
errors:
7975
404:
8076
code: ResourceNotFoundException
8177
terminal_codes:
8278
- InvalidParameter
79+
- ValidationException
8380
update_operation:
8481
custom_method_name: customUpdateTable
8582
hooks:

Diff for: ‎apis/v1alpha1/table.go

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

Diff for: ‎apis/v1alpha1/zz_generated.deepcopy.go

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

Diff for: ‎config/crd/bases/dynamodb.services.k8s.aws_tables.yaml

+3-2
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ spec:
322322
format: int64
323323
type: integer
324324
type: object
325-
replicas:
325+
replicationGroup:
326326
items:
327327
description: Represents a replica to be created.
328328
properties:
@@ -644,7 +644,8 @@ spec:
644644
645645
* StreamLabel
646646
type: string
647-
replicasDescriptions:
647+
replicas:
648+
description: Represents replicas of the table.
648649
items:
649650
description: Contains the details of the replica.
650651
properties:

Diff for: ‎generator.yaml

+3-6
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,11 @@ operations:
2525
resources:
2626
Table:
2727
fields:
28-
Replicas:
28+
ReplicationGroup:
2929
custom_field:
3030
list_of: CreateReplicationGroupMemberAction
3131
compare:
3232
is_ignored: true
33-
ReplicasDescriptions:
34-
custom_field:
35-
list_of: ReplicaDescription
36-
is_read_only: true
3733
GlobalSecondaryIndexesDescriptions:
3834
custom_field:
3935
list_of: GlobalSecondaryIndexDescription
@@ -73,13 +69,14 @@ resources:
7369
references:
7470
service_name: kms
7571
resource: Key
76-
path: Status.ACKResourceMetadata.ARN
72+
path: Status.ACKResourceMetadata.ARN
7773
exceptions:
7874
errors:
7975
404:
8076
code: ResourceNotFoundException
8177
terminal_codes:
8278
- InvalidParameter
79+
- ValidationException
8380
update_operation:
8481
custom_method_name: customUpdateTable
8582
hooks:

Diff for: ‎helm/crds/dynamodb.services.k8s.aws_tables.yaml

+3-2
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ spec:
326326
format: int64
327327
type: integer
328328
type: object
329-
replicas:
329+
replicationGroup:
330330
items:
331331
description: Represents a replica to be created.
332332
properties:
@@ -648,7 +648,8 @@ spec:
648648
649649
* StreamLabel
650650
type: string
651-
replicasDescriptions:
651+
replicas:
652+
description: Represents replicas of the table.
652653
items:
653654
description: Contains the details of the replica.
654655
properties:

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

+23-17
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ package table
1515

1616
import (
1717
"context"
18+
"errors"
1819
"fmt"
1920
"strings"
2021
"time"
@@ -49,9 +50,9 @@ var (
4950
"table GSIs in '%v' state, cannot be modified or deleted",
5051
svcsdktypes.IndexStatusCreating,
5152
)
52-
ErrReplicaNotActive = fmt.Errorf(
53-
"table replica in NOT '%v' state, cannot be modified or deleted",
54-
svcsdktypes.ReplicaStatusActive,
53+
ErrTableReplicasUpdating = fmt.Errorf(
54+
"table replica in '%v' state, cannot be modified or deleted",
55+
svcsdktypes.ReplicaStatusUpdating,
5556
)
5657
)
5758

@@ -78,14 +79,14 @@ var (
7879
)
7980
requeueWaitWhileUpdating = ackrequeue.NeededAfter(
8081
ErrTableUpdating,
81-
5*time.Second,
82+
10*time.Second,
8283
)
8384
requeueWaitGSIReady = ackrequeue.NeededAfter(
8485
ErrTableGSIsUpdating,
8586
10*time.Second,
8687
)
87-
requeueWaitForReplicasActive = ackrequeue.NeededAfter(
88-
ErrReplicaNotActive,
88+
requeueWaitReplicasActive = ackrequeue.NeededAfter(
89+
ErrTableReplicasUpdating,
8990
10*time.Second,
9091
)
9192
)
@@ -232,7 +233,16 @@ func (rm *resourceManager) customUpdateTable(
232233
}
233234
return nil, err
234235
}
235-
case delta.DifferentAt("Spec.Replicas"):
236+
case delta.DifferentAt("Spec.ReplicationGroup"):
237+
if !hasStreamSpecificationWithNewAndOldImages(desired) {
238+
msg := "table must have DynamoDB Streams enabled with StreamViewType set to NEW_AND_OLD_IMAGES for replica updates"
239+
rlog.Debug(msg)
240+
return nil, ackerr.NewTerminalError(errors.New(msg))
241+
}
242+
243+
if !canUpdateTableReplicas(latest) {
244+
return nil, requeueWaitReplicasActive
245+
}
236246
if err := rm.syncReplicaUpdates(ctx, latest, desired); err != nil {
237247
return nil, err
238248
}
@@ -274,10 +284,6 @@ func (rm *resourceManager) newUpdateTablePayload(
274284
input := &svcsdk.UpdateTableInput{
275285
TableName: aws.String(*r.ko.Spec.TableName),
276286
}
277-
// If delta is nil, we're just creating a basic payload for other operations to modify
278-
if delta == nil {
279-
return input, nil
280-
}
281287

282288
if delta.DifferentAt("Spec.BillingMode") {
283289
if r.ko.Spec.BillingMode != nil {
@@ -574,12 +580,12 @@ func customPreCompare(
574580
}
575581
}
576582

577-
// Handle ReplicaUpdates comparison
578-
if len(a.ko.Spec.Replicas) != len(b.ko.Spec.Replicas) {
579-
delta.Add("Spec.Replicas", a.ko.Spec.Replicas, b.ko.Spec.Replicas)
580-
} else if a.ko.Spec.Replicas != nil && b.ko.Spec.Replicas != nil {
581-
if !equalReplicaArrays(a.ko.Spec.Replicas, b.ko.Spec.Replicas) {
582-
delta.Add("Spec.Replicas", a.ko.Spec.Replicas, b.ko.Spec.Replicas)
583+
// Handle ReplicaUpdates API comparison
584+
if len(a.ko.Spec.ReplicationGroup) != len(b.ko.Spec.ReplicationGroup) {
585+
delta.Add("Spec.ReplicationGroup", a.ko.Spec.ReplicationGroup, b.ko.Spec.ReplicationGroup)
586+
} else if a.ko.Spec.ReplicationGroup != nil && b.ko.Spec.ReplicationGroup != nil {
587+
if !equalReplicaArrays(a.ko.Spec.ReplicationGroup, b.ko.Spec.ReplicationGroup) {
588+
delta.Add("Spec.ReplicationGroup", a.ko.Spec.ReplicationGroup, b.ko.Spec.ReplicationGroup)
583589
}
584590
}
585591

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

+29-39
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,27 @@
1+
// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License"). You may
4+
// not use this file except in compliance with the License. A copy of the
5+
// License is located at
6+
//
7+
// http://aws.amazon.com/apache2.0/
8+
//
9+
// or in the "license" file accompanying this file. This file is distributed
10+
// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
11+
// express or implied. See the License for the specific language governing
12+
// permissions and limitations under the License.
13+
114
package table
215

316
import (
417
"context"
5-
"errors"
618

7-
ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors"
819
ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
9-
1020
"github.com/aws/aws-sdk-go-v2/aws"
11-
"github.com/aws-controllers-k8s/dynamodb-controller/apis/v1alpha1"
1221
svcsdk "github.com/aws/aws-sdk-go-v2/service/dynamodb"
1322
svcsdktypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
23+
24+
"github.com/aws-controllers-k8s/dynamodb-controller/apis/v1alpha1"
1425
)
1526

1627
// equalCreateReplicationGroupMemberActions compares two CreateReplicationGroupMemberAction objects
@@ -238,17 +249,11 @@ func deleteReplicaUpdate(regionName string) svcsdktypes.ReplicationGroupUpdate {
238249
// canUpdateTableReplicas returns true if it's possible to update table replicas.
239250
// We can only modify replicas when they are in ACTIVE state.
240251
func canUpdateTableReplicas(r *resource) bool {
241-
if isTableCreating(r) || isTableDeleting(r) || isTableUpdating(r) {
242-
return false
243-
}
244-
245252
// Check if any replica is not in ACTIVE state
246-
if r.ko.Status.ReplicasDescriptions != nil {
247-
for _, replicaDesc := range r.ko.Status.ReplicasDescriptions {
248-
if replicaDesc.RegionName != nil && replicaDesc.ReplicaStatus != nil {
249-
if *replicaDesc.ReplicaStatus != string(svcsdktypes.ReplicaStatusActive) {
250-
return false
251-
}
253+
for _, replicaDesc := range r.ko.Status.Replicas {
254+
if replicaDesc.RegionName != nil && replicaDesc.ReplicaStatus != nil {
255+
if *replicaDesc.ReplicaStatus != string(svcsdktypes.ReplicaStatusActive) {
256+
return false
252257
}
253258
}
254259
}
@@ -275,32 +280,15 @@ func (rm *resourceManager) syncReplicaUpdates(
275280
) (err error) {
276281
rlog := ackrtlog.FromContext(ctx)
277282
exit := rlog.Trace("rm.syncReplicaUpdates")
278-
defer exit(err)
279-
280-
if !hasStreamSpecificationWithNewAndOldImages(desired) {
281-
msg := "table must have DynamoDB Streams enabled with StreamViewType set to NEW_AND_OLD_IMAGES for replica updates"
282-
rlog.Debug(msg)
283-
return ackerr.NewTerminalError(errors.New(msg))
284-
}
285-
286-
if !canUpdateTableReplicas(latest) {
287-
return requeueWaitForReplicasActive
288-
}
289-
290-
if isTableUpdating(latest) {
291-
return requeueWaitWhileUpdating
292-
}
283+
defer func() {
284+
exit(err)
285+
}()
293286

294287
input, replicasInQueue, err := rm.newUpdateTableReplicaUpdatesOneAtATimePayload(ctx, latest, desired)
295288
if err != nil {
296289
return err
297290
}
298291

299-
// If there are no updates to make, we don't need to requeue
300-
if len(input.ReplicaUpdates) == 0 {
301-
return nil
302-
}
303-
304292
// Call the UpdateTable API
305293
_, err = rm.sdkapi.UpdateTable(ctx, input)
306294
rm.metrics.RecordAPICall("UPDATE", "UpdateTable", err)
@@ -328,7 +316,9 @@ func (rm *resourceManager) newUpdateTableReplicaUpdatesOneAtATimePayload(
328316
) (input *svcsdk.UpdateTableInput, replicasInQueue int, err error) {
329317
rlog := ackrtlog.FromContext(ctx)
330318
exit := rlog.Trace("rm.newUpdateTableReplicaUpdatesOneAtATimePayload")
331-
defer exit(err)
319+
defer func() {
320+
exit(err)
321+
}()
332322

333323
createReplicas, updateReplicas, deleteRegions := calculateReplicaUpdates(latest, desired)
334324

@@ -378,17 +368,17 @@ func calculateReplicaUpdates(
378368
deleteRegions []string,
379369
) {
380370
existingRegions := make(map[string]*v1alpha1.CreateReplicationGroupMemberAction)
381-
if latest != nil && latest.ko.Spec.Replicas != nil {
382-
for _, replica := range latest.ko.Spec.Replicas {
371+
if latest != nil && latest.ko.Spec.ReplicationGroup != nil {
372+
for _, replica := range latest.ko.Spec.ReplicationGroup {
383373
if replica.RegionName != nil {
384374
existingRegions[*replica.RegionName] = replica
385375
}
386376
}
387377
}
388378

389379
desiredRegions := make(map[string]*v1alpha1.CreateReplicationGroupMemberAction)
390-
if desired != nil && desired.ko.Spec.Replicas != nil {
391-
for _, replica := range desired.ko.Spec.Replicas {
380+
if desired != nil && desired.ko.Spec.ReplicationGroup != nil {
381+
for _, replica := range desired.ko.Spec.ReplicationGroup {
392382
if replica.RegionName != nil {
393383
desiredRegions[*replica.RegionName] = replica
394384
}

0 commit comments

Comments
 (0)