Add AWS PV Pricing #107
Add AWS PV Pricing #107
Conversation
| @@ -489,7 +515,7 @@ func (aws *AWS) DownloadPricingData() error { | |||
| } | |||
| aws.ValidPricingKeys[key] = true | |||
| aws.ValidPricingKeys[spotKey] = true | |||
| } else if strings.HasPrefix(product.Attributes.UsageType, "EBS:Volume") { | |||
| } else if strings.Contains(product.Attributes.UsageType, "EBS:Volume") { | |||
| key := locationToRegion[product.Attributes.Location] + "," + product.Attributes.UsageType | |||
There was a problem hiding this comment.
Usagetype would also need its region prefix to be stripped here because the key mapping above doesn't have region, otherwise you would end up with a key like: us-east-2,USE2-EBS:VolumeUsage.sc1 and a lookup key is something like us-east-2,EBS:VolumeUsage.sc1
There was a problem hiding this comment.
Pushed a commit for this - let me know if you are good with the change I made to handle this or if you were thinking of something else.
There was a problem hiding this comment.
I'd suggest setting the key on L519 here to the stripped value, as the lookup further down on L354 looks up based on pvk.Features(), which returns a key of the format "region,usage type without region". This seems like a cleaner approach, as then there's no need to reconstruct a regional P-IOPS key when you reconstruct the keys on L581.
Also, as is, L596 on to get non P-IOPS data isn't correct, as the key hasn't been made regional.
There was a problem hiding this comment.
@mda590 as discussed I'll go ahead and merge this and follow it with a fix to support non us-east1 regions. Thank you for this!
| @@ -489,7 +515,7 @@ func (aws *AWS) DownloadPricingData() error { | |||
| } | |||
| aws.ValidPricingKeys[key] = true | |||
| aws.ValidPricingKeys[spotKey] = true | |||
| } else if strings.HasPrefix(product.Attributes.UsageType, "EBS:Volume") { | |||
| } else if strings.Contains(product.Attributes.UsageType, "EBS:Volume") { | |||
| key := locationToRegion[product.Attributes.Location] + "," + product.Attributes.UsageType | |||
There was a problem hiding this comment.
I'd suggest setting the key on L519 here to the stripped value, as the lookup further down on L354 looks up based on pvk.Features(), which returns a key of the format "region,usage type without region". This seems like a cleaner approach, as then there's no need to reconstruct a regional P-IOPS key when you reconstruct the keys on L581.
Also, as is, L596 on to get non P-IOPS data isn't correct, as the key hasn't been made regional.
CostPerIOfield in thePVobject to handleio1volumes which have a per GB price + a per provisioned IO priceTested in live AWS EKS environment and everything looked good. I am noticing certain cases where the
CostPerIOfield forio1volume types is not being filled out - let me know if you have any ideas on that.