Skip to content

Commit 58226da

Browse files
Julian-ChuJulian-Chu
authored
Feat/support tag motification (#116)
Issue: [[Elasticache] tags not modified](aws-controllers-k8s/community#1738) Description of changes: - add creating status (add message and set ACK.ResourceSynced to false when replication group is being created) ```yaml - status: "False" type: ACK.ResourceSynced message: "replication group currently being created." ``` - add syncTags and getTags - now sdkFind updates ko.Spec.Tags(latest status) - add E2E test case some comments are copied from RDS controller
1 parent 9fd7f95 commit 58226da

File tree

16 files changed

+546
-30
lines changed

16 files changed

+546
-30
lines changed

‎apis/v1alpha1/ack-generate-metadata.yaml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
ack_generate_info:
2-
build_date: "2023-03-22T22:30:54Z"
3-
build_hash: fa24753ea8b657d8815ae3eac7accd0958f5f9fb
4-
go_version: go1.19
5-
version: v0.25.0
2+
build_date: "2023-04-12T12:29:27Z"
3+
build_hash: a6ae2078e57187b2daf47978bc07bd67072d2cba
4+
go_version: go1.19.2
5+
version: v0.25.0-1-ga6ae207
66
api_directory_checksum: 885f952f7ca2ce7a676b9bbf8eb262de71de6238
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.44.93
99
generator_config_info:
10-
file_checksum: 8b1f9e0bb52e26722d71cbd1ee3d489a0e3970c6
10+
file_checksum: bd93202f4d05393bb2ff98344e8e603e3228cc51
1111
original_file_name: generator.yaml
1212
last_modification:
1313
reason: API generation

‎apis/v1alpha1/generator.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ resources:
7777
template_path: hooks/replication_group/sdk_delete_pre_build_request.go.tpl
7878
sdk_delete_post_request:
7979
template_path: hooks/replication_group/sdk_delete_post_request.go.tpl
80+
sdk_update_pre_build_request:
81+
template_path: hooks/replication_group/sdk_update_pre_build_request.go.tpl
8082
sdk_update_post_build_request:
8183
template_path: hooks/replication_group/sdk_update_post_build_request.go.tpl
8284
delta_post_compare:

‎generator.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ resources:
7777
template_path: hooks/replication_group/sdk_delete_pre_build_request.go.tpl
7878
sdk_delete_post_request:
7979
template_path: hooks/replication_group/sdk_delete_post_request.go.tpl
80+
sdk_update_pre_build_request:
81+
template_path: hooks/replication_group/sdk_update_pre_build_request.go.tpl
8082
sdk_update_post_build_request:
8183
template_path: hooks/replication_group/sdk_update_post_build_request.go.tpl
8284
delta_post_compare:

‎pkg/resource/replication_group/hooks.go‎

Lines changed: 147 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,31 @@
1414
package replication_group
1515

1616
import (
17+
"context"
1718
"errors"
1819

20+
ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
1921
ackrequeue "github.com/aws-controllers-k8s/runtime/pkg/requeue"
22+
ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
23+
svcsdk "github.com/aws/aws-sdk-go/service/elasticache"
24+
25+
svcapitypes "github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1"
2026
)
2127

2228
var (
29+
condMsgCurrentlyCreating string = "replication group currently being created."
2330
condMsgCurrentlyDeleting string = "replication group currently being deleted."
2431
condMsgNoDeleteWhileModifying string = "replication group currently being modified. cannot delete."
2532
condMsgTerminalCreateFailed string = "replication group in create-failed status."
2633
)
2734

35+
const (
36+
statusDeleting string = "deleting"
37+
statusModifying string = "modifying"
38+
statusCreating string = "creating"
39+
statusCreateFailed string = "create-failed"
40+
)
41+
2842
var (
2943
requeueWaitWhileDeleting = ackrequeue.NeededAfter(
3044
errors.New("Delete is in progress."),
@@ -34,6 +48,10 @@ var (
3448
errors.New("Modify is in progress."),
3549
ackrequeue.DefaultRequeueAfterDuration,
3650
)
51+
requeueWaitWhileTagUpdated = ackrequeue.NeededAfter(
52+
errors.New("tags Update is in progress"),
53+
ackrequeue.DefaultRequeueAfterDuration,
54+
)
3755
)
3856

3957
// isDeleting returns true if supplied replication group resource state is 'deleting'
@@ -42,7 +60,7 @@ func isDeleting(r *resource) bool {
4260
return false
4361
}
4462
status := *r.ko.Status.Status
45-
return status == "deleting"
63+
return status == statusDeleting
4664
}
4765

4866
// isModifying returns true if supplied replication group resource state is 'modifying'
@@ -51,7 +69,16 @@ func isModifying(r *resource) bool {
5169
return false
5270
}
5371
status := *r.ko.Status.Status
54-
return status == "modifying"
72+
return status == statusModifying
73+
}
74+
75+
// isCreating returns true if supplied replication group resource state is 'modifying'
76+
func isCreating(r *resource) bool {
77+
if r == nil || r.ko.Status.Status == nil {
78+
return false
79+
}
80+
status := *r.ko.Status.Status
81+
return status == statusCreating
5582
}
5683

5784
// isCreateFailed returns true if supplied replication group resource state is
@@ -61,5 +88,122 @@ func isCreateFailed(r *resource) bool {
6188
return false
6289
}
6390
status := *r.ko.Status.Status
64-
return status == "create-failed"
91+
return status == statusCreateFailed
92+
}
93+
94+
// getTags retrieves the resource's associated tags
95+
func (rm *resourceManager) getTags(
96+
ctx context.Context,
97+
resourceARN string,
98+
) ([]*svcapitypes.Tag, error) {
99+
resp, err := rm.sdkapi.ListTagsForResourceWithContext(
100+
ctx,
101+
&svcsdk.ListTagsForResourceInput{
102+
ResourceName: &resourceARN,
103+
},
104+
)
105+
rm.metrics.RecordAPICall("GET", "ListTagsForResource", err)
106+
if err != nil {
107+
return nil, err
108+
}
109+
tags := make([]*svcapitypes.Tag, 0, len(resp.TagList))
110+
for _, tag := range resp.TagList {
111+
tags = append(tags, &svcapitypes.Tag{
112+
Key: tag.Key,
113+
Value: tag.Value,
114+
})
115+
}
116+
return tags, nil
117+
}
118+
119+
// syncTags keeps the resource's tags in sync
120+
//
121+
// NOTE(jaypipes): Elasticache's Tagging APIs differ from other AWS APIs in the
122+
// following ways:
123+
//
124+
// 1. The names of the tagging API operations are different. Other APIs use the
125+
// Tagris `ListTagsForResource`, `TagResource` and `UntagResource` API
126+
// calls. RDS uses `ListTagsForResource`, `AddTagsToResource` and
127+
// `RemoveTagsFromResource`.
128+
//
129+
// 2. Even though the name of the `ListTagsForResource` API call is the same,
130+
// the structure of the input and the output are different from other APIs.
131+
// For the input, instead of a `ResourceArn` field, Elasticache names the field
132+
// `ResourceName`, but actually expects an ARN, not the replication group
133+
// name. This is the same for the `AddTagsToResource` and
134+
// `RemoveTagsFromResource` input shapes. For the output shape, the field is
135+
// called `TagList` instead of `Tags` but is otherwise the same struct with
136+
// a `Key` and `Value` member field.
137+
func (rm *resourceManager) syncTags(
138+
ctx context.Context,
139+
desired *resource,
140+
latest *resource,
141+
) (err error) {
142+
rlog := ackrtlog.FromContext(ctx)
143+
exit := rlog.Trace("rm.syncTags")
144+
defer func() { exit(err) }()
145+
146+
arn := (*string)(latest.ko.Status.ACKResourceMetadata.ARN)
147+
148+
from := ToACKTags(latest.ko.Spec.Tags)
149+
to := ToACKTags(desired.ko.Spec.Tags)
150+
151+
added, _, removed := ackcompare.GetTagsDifference(from, to)
152+
153+
// NOTE(jaypipes): According to the elasticache API documentation, adding a tag
154+
// with a new value overwrites any existing tag with the same key. So, we
155+
// don't need to do anything to "update" a Tag. Simply including it in the
156+
// AddTagsToResource call is enough.
157+
for key := range removed {
158+
if _, ok := added[key]; ok {
159+
delete(removed, key)
160+
}
161+
}
162+
163+
// Modify tags causing the replication group to be modified and become unavailable temporarily
164+
// so after adding or removing tags, we have to wait for the replication group to be available again
165+
// process: add tags -> requeue -> remove tags -> requeue -> other update
166+
if len(added) > 0 {
167+
toAdd := make([]*svcsdk.Tag, 0, len(added))
168+
for key, val := range added {
169+
key, val := key, val
170+
toAdd = append(toAdd, &svcsdk.Tag{
171+
Key: &key,
172+
Value: &val,
173+
})
174+
}
175+
176+
rlog.Debug("adding tags to replication group", "tags", added)
177+
_, err = rm.sdkapi.AddTagsToResourceWithContext(
178+
ctx,
179+
&svcsdk.AddTagsToResourceInput{
180+
ResourceName: arn,
181+
Tags: toAdd,
182+
},
183+
)
184+
rm.metrics.RecordAPICall("UPDATE", "AddTagsToResource", err)
185+
if err != nil {
186+
return err
187+
}
188+
} else if len(removed) > 0 {
189+
toRemove := make([]*string, 0, len(removed))
190+
for key := range removed {
191+
key := key
192+
toRemove = append(toRemove, &key)
193+
}
194+
rlog.Debug("removing tags from replication group", "tags", removed)
195+
_, err = rm.sdkapi.RemoveTagsFromResourceWithContext(
196+
ctx,
197+
&svcsdk.RemoveTagsFromResourceInput{
198+
ResourceName: arn,
199+
TagKeys: toRemove,
200+
},
201+
)
202+
rm.metrics.RecordAPICall("UPDATE", "RemoveTagsFromResource", err)
203+
if err != nil {
204+
return err
205+
}
206+
}
207+
208+
return requeueWaitWhileTagUpdated
65209
}

‎pkg/resource/replication_group/hooks_test.go‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
package replication_group
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
8+
"github.com/aws/aws-sdk-go/aws"
9+
"github.com/stretchr/testify/mock"
10+
11+
svcapitypes "github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1"
12+
mocksvcsdkapi "github.com/aws-controllers-k8s/elasticache-controller/mocks/aws-sdk-go/elasticache"
13+
)
14+
15+
func Test_resourceManager_syncTags(t *testing.T) {
16+
testhelper := func() (*resourceManager, *mocksvcsdkapi.ElastiCacheAPI) {
17+
mocksdkapi := &mocksvcsdkapi.ElastiCacheAPI{}
18+
mocksdkapi.On("RemoveTagsFromResourceWithContext", mock.Anything, mock.Anything).Return(nil, nil)
19+
mocksdkapi.On("AddTagsToResourceWithContext", mock.Anything, mock.Anything).Return(nil, nil)
20+
rm := provideResourceManagerWithMockSDKAPI(mocksdkapi)
21+
return rm, mocksdkapi
22+
}
23+
t.Run("add and remove tags, only execute add tags", func(t *testing.T) {
24+
rm, mocksdkapi := testhelper()
25+
_ = rm.syncTags(context.Background(),
26+
&resource{ko: &svcapitypes.ReplicationGroup{
27+
Spec: svcapitypes.ReplicationGroupSpec{
28+
Tags: []*svcapitypes.Tag{
29+
{
30+
Key: aws.String("add 1"),
31+
Value: aws.String("to add"),
32+
},
33+
{
34+
Key: aws.String("add 2"),
35+
Value: aws.String("to add"),
36+
},
37+
},
38+
},
39+
}},
40+
&resource{ko: &svcapitypes.ReplicationGroup{
41+
Spec: svcapitypes.ReplicationGroupSpec{
42+
Tags: []*svcapitypes.Tag{
43+
{
44+
Key: aws.String("remove"),
45+
Value: aws.String("to remove"),
46+
},
47+
},
48+
},
49+
Status: svcapitypes.ReplicationGroupStatus{
50+
ACKResourceMetadata: &ackv1alpha1.ResourceMetadata{
51+
ARN: (*ackv1alpha1.AWSResourceName)(aws.String("testARN")),
52+
},
53+
},
54+
}})
55+
mocksdkapi.AssertNumberOfCalls(t, "RemoveTagsFromResourceWithContext", 0)
56+
mocksdkapi.AssertNumberOfCalls(t, "AddTagsToResourceWithContext", 1)
57+
})
58+
59+
t.Run("remove tags", func(t *testing.T) {
60+
rm, mocksdkapi := testhelper()
61+
_ = rm.syncTags(context.Background(),
62+
&resource{ko: &svcapitypes.ReplicationGroup{
63+
Spec: svcapitypes.ReplicationGroupSpec{
64+
Tags: []*svcapitypes.Tag{},
65+
},
66+
}},
67+
&resource{ko: &svcapitypes.ReplicationGroup{
68+
Spec: svcapitypes.ReplicationGroupSpec{
69+
Tags: []*svcapitypes.Tag{
70+
{
71+
Key: aws.String("remove 1"),
72+
Value: aws.String("to remove"),
73+
},
74+
{
75+
Key: aws.String("remove 2"),
76+
Value: aws.String("to remove"),
77+
},
78+
},
79+
},
80+
Status: svcapitypes.ReplicationGroupStatus{
81+
ACKResourceMetadata: &ackv1alpha1.ResourceMetadata{
82+
ARN: (*ackv1alpha1.AWSResourceName)(aws.String("testARN")),
83+
},
84+
},
85+
}})
86+
mocksdkapi.AssertNumberOfCalls(t, "RemoveTagsFromResourceWithContext", 1)
87+
mocksdkapi.AssertNumberOfCalls(t, "AddTagsToResourceWithContext", 0)
88+
})
89+
90+
t.Run("modify existent tags, not remove call", func(t *testing.T) {
91+
rm, mocksdkapi := testhelper()
92+
_ = rm.syncTags(context.Background(),
93+
&resource{ko: &svcapitypes.ReplicationGroup{
94+
Spec: svcapitypes.ReplicationGroupSpec{
95+
Tags: []*svcapitypes.Tag{
96+
{
97+
Key: aws.String("key1"),
98+
Value: aws.String("new value1"),
99+
},
100+
{
101+
Key: aws.String("key2"),
102+
Value: aws.String("new value2"),
103+
},
104+
},
105+
},
106+
}},
107+
&resource{ko: &svcapitypes.ReplicationGroup{
108+
Spec: svcapitypes.ReplicationGroupSpec{
109+
Tags: []*svcapitypes.Tag{
110+
{
111+
Key: aws.String("key1"),
112+
Value: aws.String("value1"),
113+
},
114+
{
115+
Key: aws.String("key2"),
116+
Value: aws.String("value2"),
117+
},
118+
},
119+
},
120+
Status: svcapitypes.ReplicationGroupStatus{
121+
ACKResourceMetadata: &ackv1alpha1.ResourceMetadata{
122+
ARN: (*ackv1alpha1.AWSResourceName)(aws.String("testARN")),
123+
},
124+
},
125+
}})
126+
mocksdkapi.AssertNumberOfCalls(t, "RemoveTagsFromResourceWithContext", 0)
127+
mocksdkapi.AssertNumberOfCalls(t, "AddTagsToResourceWithContext", 1)
128+
})
129+
}

0 commit comments

Comments
 (0)