Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Issue #459 - EBS information required during cluster config change #1131

Merged
merged 10 commits into from
Jul 31, 2017
Merged

Issue #459 - EBS information required during cluster config change #1131

Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b0a8352
Issue #459 - EBS information required during cluster configuration ch…
Jul 12, 2017
40a82fe
fix formatting
Jul 12, 2017
7071e77
add test for #459
Jul 26, 2017
268a455
fix and adjust testing method
Jul 31, 2017
eaa9795
fix function name for as an entrypoint for Config
Jul 31, 2017
d94f173
test: Randomize name of the cluster
radeksimko Jul 31, 2017
b24b01a
test: Use even number of data nodes
radeksimko Jul 31, 2017
53ccd7c
test: Specify valid snapshot hours
radeksimko Jul 31, 2017
700bb88
test: Fix test functions
radeksimko Jul 31, 2017
9a41771
Fix tests
radeksimko Jul 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions aws/resource_aws_elasticsearch_domain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,10 +358,10 @@ func resourceAwsElasticSearchDomainUpdate(d *schema.ResourceData, meta interface

if err := setTagsElasticsearchService(conn, d, d.Id()); err != nil {
return err
} else {
d.SetPartial("tags")
}

d.SetPartial("tags")

Copy link
Member

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?

Copy link
Contributor Author

@dsalbert dsalbert Jul 20, 2017 •
edited
Loading

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

Copy link
Member

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 keep else

Ah, you're right, good point. Early return FTW 👍

input := elasticsearch.UpdateElasticsearchDomainConfigInput{
DomainName: aws.String(d.Get("domain_name").(string)),
}
Expand All @@ -374,7 +374,7 @@ func resourceAwsElasticSearchDomainUpdate(d *schema.ResourceData, meta interface
input.AdvancedOptions = stringMapToPointers(d.Get("advanced_options").(map[string]interface{}))
}

if d.HasChange("ebs_options") {
if d.HasChange("ebs_options") || d.HasChange("cluster_config") {
options := d.Get("ebs_options").([]interface{})

if len(options) > 1 {
Expand All @@ -383,17 +383,18 @@ func resourceAwsElasticSearchDomainUpdate(d *schema.ResourceData, meta interface
s := options[0].(map[string]interface{})
input.EBSOptions = expandESEBSOptions(s)
}
}

if d.HasChange("cluster_config") {
config := d.Get("cluster_config").([]interface{})
if d.HasChange("cluster_config") {
config := d.Get("cluster_config").([]interface{})

if len(config) > 1 {
return fmt.Errorf("Only a single cluster_config block is expected")
} else if len(config) == 1 {
m := config[0].(map[string]interface{})
input.ElasticsearchClusterConfig = expandESClusterConfig(m)
if len(config) > 1 {
return fmt.Errorf("Only a single cluster_config block is expected")
} else if len(config) == 1 {
m := config[0].(map[string]interface{})
input.ElasticsearchClusterConfig = expandESClusterConfig(m)
}
}

}

if d.HasChange("snapshot_options") {
Expand Down