-
Notifications
You must be signed in to change notification settings - Fork 9
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
Tag and Untag API for cluster. #43
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
---|---|---|
|
@@ -27,4 +27,11 @@ | |
respErr := rm.setAllowedNodeTypeUpdates(ctx, ko) | ||
if respErr != nil { | ||
return nil, respErr | ||
} | ||
} | ||
|
||
resourceARN := (*string)(ko.Status.ACKResourceMetadata.ARN) | ||
tags, err := rm.getTags(ctx, *resourceARN) | ||
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. Indentation needs fixing here 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. Not sure why this happened. The indentation looks good in my local file. 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. Github bug I think |
||
if err != nil { | ||
return nil, err | ||
} | ||
ko.Spec.Tags = tags |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
id: "CLUSTER_UPDATE_WITH_TAGS" | ||
description: "In this test we create cluster and update cluster with tags" | ||
#marks: | ||
# - slow | ||
# - blocked | ||
steps: | ||
- id: "CLUSTER_INITIAL_CREATE" | ||
description: "Create Cluster with no tags" | ||
create: | ||
apiVersion: $CRD_GROUP/$CRD_VERSION | ||
kind: Cluster | ||
metadata: | ||
name: cluster$RANDOM_SUFFIX | ||
spec: | ||
name: cluster$RANDOM_SUFFIX | ||
nodeType: db.t4g.small | ||
aclName: open-access | ||
numShards: 1 | ||
wait: | ||
status: | ||
conditions: | ||
ACK.ResourceSynced: | ||
status: "True" | ||
timeout: 7200 | ||
- id: "CLUSTER_ADD_TAGS" | ||
description: "Add tags in Cluster" | ||
patch: | ||
apiVersion: $CRD_GROUP/$CRD_VERSION | ||
kind: Cluster | ||
metadata: | ||
name: cluster$RANDOM_SUFFIX | ||
spec: | ||
tags: | ||
- key: "test_key_1" | ||
value: "test_value_1" | ||
- key: "test_key_2" | ||
- key: | ||
wait: | ||
status: | ||
conditions: | ||
ACK.ResourceSynced: | ||
status: "True" | ||
timeout: 100 | ||
- id: "CLUSTER_DELETE_TAGS" | ||
description: "Delete tags in Cluster" | ||
patch: | ||
apiVersion: $CRD_GROUP/$CRD_VERSION | ||
kind: Cluster | ||
metadata: | ||
name: cluster$RANDOM_SUFFIX | ||
spec: | ||
tags: | ||
- key: "test_key_1" | ||
value: "test_value_1" | ||
wait: | ||
status: | ||
conditions: | ||
ACK.ResourceSynced: | ||
status: "True" | ||
timeout: 100 | ||
- id: "Cluster_ADD_AND_DELETE_TAGS" | ||
description: "Add some tags and delete some tags in Cluster" | ||
patch: | ||
apiVersion: $CRD_GROUP/$CRD_VERSION | ||
kind: Cluster | ||
metadata: | ||
name: cluster$RANDOM_SUFFIX | ||
spec: | ||
tags: | ||
- key: "test_key_2" | ||
value: "test_value_2" | ||
wait: | ||
status: | ||
conditions: | ||
ACK.ResourceSynced: | ||
status: "True" | ||
timeout: 100 | ||
- id: "DELETE_CLUSTER" | ||
description: "Delete the cluster" | ||
delete: | ||
apiVersion: $CRD_GROUP/$CRD_VERSION | ||
kind: Cluster | ||
metadata: | ||
name: cluster$RANDOM_SUFFIX |
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.
This should be a
READ_MANY
instead ofGET
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.
Thank you for correcting me!
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.
After careful consideration, I still think it should be
GET
here. May I know why you think it should beREAD_MANY
here instead? @jljacoThere 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.
@kyriechen96 please disregard this comment -- there may be some inconsistencies across ACK controllers in how this choice is made, and we will circle back to this later if necessary. you're good with
GET
!