[Elasticsearch] Implement specifying availability zone count ... #8144

jtcressy · 2019-04-01T14:36:33Z

... when zone awareness is enabled

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Changes proposed in this pull request:

add parameter in cluster_config: zone_awareness_count
- parameter is mapped to config.ZoneAwarenessConfig.AvailabilityZoneCount as int64
add acceptance test case TestAccAWSElasticSearchDomain_ZoneAwarenessWith3AZ
- creates 3 t2.micro.elasticsearch nodes across 3 az's with zone awareness to test this PR

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSElasticSearchDomain_ZoneAwarenessWith3AZ'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSElasticSearchDomain_ZoneAwarenessWith3AZ -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
=== RUN   TestAccAWSElasticSearchDomain_ZoneAwarenessWith3AZ
=== PAUSE TestAccAWSElasticSearchDomain_ZoneAwarenessWith3AZ
=== CONT  TestAccAWSElasticSearchDomain_ZoneAwarenessWith3AZ
--- PASS: TestAccAWSElasticSearchDomain_ZoneAwarenessWith3AZ (772.59s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       797.900s

Implemented my own tests for this. It's based off of TestAccAWSElasticSearchDomain_basic and simply uses cluster_config to deploy 3 nodes across 3 AZ's.

…e awareness enabled.

aws/structure.go

jtcressy · 2019-04-04T22:43:25Z

Removed the WIP marker, as I think this PR is ready for review! It even has tests now! 😄

bflad

Hi @jtcressy 👋 Thanks for submitting this. This will need a few things adjusted before it can be accepted. Please let us know if you have any questions or do not have time to implement the feedback.

Currently the other acceptance testing is failing:

--- FAIL: TestAccAWSElasticSearchDomain_update (35.17s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: NumberOfAvailabilityZones should be either 2 or 3
        	status code: 400, request id: d692895f-5748-11e9-9abc-25f5949255f2

--- FAIL: TestAccAWSElasticSearchDomain_update_version (8.20s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: ZoneAwarenessEnabled must be set to True to specify the ZoneAwarenessSettings
        	status code: 400, request id: db7a06b6-5748-11e9-b707-5f7cde003491

--- FAIL: TestAccAWSElasticSearchDomain_encrypt_at_rest_default_key (54.06s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: ZoneAwarenessEnabled must be set to True to specify the ZoneAwarenessSettings
        	status code: 400, request id: e19f0e51-5748-11e9-b09e-1de5efe18623

--- FAIL: TestAccAWSElasticSearchDomain_complex (71.68s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: NumberOfAvailabilityZones should be either 2 or 3
        	status code: 400, request id: ec15bc6d-5748-11e9-b09e-1de5efe18623

--- FAIL: TestAccAWSElasticSearchDomain_encrypt_at_rest_specify_key (82.53s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: ZoneAwarenessEnabled must be set to True to specify the ZoneAwarenessSettings
        	status code: 400, request id: e62a3b8a-5748-11e9-b15e-f3e3bdb4bca1

--- FAIL: TestAccAWSElasticSearchDomain_vpc_update (87.55s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: NumberOfAvailabilityZones should be either 2 or 3
        	status code: 400, request id: f35f1f92-5748-11e9-ab97-b33e6d431941

--- FAIL: TestAccAWSElasticSearchDomain_update_volume_type (90.56s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: NumberOfAvailabilityZones should be either 2 or 3
        	status code: 400, request id: f7536447-5748-11e9-a09a-4f2eaa601f9d

--- FAIL: TestAccAWSElasticSearchDomain_vpc (107.82s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: NumberOfAvailabilityZones should be either 2 or 3
        	status code: 400, request id: f8369866-5748-11e9-82e4-ab4437639e4f

--- FAIL: TestAccAWSElasticSearchDomain_NodeToNodeEncryption (195.56s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: ZoneAwarenessEnabled must be set to True to specify the ZoneAwarenessSettings
        	status code: 400, request id: 35fc3149-5749-11e9-997c-35621615dd41

--- FAIL: TestAccAWSElasticSearchDomain_withDedicatedMaster (221.49s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: ZoneAwarenessEnabled must be set to True to specify the ZoneAwarenessSettings
        	status code: 400, request id: 454d1792-5749-11e9-8cfc-a9b4ddbad0eb

--- FAIL: TestAccAWSElasticSearchDomain_internetToVpcEndpoint (669.04s)
    testing.go:538: Step 1 error: Error applying: 1 error occurred:
        	* aws_elasticsearch_domain.example: 1 error occurred:
        	* aws_elasticsearch_domain.example: ValidationException: NumberOfAvailabilityZones should be either 2 or 3
        	status code: 400, request id: 4e0afc85-574a-11e9-a303-6be082a5aae1

bflad · 2019-04-05T10:15:41Z

aws/resource_aws_elasticsearch_domain.go

@@ -194,6 +194,10 @@ func resourceAwsElasticSearchDomain() *schema.Resource {
 							Type:     schema.TypeBool,
 							Optional: true,
 						},
+						"zone_awareness_count": {


Two notes:

This argument needs documentation added in website/docs/r/elasticsearch_domain.html.markdown

Since this argument is not marked ForceNew: true, an additional TestStep should be added to the acceptance test that attempts to update or remove it

bflad · 2019-04-05T10:17:48Z

aws/resource_aws_elasticsearch_domain_test.go

+					testAccCheckESDomainExists("aws_elasticsearch_domain.example", &domain),
+					resource.TestCheckResourceAttr(
+						"aws_elasticsearch_domain.example", "elasticsearch_version", "1.5"),
+					resource.TestMatchResourceAttr("aws_elasticsearch_domain.example", "kibana_endpoint", regexp.MustCompile(".*es.amazonaws.com/_plugin/kibana/")),


Should this be testing the new attribute instead? e.g.

Suggested change

resource.TestMatchResourceAttr("aws_elasticsearch_domain.example", "kibana_endpoint", regexp.MustCompile(".*es.amazonaws.com/_plugin/kibana/")),

resource.TestCheckResourceAttr("aws_elasticsearch_domain.example", "cluster_config.0.zone_awareness_count", "3"),

Ok, i'm taking this recommendation and expanding it to multiple test steps:

zone awareness disabled and zone awareness count set to 3

update cluster with zone awareness enabled

update cluster again with zone awareness disabled

update cluster with zone awareness enabled and count set to 2

update cluster zone aware count to 3

jtcressy · 2019-04-05T14:58:25Z

@bflad So, when the vpc_update validation exception of NumberOfAvailabilityZones should be either 2 or 3 occurs, the test seems to be setting zone_awareness_enabled = true but only supplying one subnet in the vpc_config. I'm not sure if this is something I affected by my change. I'll let you know if I still encounter it in my tests.

I've got some changes in strucutre.go to mimic the control structure of setting the dedicated master parameters, and i'll be committing/pushing it soon. I think it'll fix some of the problems we have right now.

…icated masters

… cluster each time.

jtcressy · 2019-04-05T15:56:52Z

@bflad BTW, make testacc TESTARGS='-run=TestAccAWSElasticSearchDomain' is extremely slow... if you have any tips to make this testing phase run faster i'm all ears.

nywilken · 2019-04-05T16:43:53Z

@bflad BTW, make testacc TESTARGS='-run=TestAccAWSElasticSearchDomain' is extremely slow... if you have any tips to make this testing phase run faster i'm all ears.

The string after run= works as a regex expression to match tests to run. You can filter down the selected test cases by narrowing down the scope of the matching expression. For examplemake testacc TESTARGS='-run=TestAccAWSElasticSearchDomain_ZoneAwarenessWith3AZ' will only run this specific test. In order to help speed things up it may help to attack one or two test cases at a time.

Sorry I should mention that you can use the OR operator to call out two distinct test cases

make testacc TESTARGS='-run=TestAccAWSElasticSearchDomain_basic\|TestAccAWSElasticSearchDomain_Zone'

bflad · 2019-04-05T17:29:04Z

Unfortunately the limiting factor here is the AWS Elasticsearch service itself where each of the create/update/delete operations can take 5-60 minutes to complete. I have not seen the default 20 parallelism of the acceptance testing be of any particular problem to the service since most of wait time is polling read-only APIs for a status update and not caused by API retries due to throttling.

keety · 2019-05-30T20:48:02Z

@jtcressy, @bflad thanks for aws_elasticsearch_domain support it has been super useful and I use it extensively to provision elasticsearch on aws. .
I'm pinging this thread since #7504 seems to have a dependency on this .
Was curious if this is being fixed in v2.13.0 or if there was a specific version the fix was targeted for ?

neovatar · 2019-06-06T09:20:21Z

What needs to be done to get this PR merged? It would be great, if we could create a 3 AZ elasticsearch domain with terraform.

bflad

Hi @jtcressy 👋 Its been awhile since we heard from you and it looks like the acceptance testing is still failing in some places (see below). Do you need any assistance or still have time to work on this? The new argument also needs documentation in website/docs/r/elasticsearch_domain.html.markdown. Thanks!

--- FAIL: TestAccAWSElasticSearchDomain_ZoneAwareness (2028.78s)
    testing.go:568: Step 0 error: Check failed: Check 3/3 error: aws_elasticsearch_domain.example: Attribute 'cluster_config.0.zone_awareness_count' expected "3", got "0"

--- FAIL: TestAccAWSElasticSearchDomain_complex (8.79s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: ValidationException: NumberOfAvailabilityZones should be either 2 or 3

--- FAIL: TestAccAWSElasticSearchDomain_internetToVpcEndpoint (1349.31s)
    testing.go:568: Step 1 error: errors during apply:
        
        Error: ValidationException: NumberOfAvailabilityZones should be either 2 or 3

--- FAIL: TestAccAWSElasticSearchDomain_update (9.06s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: ValidationException: NumberOfAvailabilityZones should be either 2 or 3

--- FAIL: TestAccAWSElasticSearchDomain_update_volume_type (62.50s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: ValidationException: NumberOfAvailabilityZones should be either 2 or 3

--- FAIL: TestAccAWSElasticSearchDomain_vpc (63.80s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: ValidationException: NumberOfAvailabilityZones should be either 2 or 3

--- FAIL: TestAccAWSElasticSearchDomain_vpc_update (66.13s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: ValidationException: NumberOfAvailabilityZones should be either 2 or 3

jtcressy · 2019-07-10T16:05:19Z

Hi @bflad, please take over if you have the means/time. I've been swamped at work and haven't had the time to make progress on this PR.

bflad · 2019-07-17T23:07:01Z

No worries, @jtcressy! Thank you so much for your hard work on this. We hope you can contribute again in the future.

I've opened #9398 which includes your commits. Please follow that pull request for further tracking of this functionality. 👍

ghost · 2019-11-02T15:23:20Z

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!

[Elasticsearch] Implement specifying availability zone count when zon…

2c60a5d

…e awareness enabled.

ghost added size/XS Managed by automation to categorize the size of a PR. service/elasticsearch Issues and PRs that pertain to the elasticsearch service. labels Apr 1, 2019

MrBagheera reviewed Apr 4, 2019

View reviewed changes

aws/structure.go Outdated Show resolved Hide resolved

Joel Cressy added 3 commits April 4, 2019 09:17

catch a nil c.ZoneAwarenessConfig before checking its attributes

bc2ea84

implement acceptance test for 3AZ zone-aware deployment

21f3700

assign a ZoneAwarenessConfig struct to the ZAC parameter with AZ Count

0f58958

ghost added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. size/S Managed by automation to categorize the size of a PR. and removed size/XS Managed by automation to categorize the size of a PR. labels Apr 4, 2019

jtcressy changed the title ~~[WIP] [Elasticsearch] Implement specifying availability zone count ...~~ [Elasticsearch] Implement specifying availability zone count ... Apr 4, 2019

bflad added the enhancement Requests to existing resources that expand the functionality or scope. label Apr 5, 2019

bflad reviewed Apr 5, 2019

View reviewed changes

Joel Cressy added 2 commits April 5, 2019 10:19

only set zac when zone awareness is enabled, to mimic the flow of ded…

a5b7d35

…icated masters

Expand test case to multiple zone aware configurations and update the…

87c8646

… cluster each time.

ghost added size/M Managed by automation to categorize the size of a PR. and removed size/S Managed by automation to categorize the size of a PR. labels Apr 5, 2019

irvingpop mentioned this pull request Apr 24, 2019

Elasticsearch, Cloudformation and ZoneAwareness, the struggle is real chef-boneyard/aws_native_chef_server#49

Closed

aeschright requested a review from a team June 26, 2019 00:54

bflad suggested changes Jul 10, 2019

View reviewed changes

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Jul 10, 2019

ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Jul 10, 2019

bflad self-assigned this Jul 17, 2019

bflad mentioned this pull request Jul 17, 2019

resource/aws_elasticsearch_domain: Support three Availability Zone awareness #9398

Merged

bflad closed this Jul 17, 2019

ghost locked and limited conversation to collaborators Nov 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Elasticsearch] Implement specifying availability zone count ... #8144

[Elasticsearch] Implement specifying availability zone count ... #8144

jtcressy commented Apr 1, 2019 •

edited

Loading

jtcressy commented Apr 4, 2019

bflad left a comment

bflad Apr 5, 2019

bflad Apr 5, 2019

jtcressy Apr 5, 2019

jtcressy commented Apr 5, 2019

jtcressy commented Apr 5, 2019

nywilken commented Apr 5, 2019 •

edited

Loading

bflad commented Apr 5, 2019

keety commented May 30, 2019

neovatar commented Jun 6, 2019

bflad left a comment

jtcressy commented Jul 10, 2019

bflad commented Jul 17, 2019

ghost commented Nov 2, 2019

	resource.TestMatchResourceAttr("aws_elasticsearch_domain.example", "kibana_endpoint", regexp.MustCompile(".*es.amazonaws.com/_plugin/kibana/")),
	resource.TestCheckResourceAttr("aws_elasticsearch_domain.example", "cluster_config.0.zone_awareness_count", "3"),

[Elasticsearch] Implement specifying availability zone count ... #8144

[Elasticsearch] Implement specifying availability zone count ... #8144

Conversation

jtcressy commented Apr 1, 2019 • edited Loading

Community Note

jtcressy commented Apr 4, 2019

bflad left a comment

Choose a reason for hiding this comment

bflad Apr 5, 2019

Choose a reason for hiding this comment

bflad Apr 5, 2019

Choose a reason for hiding this comment

jtcressy Apr 5, 2019

Choose a reason for hiding this comment

jtcressy commented Apr 5, 2019

jtcressy commented Apr 5, 2019

nywilken commented Apr 5, 2019 • edited Loading

bflad commented Apr 5, 2019

keety commented May 30, 2019

neovatar commented Jun 6, 2019

bflad left a comment

Choose a reason for hiding this comment

jtcressy commented Jul 10, 2019

bflad commented Jul 17, 2019

ghost commented Nov 2, 2019

jtcressy commented Apr 1, 2019 •

edited

Loading

nywilken commented Apr 5, 2019 •

edited

Loading