Skip to content

Commit ec7725e

Browse files
Julian-ChuJulian-Chu
authored
Fix/gsi provisionthroughout on demand mode (#71)
Issue [#1782](aws-controllers-k8s/community#1782) Description of changes: - add logic to skip comparison when GSI is under `PAY_PER_REQUEST` mode - add unit test cases By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent d5c7414 commit ec7725e

File tree

4 files changed

+150
-1
lines changed

4 files changed

+150
-1
lines changed

‎go.mod‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ require (
77
github.com/aws/aws-sdk-go v1.44.93
88
github.com/go-logr/logr v1.2.3
99
github.com/spf13/pflag v1.0.5
10+
github.com/stretchr/testify v1.8.0
1011
k8s.io/api v0.26.1
1112
k8s.io/apimachinery v0.26.1
1213
k8s.io/client-go v0.26.1
@@ -45,6 +46,7 @@ require (
4546
github.com/modern-go/reflect2 v1.0.2 // indirect
4647
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
4748
github.com/pkg/errors v0.9.1 // indirect
49+
github.com/pmezard/go-difflib v1.0.0 // indirect
4850
github.com/prometheus/client_golang v1.14.0 // indirect
4951
github.com/prometheus/client_model v0.3.0 // indirect
5052
github.com/prometheus/common v0.37.0 // indirect

‎go.sum‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,13 +274,16 @@ github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag
274274
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
275275
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
276276
github.com/stretchr/objx v0.4.0 h1:M2gUjqZET1qApGOWNSnZ49BAIMX4F/1plDv3+l31EJ4=
277+
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
277278
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
278279
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
279280
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
280281
github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
281282
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
282283
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
284+
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
283285
github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
286+
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
284287
github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
285288
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
286289
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

‎pkg/resource/table/hooks_global_secondary_indexes.go‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,9 @@ func equalGlobalSecondaryIndexes(
8484
b *v1alpha1.GlobalSecondaryIndex,
8585
) bool {
8686
if ackcompare.HasNilDifference(a.ProvisionedThroughput, b.ProvisionedThroughput) {
87-
return false
87+
if !isPayPerRequestMode(a, b) {
88+
return false
89+
}
8890
}
8991
if a.ProvisionedThroughput != nil && b.ProvisionedThroughput != nil {
9092
if !equalInt64s(a.ProvisionedThroughput.ReadCapacityUnits, b.ProvisionedThroughput.ReadCapacityUnits) {
@@ -115,6 +117,29 @@ func equalGlobalSecondaryIndexes(
115117
return true
116118
}
117119

120+
// isPayPerRequestMode catches the exceptional case for GSI with PAY_PER_REQUEST billing mode
121+
// if a.ProvisionedThroughput is nil and b.ProvisionedThroughput is not nil but with 0 capacity
122+
// because aws set the default value to 0 for provisioned throughput when billing mode is PAY_PER_REQUEST
123+
// see https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_ProvisionedThroughput.html
124+
func isPayPerRequestMode(desired *v1alpha1.GlobalSecondaryIndex, latest *v1alpha1.GlobalSecondaryIndex) bool {
125+
if desired.ProvisionedThroughput == nil &&
126+
latest.ProvisionedThroughput != nil &&
127+
aws.Int64Value(latest.ProvisionedThroughput.WriteCapacityUnits) == 0 &&
128+
aws.Int64Value(latest.ProvisionedThroughput.ReadCapacityUnits) == 0 {
129+
return true
130+
}
131+
132+
// this case should not happen with dynamodb request validation, but just in case
133+
if desired.ProvisionedThroughput != nil &&
134+
latest.ProvisionedThroughput == nil &&
135+
aws.Int64Value(desired.ProvisionedThroughput.WriteCapacityUnits) == 0 &&
136+
aws.Int64Value(desired.ProvisionedThroughput.ReadCapacityUnits) == 0 {
137+
return true
138+
}
139+
140+
return false
141+
}
142+
118143
// syncTableGlobalSecondaryIndexes updates a global table secondary indexes.
119144
func (rm *resourceManager) syncTableGlobalSecondaryIndexes(
120145
ctx context.Context,

‎pkg/resource/table/hooks_test.go‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919

2020
"github.com/aws-controllers-k8s/runtime/pkg/compare"
2121
"github.com/aws/aws-sdk-go/aws"
22+
"github.com/stretchr/testify/require"
2223

2324
"github.com/aws-controllers-k8s/dynamodb-controller/apis/v1alpha1"
2425
)
@@ -147,6 +148,124 @@ func Test_customPreCompare(t *testing.T) {
147148
t.Errorf("b.Spec.ProvisionedThroughput should be nil, but got %+v", a.ko.Spec.ProvisionedThroughput)
148149
}
149150
})
151+
152+
t.Run("GSI ProvisionedThroughput should be equal when nil and 0 capacity", func(t *testing.T) {
153+
a := &resource{ko: &v1alpha1.Table{
154+
Spec: v1alpha1.TableSpec{
155+
BillingMode: aws.String(string(v1alpha1.BillingMode_PAY_PER_REQUEST)),
156+
ProvisionedThroughput: &v1alpha1.ProvisionedThroughput{},
157+
GlobalSecondaryIndexes: []*v1alpha1.GlobalSecondaryIndex{
158+
{
159+
IndexName: aws.String("index1"),
160+
KeySchema: []*v1alpha1.KeySchemaElement{
161+
{
162+
AttributeName: aws.String("id"),
163+
KeyType: aws.String("HASH"),
164+
},
165+
{
166+
AttributeName: aws.String("email"),
167+
KeyType: aws.String("RANGE"),
168+
},
169+
},
170+
Projection: &v1alpha1.Projection{
171+
ProjectionType: aws.String("ALL"),
172+
},
173+
ProvisionedThroughput: nil,
174+
},
175+
},
176+
},
177+
}}
178+
179+
b := &resource{ko: &v1alpha1.Table{
180+
Spec: v1alpha1.TableSpec{
181+
BillingMode: aws.String(string(v1alpha1.BillingMode_PAY_PER_REQUEST)),
182+
ProvisionedThroughput: &v1alpha1.ProvisionedThroughput{},
183+
GlobalSecondaryIndexes: []*v1alpha1.GlobalSecondaryIndex{
184+
{
185+
IndexName: aws.String("index1"),
186+
KeySchema: []*v1alpha1.KeySchemaElement{
187+
{
188+
AttributeName: aws.String("id"),
189+
KeyType: aws.String("HASH"),
190+
},
191+
{
192+
AttributeName: aws.String("email"),
193+
KeyType: aws.String("RANGE"),
194+
},
195+
},
196+
Projection: &v1alpha1.Projection{
197+
ProjectionType: aws.String("ALL"),
198+
},
199+
ProvisionedThroughput: &v1alpha1.ProvisionedThroughput{
200+
ReadCapacityUnits: aws.Int64(0),
201+
WriteCapacityUnits: aws.Int64(0),
202+
},
203+
},
204+
},
205+
},
206+
}}
207+
delta := &compare.Delta{}
208+
customPreCompare(delta, a, b)
209+
require.False(t, delta.DifferentAt("Spec.GlobalSecondaryIndexes"))
210+
211+
// the following case should not happen, just in case
212+
c := &resource{ko: &v1alpha1.Table{
213+
Spec: v1alpha1.TableSpec{
214+
BillingMode: aws.String(string(v1alpha1.BillingMode_PAY_PER_REQUEST)),
215+
ProvisionedThroughput: &v1alpha1.ProvisionedThroughput{},
216+
GlobalSecondaryIndexes: []*v1alpha1.GlobalSecondaryIndex{
217+
{
218+
IndexName: aws.String("index1"),
219+
KeySchema: []*v1alpha1.KeySchemaElement{
220+
{
221+
AttributeName: aws.String("id"),
222+
KeyType: aws.String("HASH"),
223+
},
224+
{
225+
AttributeName: aws.String("email"),
226+
KeyType: aws.String("RANGE"),
227+
},
228+
},
229+
Projection: &v1alpha1.Projection{
230+
ProjectionType: aws.String("ALL"),
231+
},
232+
ProvisionedThroughput: nil,
233+
},
234+
},
235+
},
236+
}}
237+
238+
d := &resource{ko: &v1alpha1.Table{
239+
Spec: v1alpha1.TableSpec{
240+
BillingMode: aws.String(string(v1alpha1.BillingMode_PAY_PER_REQUEST)),
241+
ProvisionedThroughput: &v1alpha1.ProvisionedThroughput{},
242+
GlobalSecondaryIndexes: []*v1alpha1.GlobalSecondaryIndex{
243+
{
244+
IndexName: aws.String("index1"),
245+
KeySchema: []*v1alpha1.KeySchemaElement{
246+
{
247+
AttributeName: aws.String("id"),
248+
KeyType: aws.String("HASH"),
249+
},
250+
{
251+
AttributeName: aws.String("email"),
252+
KeyType: aws.String("RANGE"),
253+
},
254+
},
255+
Projection: &v1alpha1.Projection{
256+
ProjectionType: aws.String("ALL"),
257+
},
258+
ProvisionedThroughput: &v1alpha1.ProvisionedThroughput{
259+
ReadCapacityUnits: aws.Int64(0),
260+
WriteCapacityUnits: aws.Int64(0),
261+
},
262+
},
263+
},
264+
},
265+
}}
266+
customPreCompare(delta, c, d)
267+
require.False(t, delta.DifferentAt("Spec.GlobalSecondaryIndexes"))
268+
})
150269
}
151270

152271
func Test_newResourceDelta_customDeltaFunction_AttributeDefinitions(t *testing.T) {

0 commit comments

Comments
 (0)