[Improvement] state/elasticsearch: add option to spread nodes in 3-AZs #524
Conversation
| @@ -78,8 +79,13 @@ Parameters: | |||
| Type: String | |||
| Default: '5.5' | |||
| AllowedValues: ['7.7', '7.4', '7.1', '6.8', '6.7', '6.5', '6.4', '6.3', '6.2', '6.0', '5.6', '5.5'] # aws es list-elasticsearch-versions --query "ElasticsearchVersions[]" | |||
| AvailabilityZones: | |||
There was a problem hiding this comment.
What's the benefit of using a String instead of a Number type?
There was a problem hiding this comment.
I can find the configuration value faster in the list of parameters, because it looks differently visually. I do not know anymore why I choose a string instead of a number, because the change was made months ago. It could be that I wanted to create a similarity to the 2azs and 3azs vpc templates.
| @@ -163,25 +171,37 @@ Resources: | |||
| DedicatedMasterCount: !If [HasDedicatedMasterNodes, !Ref DedicatedMasterCount, !Ref 'AWS::NoValue'] | |||
| DedicatedMasterEnabled: !If [HasDedicatedMasterNodes, true, false] | |||
| DedicatedMasterType: !If [HasDedicatedMasterNodes, !Ref DedicatedMasterType, !Ref 'AWS::NoValue'] | |||
| InstanceCount: !Ref ClusterInstanceCount | |||
| InstanceCount: !If | |||
There was a problem hiding this comment.
Why did you decide to do it like that? Why not just use the InstanceCount in any case? Wouldn't that be less complex?
There was a problem hiding this comment.
I always try to automatically set the correct values for dependent values and the count of instances must be at least the number of availability zones. I contemplated setting the default value to 0 and only then use the number of availability zones as instance count. But I was not sure if this will cause an issue if someone updates the template and is not aware, that the default value has changed.
There was a problem hiding this comment.
I think we should add one parameter AvailabilityZoneCount that can be either 2 or 3.
If InstanceCount = 1: we ignore AvailabilityZoneCount and use subnet A.
If InstanceCount > 1 && AvailabilityZoneCount = 2: we use subnet A, and B.
If InstanceCount > 1 && AvailabilityZoneCount = 3: we use subnetA, B, and C.
Would that make sense?
Adds the option to spread the nodes over 3 availability zones