-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Issue #459 - EBS information required during cluster config change #1131
Issue #459 - EBS information required during cluster config change #1131
Conversation
…ration change AWS demands to pass information about EBS volume during cluster configuration change like e.g. number of data nodes. Issue: hashicorp#459 Debug data from issue: ``` cluster_config.0.instance_count: "2" => "1" 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:01 [DEBUG] [aws-sdk-go] DEBUG: Request es/UpdateElasticsearchDomainConfig Details: 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: ---[ REQUEST POST-SIGN ]----------------------------- 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: POST /2015-01-01/es/domain/daniel-test/config HTTP/1.1 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Host: es.us-east-1.amazonaws.com 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: User-Agent: aws-sdk-go/1.10.8 (go1.8.1; linux; amd64) APN/1.0 HashiCorp/1.0 Terraform/0.9.8 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Content-Length: 150 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Authorization: AWS4-HMAC-SHA256 Credential=/20170712/us-east-1/es/aws4_request, SignedHeaders=content-length;host;x-amz-date, Signature= 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: X-Amz-Date: 20170712T114701Z 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Accept-Encoding: gzip 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: {"ElasticsearchClusterConfig":{"DedicatedMasterEnabled":false,"InstanceCount":2,"InstanceType":"t2.small.elasticsearch","ZoneAwarenessEnabled":false}} 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: ----------------------------------------------------- 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:02 [DEBUG] [aws-sdk-go] DEBUG: Response es/UpdateElasticsearchDomainConfig Details: 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: ---[ RESPONSE ]-------------------------------------- 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: HTTP/1.1 400 Bad Request 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: Content-Length: 188 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: Content-Type: application/json 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: Date: Wed, 12 Jul 2017 11:47:01 GMT 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: X-Amzn-Errortype: ValidationException 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: X-Amzn-Requestid: 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: ----------------------------------------------------- 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:02 [DEBUG] [aws-sdk-go] {"message":"New cluster configuration has insufficient storage capacity for your current data. The new configuration only supports up to 0.0GB. Your current Elasticsearch usage is 0.00GB"} ``` Debug data after fix: ``` cluster_config.0.instance_count: "2" => "1" 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:35 [DEBUG] [aws-sdk-go] DEBUG: Request es/UpdateElasticsearchDomainConfig Details: 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: ---[ REQUEST POST-SIGN ]----------------------------- 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: POST /2015-01-01/es/domain/daniel-test/config HTTP/1.1 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Host: es.us-east-1.amazonaws.com 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: User-Agent: aws-sdk-go/1.10.8 (go1.8.1; linux; amd64) APN/1.0 HashiCorp/1.0 Terraform/0.9.8 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Content-Length: 218 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Authorization: AWS4-HMAC-SHA256 Credential=/20170712/us-east-1/es/aws4_request, SignedHeaders=content-length;host;x-amz-date, Signature= 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: X-Amz-Date: 20170712T124235Z 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Accept-Encoding: gzip 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: {"EBSOptions":{"EBSEnabled":true,"VolumeSize":10,"VolumeType":"gp2"},"ElasticsearchClusterConfig":{"DedicatedMasterEnabled":false,"InstanceCount":1,"InstanceType":"t2.small.elasticsearch","ZoneAwarenessEnabled":false}} 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: ----------------------------------------------------- 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:36 [DEBUG] [aws-sdk-go] DEBUG: Response es/UpdateElasticsearchDomainConfig Details: 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: ---[ RESPONSE ]-------------------------------------- 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: HTTP/1.1 200 OK 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: Content-Length: 1355 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: Content-Type: application/json 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: Date: Wed, 12 Jul 2017 12:42:36 GMT 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: X-Amzn-Requestid: 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: ----------------------------------------------------- 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:36 [DEBUG] [aws-sdk-go] {"DomainConfig":{"AccessPo...<CUT> ``` Please verify. Thanks
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.
Hi @DanKans
do you mind attaching an acceptance test with the example from #459
We store the relevant acceptance tests in https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_elasticsearch_domain_test.go where you will also find some existing tests for this resource, so feel free to take the "copy - paste - modify" approach.
Thanks!
} | ||
|
||
d.SetPartial("tags") | ||
|
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.
What exactly is the reason for this change?
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.
The reason is purely cosmetic. In case where there is an error, function will return
from function so there is no need to keep else
. From other side, by using 'else' we can show that there is a relation between setTagsElasticsearchService
and d.SetPartial("tags")
, so if there is any relation between them maybe we should keep else
:>
btw. In documentation there is no else
block -https://www.terraform.io/docs/plugins/provider.html
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.
In case where there is an error, function will
return
from function so there is no need to keepelse
Ah, you're right, good point. Early return FTW 👍
Simple test, just to check if number of instances is correct.
Hi @radeksimko, I've added a simply test to check if number of instances is equal. IMHO there should be some general test for update feature. Please verify and let me know what do you think. Thanks! |
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 making changes to the PR.
I left you some comments.
testAccCheckESDomainExists("aws_elasticsearch_domain.example", &input), | ||
testAccCheckESNumberOfInstances(ri, input.ElasticsearchClusterConfig), | ||
), | ||
}, |
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.
IMHO there should be some general test for update feature.
There's a way to test updates. You just add another TestStep
into the slice with modified config - then the first step will create resources and second step will modify.
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.
Take a look at how we test updates of tags in TestAccAWSElasticSearchDomain_tags
above. 😉
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.
Thanks! I've added some code. Could you please take a look and let me know what do you think?
} | ||
|
||
cluster_config { | ||
instance_count = %d |
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 certainly static number, we don't want to be spinning up an unbound random number of instances 💸 😃
Randomization is however certainly desired, but only in places that are unique, like domain_name
see other tests in this file.
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.
Good point. I've also changed instance type to t2.micro.elasticsearch
.
da086f5
to
d94f173
Compare
635c5b7
to
93d7403
Compare
93d7403
to
700bb88
Compare
I made some enhancements to the test to make it pass. Thanks for all the efforts. Merging 🎉 |
I'm looking into your commits in order to learn! |
…change (hashicorp#1131) * Issue hashicorp#459 - EBS information required during cluster configuration change AWS demands to pass information about EBS volume during cluster configuration change like e.g. number of data nodes. Issue: hashicorp#459 Debug data from issue: ``` cluster_config.0.instance_count: "2" => "1" 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:01 [DEBUG] [aws-sdk-go] DEBUG: Request es/UpdateElasticsearchDomainConfig Details: 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: ---[ REQUEST POST-SIGN ]----------------------------- 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: POST /2015-01-01/es/domain/daniel-test/config HTTP/1.1 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Host: es.us-east-1.amazonaws.com 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: User-Agent: aws-sdk-go/1.10.8 (go1.8.1; linux; amd64) APN/1.0 HashiCorp/1.0 Terraform/0.9.8 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Content-Length: 150 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Authorization: AWS4-HMAC-SHA256 Credential=/20170712/us-east-1/es/aws4_request, SignedHeaders=content-length;host;x-amz-date, Signature= 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: X-Amz-Date: 20170712T114701Z 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Accept-Encoding: gzip 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: {"ElasticsearchClusterConfig":{"DedicatedMasterEnabled":false,"InstanceCount":2,"InstanceType":"t2.small.elasticsearch","ZoneAwarenessEnabled":false}} 2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: ----------------------------------------------------- 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:02 [DEBUG] [aws-sdk-go] DEBUG: Response es/UpdateElasticsearchDomainConfig Details: 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: ---[ RESPONSE ]-------------------------------------- 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: HTTP/1.1 400 Bad Request 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: Content-Length: 188 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: Content-Type: application/json 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: Date: Wed, 12 Jul 2017 11:47:01 GMT 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: X-Amzn-Errortype: ValidationException 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: X-Amzn-Requestid: 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: ----------------------------------------------------- 2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:02 [DEBUG] [aws-sdk-go] {"message":"New cluster configuration has insufficient storage capacity for your current data. The new configuration only supports up to 0.0GB. Your current Elasticsearch usage is 0.00GB"} ``` Debug data after fix: ``` cluster_config.0.instance_count: "2" => "1" 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:35 [DEBUG] [aws-sdk-go] DEBUG: Request es/UpdateElasticsearchDomainConfig Details: 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: ---[ REQUEST POST-SIGN ]----------------------------- 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: POST /2015-01-01/es/domain/daniel-test/config HTTP/1.1 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Host: es.us-east-1.amazonaws.com 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: User-Agent: aws-sdk-go/1.10.8 (go1.8.1; linux; amd64) APN/1.0 HashiCorp/1.0 Terraform/0.9.8 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Content-Length: 218 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Authorization: AWS4-HMAC-SHA256 Credential=/20170712/us-east-1/es/aws4_request, SignedHeaders=content-length;host;x-amz-date, Signature= 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: X-Amz-Date: 20170712T124235Z 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Accept-Encoding: gzip 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: {"EBSOptions":{"EBSEnabled":true,"VolumeSize":10,"VolumeType":"gp2"},"ElasticsearchClusterConfig":{"DedicatedMasterEnabled":false,"InstanceCount":1,"InstanceType":"t2.small.elasticsearch","ZoneAwarenessEnabled":false}} 2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: ----------------------------------------------------- 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:36 [DEBUG] [aws-sdk-go] DEBUG: Response es/UpdateElasticsearchDomainConfig Details: 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: ---[ RESPONSE ]-------------------------------------- 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: HTTP/1.1 200 OK 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: Content-Length: 1355 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: Content-Type: application/json 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: Date: Wed, 12 Jul 2017 12:42:36 GMT 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: X-Amzn-Requestid: 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: ----------------------------------------------------- 2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:36 [DEBUG] [aws-sdk-go] {"DomainConfig":{"AccessPo...<CUT> ``` Please verify. Thanks * fix formatting * add test for hashicorp#459 Simple test, just to check if number of instances is correct. * fix and adjust testing method * fix function name for as an entrypoint for Config * test: Randomize name of the cluster * test: Use even number of data nodes * test: Specify valid snapshot hours * test: Fix test functions * Fix tests
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
AWS demands to pass information about EBS volume during cluster configuration change like e.g. number of data nodes.
Fixes #459
Debug data from issue:
Debug data after fix:
Please verify.
Thanks