Add support to filter instances by price range, and expose the prices/spot prices in output

[krishna-birla]

It would be great if the support is added to filter out instances based on live price range. Also, to show the price of the instances (both spot and on-demand) in the output.

[advissor]

I think price column in output could be very handy. We can get this info from pricing api from aws
Cause, even with same specs some instances can be twice more expensive

[bwagner5]

This has been merged and can be built from source for testing. I'm going to hold off on doing an official release because we'll have to increment the major version to v3 because of some lib changes that were required to support the pricing retrieval changes.

[krishna-birla]

Thanks a lot! @bwagner5

[bwagner5]

Would love to hear feedback here!

[krishna-birla]

A few things. First the nit picks:

1. In --help text, mention that accepted price is in USD.

Unexpected behaviour:

1. Can't get On-Demand pricing for us-west-1, us-west-2, us-east-2 upon multiple retries. Interestingly, us-west-1 works fine all the time. My credentials have admin privileges, so it is not an IAM issue.

Krishna-Birla:amazon-ec2-instance-selector krishna.birla$ ./build/ec2-instance-selector --region us-east-2 --max-results 5 -o table-wide --price-per-hour-max 0.2
Could not retrieve hourly price for instance type r4.16xlarge
Could not retrieve hourly price for instance type m5a.16xlarge
Could not retrieve hourly price for instance type t2.2xlarge
Could not retrieve hourly price for instance type m5d.8xlarge
Could not retrieve hourly price for instance type m6g.metal
Could not retrieve hourly price for instance type m6gd.medium
Could not retrieve hourly price for instance type r5dn.8xlarge
Could not retrieve hourly price for instance type c6gn.4xlarge
...

Krishna-Birla:amazon-ec2-instance-selector krishna.birla$ ./build/ec2-instance-selector --region us-west-1 --max-results 5 -o table-wide --price-per-hour-max 0.2
Could not retrieve hourly price for instance type c5a.xlarge
Could not retrieve hourly price for instance type g4dn.12xlarge
Could not retrieve hourly price for instance type r5ad.12xlarge
Could not retrieve hourly price for instance type m5.xlarge
Could not retrieve hourly price for instance type c4.large
^C

Krishna-Birla:amazon-ec2-instance-selector krishna.birla$ ./build/ec2-instance-selector --region us-west-2 --max-results 5 -o table-wide --price-per-hour-max 0.2
Could not retrieve hourly price for instance type c6g.xlarge
Could not retrieve hourly price for instance type m5dn.12xlarge
Could not retrieve hourly price for instance type r6g.medium
Could not retrieve hourly price for instance type r6g.8xlarge
Could not retrieve hourly price for instance type c4.4xlarge
Could not retrieve hourly price for instance type c6gn.xlarge
Could not retrieve hourly price for instance type i3en.24xlarge
Could not retrieve hourly price for instance type c6g.12xlarge
Could not retrieve hourly price for instance type m5d.16xlarge
Could not retrieve hourly price for instance type c3.8xlarge
Could not retrieve hourly price for instance type r5n.12xlarge
Could not retrieve hourly price for instance type inf1.xlarge
Could not retrieve hourly price for instance type c5n.large
Could not retrieve hourly price for instance type m5a.16xlarge
...

Krishna-Birla:amazon-ec2-instance-selector krishna.birla$ ./build/ec2-instance-selector --region us-east-1 --max-results 5 -o table-wide --price-per-hour-max 0.2
Instance Type  VCPUs   Mem (GiB)  Hypervisor  Current Gen  Hibernation Support  CPU Arch      Network Performance  ENIs    GPUs    GPU Mem (GiB)  GPU Info  On-Demand Price/Hr
-------------  -----   ---------  ----------  -----------  -------------------  --------      -------------------  ----    ----    -------------  --------  ------------------
a1.large       2       4          nitro       false        false                arm64         Up to 10 Gigabit     3       0       0                        $0.051
a1.medium      1       2          nitro       false        false                arm64         Up to 10 Gigabit     2       0       0                        $0.0255
a1.xlarge      4       8          nitro       false        false                arm64         Up to 10 Gigabit     4       0       0                        $0.102
c1.medium      2       1.69922    xen         false        false                i386, x86_64  Moderate             2       0       0                        $0.13
c3.large       2       3.75       xen         false        true                 i386, x86_64  Moderate             3       0       0                        $0.105
NOTE: 81 entries were truncated, increase --max-results to see more

Suggestions

1. Display On-Demand/Spot pricing for all table-wide outputs

In current code, we only hydrate the cache if --price-per-hour filter is set. Can we change this behaviour, and use --usage-class instead to hydrate the respective cache. I know this comes at the expense of startup time for hydrating the cache, but there is a strong use case here:

For any AWS user, generally price is the "deciding" factor but not the "limiting" factor. The most common use case IMO is: I have a list of EC2 instances as per my vCPU, memory, ENI, I/O and other hardware requirements; now I have a list of instances I can use and I want to choose the cheapest one. For that I need the prices listed.

The price filter is for people who already have some idea about how AWS prices their instances.

If you approve of this change, I can give a PR right away.

[bwagner5]

Thanks so much for taking the time to try it out and write this feedback!

I'll investigate the regions not working, thanks for bringing that up.

I'm fine with doing price retrieval when --usage-class is specified too. Just to clarify, you mean retrieve pricing on usage-class and price-per-hour filter right (not just on usage-class) ? Feel free to raise a PR for that!

[krishna-birla]

I'm fine with doing price retrieval when --usage-class is specified too. Just to clarify, you mean retrieve pricing on usage-class and price-per-hour filter right (not just on usage-class) ? Feel free to raise a PR for that!

I am suggesting to retrieve price in all cases of table wide output. (On-demand price for usageClass == nil || usageClass == on-demand and Spot price for usageClass == spot)

Current code (only hydrate if price per hour is given):

if pricePerHour != nil {
	if usageClass == nil || usageClass == on-demand {
		// Hydrate on demand cache
	} else {
		// Hydrate spot cache
	}
}

My suggestion (always hydrate):

if usageClass == nil || usageClass == on-demand {
	// Hydrate on demand cache
} else {
	// Hydrate spot cache
}

[krishna-birla]

Just like we list hypervisor, current gen, gpu, ENI, etc. we should always list price. Is that okay?

[bwagner5]

I see, I'm not opposed to figuring out a way that we could enable it all the time for table-wide output, but as it stands right now, hydrating the cache will still not expose it on the table. Currently the outputs know nothing about how the filtering works. Hydrating the cache will speed up price retrieval when calling the selector, but the selector will only retrieve pricing data if a pricing filter is present. So basically the change you're suggesting would not be sufficient to show the price on the table-wide view and if we make a further change in selector to fetch pricing data on every filter call, then it would slow down for all filters not just ones that are displayed w/ table-wide.

https://github.com/aws/amazon-ec2-instance-selector/blob/main/pkg/selector/selector.go#L189-L207

We may be able to refactor a bit to check if the cache is already hydrated and if it is, then go ahead and add pricing data... It feels little dirty to do that since we're inferring information about which output we're using to do further filter work (even if it's just attaching the data to the instanceType struct).

[krishna-birla]

but as it stands right now, hydrating the cache will still not expose it on the table.

Yes, ofc I meant not just this if condition but other relevant changes overall to make it possible. If you are okay with the startup time of hydration, I can add that in table-wide, make whatever change is needed. Should I start development then?

[krishna-birla]

So basically the change you're suggesting would not be sufficient to show the price on the table-wide view and if we make a further change in selector to fetch pricing data on every filter call, then it would slow down for all filters not just ones that are displayed w/ table-wide.

I am sure we can prevent this, so that if a user does not want table wide output, we do not hydrate. I can try.