feat(aws): Adds support for Elasticache. #218
Conversation
There was a problem hiding this comment.
Thanks @robh007! I left my suggestions, they also apply to aws_elasticache_replication_group.
Also, what happened to this discussion:
Rob I've added support for elasticache cluster. However with elasticache cluster you can also create an elasticache_replcation_group. So in terraform there's two resources, the cluster can reference the replication group. So the cost is associated with the replication group. I was thinking that I add the elasticache_replication_group as another supported resource. If the cluster specifies the replication group the cost will be shown in the replication group & not the cluster. Does that make sense? I looked at what DynamoDB does with Global tables, but that's slightly different as the dynamoDB resource supports global tables & there's also an actual resource for global_tables.
Ali: Yeah I think that makes sense, so if aws_elasticache_cluster.replication_group_id is set, it gets no costs and the aws_elasticache_replication_group.number_cache_clusters/node_type are used to calculate the cost, right?
Rob: Yeah that appears to be correct. It's only applicable for Redis clusters.
|
|
||
| if snapShotRetentionLimit.GreaterThan(decimal.NewFromInt(0)) { | ||
| costComponents = append(costComponents, &schema.CostComponent{ | ||
| Name: "Elasticache snapshot storage", |
There was a problem hiding this comment.
| Name: "Elasticache snapshot storage", | |
| Name: "Backup storage", |
We use that already and it matches the AWS pricing page
|
|
||
| if cacheEngine == "redis" && d.Get("snapshot_retention_limit").Exists() { | ||
| snapShotRetentionLimit = decimal.NewFromInt(d.Get("snapshot_retention_limit").Int()) | ||
| backupRetention = snapShotRetentionLimit.Sub(decimal.NewFromInt(1)) |
There was a problem hiding this comment.
This should only be done if snapShotRetentionLimit > 0, right? the docs mention "f the value of SnapshotRetentionLimit is set to zero (0), backups are turned off."
|
|
||
| snapShotRetentionLimit := decimal.Zero | ||
| backupRetention := decimal.Zero | ||
| monthlyBackupStorageTotal := decimal.Zero |
There was a problem hiding this comment.
I think we should default this to nil (not zero), that means that the costComponent's MonthlyQuantity defaults to nil, so the table/json output shows -/nil for the cost but the price should be shown. Zero implies that either we or the user has provided params that can be used to calculate the cost, and that cost might be 0, but nil means it's unknown.
| backupRetention = snapShotRetentionLimit.Sub(decimal.NewFromInt(1)) | ||
| } | ||
|
|
||
| if u != nil && u.Get("monthly_backup_storage").Exists() { |
There was a problem hiding this comment.
| if u != nil && u.Get("monthly_backup_storage").Exists() { | |
| if u != nil && u.Get("backup_storage").Exists() { |
This might be clearer? so the user defines backup_storage.snapshot_storage_size and we multiply that by the backupRetention to calculate the monthly storage vs the user providing the monthly_backup_storage...
| }, | ||
| } | ||
|
|
||
| if snapShotRetentionLimit.GreaterThan(decimal.NewFromInt(0)) { |
There was a problem hiding this comment.
| if snapShotRetentionLimit.GreaterThan(decimal.NewFromInt(0)) { | |
| if cacheEngine == "redis" && snapShotRetentionLimit.GreaterThan(decimal.NewFromInt(1)) { |
How about if we show this costComponent for redis where the retention is more than 1 (0 means no backups so no point in showing it, and 1 means it's free per the AWS pricing page)? Lines 35-42 could also move inside this if block, makes it slightly easier to read the code...
|
@alikhajeh1 I've updated inline with your comments. Plus I've added the reference to the |
|
Closing this as #222 has an updated/refactored version |
Objective:
Adds support for
elasticahe_cluster&elasticache_replication_group** Closes ** #105
Example output:
Pricing details:
The elasticache service provides 2 pricing components described in https://aws.amazon.com/elasticache/pricing/. This adds support for the Elasticache clusters & replication_groups.