-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: creating gsi under on-demand mode #85
Changes from all commits
896b8f8
c5cf663
9ef9ac6
99c30f2
35de4f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -233,21 +233,21 @@ func (rm *resourceManager) newUpdateTableGlobalSecondaryIndexUpdatesPayload( | |
|
||
// newSDKProvisionedThroughput builds a new *svcsdk.ProvisionedThroughput | ||
func newSDKProvisionedThroughput(pt *v1alpha1.ProvisionedThroughput) *svcsdk.ProvisionedThroughput { | ||
provisionedThroughput := &svcsdk.ProvisionedThroughput{} | ||
if pt != nil { | ||
if pt.ReadCapacityUnits != nil { | ||
provisionedThroughput.ReadCapacityUnits = aws.Int64(*pt.ReadCapacityUnits) | ||
} else { | ||
provisionedThroughput.ReadCapacityUnits = aws.Int64(0) | ||
} | ||
if pt.WriteCapacityUnits != nil { | ||
provisionedThroughput.WriteCapacityUnits = aws.Int64(*pt.WriteCapacityUnits) | ||
} else { | ||
provisionedThroughput.WriteCapacityUnits = aws.Int64(0) | ||
} | ||
} else { | ||
provisionedThroughput.ReadCapacityUnits = aws.Int64(0) | ||
provisionedThroughput.WriteCapacityUnits = aws.Int64(0) | ||
if pt == nil { | ||
return nil | ||
} | ||
provisionedThroughput := &svcsdk.ProvisionedThroughput{ | ||
// ref: https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_ProvisionedThroughput.html | ||
// Minimum capacity units is 1 when using provisioned capacity mode | ||
ReadCapacityUnits: aws.Int64(1), | ||
WriteCapacityUnits: aws.Int64(1), | ||
} | ||
if pt.ReadCapacityUnits != nil { | ||
provisionedThroughput.ReadCapacityUnits = aws.Int64(*pt.ReadCapacityUnits) | ||
} | ||
|
||
if pt.WriteCapacityUnits != nil { | ||
provisionedThroughput.WriteCapacityUnits = aws.Int64(*pt.WriteCapacityUnits) | ||
Comment on lines
+239
to
+250
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thoughts: What if we're not using PROVISIONED capacity mode? Would 0's be allowed? If that's the case would it be worth to evaluate the capacity mode in this function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you mean the conditions:
- message: |
InvalidParameter: 3 validation error(s) found.
- minimum field size of 1, UpdateTableInput.GlobalSecondaryIndexUpdates[0].Create.Projection.NonKeyAttributes.
- minimum field value of 1, UpdateTableInput.GlobalSecondaryIndexUpdates[0].Create.ProvisionedThroughput.ReadCapacityUnits.
- minimum field value of 1, UpdateTableInput.GlobalSecondaryIndexUpdates[0].Create.ProvisionedThroughput.WriteCapacityUnits.
status: "True"
type: ACK.Terminal but aws DDB api will set capacity units as 0 when
https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_ProvisionedThroughput.html |
||
} | ||
return provisionedThroughput | ||
} | ||
|
@@ -263,12 +263,10 @@ func newSDKProjection(p *v1alpha1.Projection) *svcsdk.Projection { | |
} | ||
if p.NonKeyAttributes != nil { | ||
projection.NonKeyAttributes = p.NonKeyAttributes | ||
} else { | ||
projection.NonKeyAttributes = []*string{} | ||
} | ||
} else { | ||
projection.ProjectionType = aws.String("") | ||
projection.NonKeyAttributes = []*string{} | ||
projection.NonKeyAttributes = nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another nice catch! |
||
} | ||
return projection | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
package table | ||
|
||
import ( | ||
"reflect" | ||
"testing" | ||
|
||
"github.com/aws/aws-sdk-go/aws" | ||
svcsdk "github.com/aws/aws-sdk-go/service/dynamodb" | ||
|
||
"github.com/aws-controllers-k8s/dynamodb-controller/apis/v1alpha1" | ||
) | ||
|
||
func Test_newSDKProvisionedThroughput(t *testing.T) { | ||
type args struct { | ||
pt *v1alpha1.ProvisionedThroughput | ||
} | ||
tests := []struct { | ||
name string | ||
args args | ||
want *svcsdk.ProvisionedThroughput | ||
}{ | ||
{ | ||
name: "provisioned throughput is nil", | ||
args: args{ | ||
pt: nil, | ||
}, | ||
want: nil, | ||
}, | ||
{ | ||
name: "provisioned throughput is not nil, read capacity units is nil", | ||
args: args{ | ||
pt: &v1alpha1.ProvisionedThroughput{ | ||
ReadCapacityUnits: nil, | ||
WriteCapacityUnits: aws.Int64(10), | ||
}, | ||
}, | ||
want: &svcsdk.ProvisionedThroughput{ | ||
ReadCapacityUnits: aws.Int64(1), | ||
WriteCapacityUnits: aws.Int64(10), | ||
}, | ||
}, | ||
{ | ||
name: "provisioned throughput is not nil, write capacity units is nil", | ||
args: args{ | ||
pt: &v1alpha1.ProvisionedThroughput{ | ||
ReadCapacityUnits: aws.Int64(10), | ||
WriteCapacityUnits: nil, | ||
}, | ||
}, | ||
want: &svcsdk.ProvisionedThroughput{ | ||
ReadCapacityUnits: aws.Int64(10), | ||
WriteCapacityUnits: aws.Int64(1), | ||
}, | ||
}, | ||
{ | ||
name: "provisioned throughput is not nil, write and read capacity units are not nil", | ||
args: args{ | ||
pt: &v1alpha1.ProvisionedThroughput{ | ||
ReadCapacityUnits: aws.Int64(5), | ||
WriteCapacityUnits: aws.Int64(5), | ||
}, | ||
}, | ||
want: &svcsdk.ProvisionedThroughput{ | ||
ReadCapacityUnits: aws.Int64(5), | ||
WriteCapacityUnits: aws.Int64(5), | ||
}, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
if got := newSDKProvisionedThroughput(tt.args.pt); !reflect.DeepEqual(got, tt.want) { | ||
t.Errorf("newSDKProvisionedThroughput() = %v, want %v", got, tt.want) | ||
} | ||
}) | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
# Table used to test GSI creation under on-demand billing mode | ||
apiVersion: dynamodb.services.k8s.aws/v1alpha1 | ||
kind: Table | ||
metadata: | ||
name: $TABLE_NAME | ||
spec: | ||
tableName: $TABLE_NAME | ||
billingMode: PAY_PER_REQUEST | ||
tableClass: STANDARD | ||
attributeDefinitions: | ||
- attributeName: Bill | ||
attributeType: S | ||
- attributeName: Total | ||
attributeType: S | ||
keySchema: | ||
- attributeName: Bill | ||
keyType: HASH | ||
- attributeName: Total | ||
keyType: RANGE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch sir!