amazonec2: send no MaxPrice for an empty spot price

José M. Requena Plensrequested to merge
jmrp/docker-machine:jmrp-docker-machine-empty-spot-price into main

⚠️ NOTE THAT THIS FORK IS MAINTAINED FOR CRITICAL BUG FIXES AFFECTING RUNNING COSTS ONLY. NO OTHER CONTRIBUTIONS WILL BE ACCEPTED. ⚠️

What critical bug this MR is fixing?

With --amazonec2-request-spot-instance and an empty --amazonec2-spot-price=, no spot instance is created. Since !100 (merged) moved spot instances from RequestSpotInstances to RunInstances, the driver always sets SpotOptions.MaxPrice to the spot price, so an empty price goes out as InstanceMarketOptions.SpotOptions.MaxPrice= with no value, and AWS refuses the call:

InvalidParameterValue: The value '' is invalid for parameter MaxPrice

The GitLab Runner documentation gives exactly this setting as the way to leave the maximum price to AWS.

This MR sets MaxPrice only when the spot price is not empty. A configured price is sent as before, and the 0.50 default of the flag does not change. With no MaxPrice, the instance is charged the current Spot price, as AWS's SpotMarketOptions reference says.

Closes #151

How does this change help reduce cost of usage? What scale of cost reduction is it?

A runner configured this way gets no machines today, because every create fails, and the workarounds are a fixed bid or On-Demand instances. AWS describes Spot Instances as "savings of up to 90% compared to On-Demand prices" (Best practices for Amazon EC2 Spot), and leaving the maximum price unset is what AWS recommends, because a set maximum price leads to more interruptions. I have no measurement of my own for a given fleet: the saving is the gap between the On-Demand and Spot price of the instance type, for every machine the runner creates.

In what scenarios is this change usable with GitLab Runner's docker+machine executor?

Any [runners.machine] section that asks for spot instances with an empty spot price, as in the Runner documentation:

  [runners.machine]
    MachineDriver = "amazonec2"
    MachineOptions = [
      "amazonec2-request-spot-instance=true",
      "amazonec2-spot-price=",
    ]

Configurations that set a price, or that leave the option out and get the 0.50 default, send the same request as before.

Commits

The project merges without squashing, so the three commits stay as they are:

  1. amazonec2: test the spot options sent to RunInstances: a fake EC2 client that records the RunInstances input, and a test that a configured spot price is sent as MaxPrice with the market type spot. It passes on main.
  2. amazonec2: send no MaxPrice for an empty spot price: the fix, and the test case for an empty price, which fails on the first commit and passes here.
  3. docs: say what an empty --amazonec2-spot-price does: one sentence in docs/drivers/aws.md.

How I tested it

make build validate (the CI validate job's command) passes on each commit, run natively in golang:1.26.5 with rsync installed and USER set, which TestGetScpCmdWithDelta and TestGetUsername need. I have not run the driver against AWS: the refusal above is the one the issue reports, and what I checked locally is the request the SDK builds.

The new test on the first commit's code with the second commit's test file 
        	            	- MaxPrice: (*string)(<nil>),
        	            	+ MaxPrice: (*string)(""),
--- FAIL: TestSpotInstanceRequestMaxPrice (0.07s)
    --- PASS: TestSpotInstanceRequestMaxPrice/spot_price_is_sent_as_the_maximum_price (0.05s)
    --- FAIL: TestSpotInstanceRequestMaxPrice/empty_spot_price_sends_no_maximum_price (0.03s)
FAIL
FAIL	github.com/docker/machine/drivers/amazonec2	0.078s

With the fix:

--- PASS: TestSpotInstanceRequestMaxPrice (0.05s)
    --- PASS: TestSpotInstanceRequestMaxPrice/spot_price_is_sent_as_the_maximum_price (0.02s)
    --- PASS: TestSpotInstanceRequestMaxPrice/empty_spot_price_sends_no_maximum_price (0.03s)
ok  	github.com/docker/machine/drivers/amazonec2	0.055s
What aws-sdk-go v1.55.7 puts in the RunInstances query for each MaxPrice

Built with svc.RunInstancesRequest(...) and req.Build(), nothing sent, spot options only:

MaxPrice: aws.String("") (before):
  InstanceMarketOptions.MarketType="spot"
  InstanceMarketOptions.SpotOptions.MaxPrice=""
MaxPrice: nil (after):
  InstanceMarketOptions.MarketType="spot"
MaxPrice: aws.String("0.25"):
  InstanceMarketOptions.MarketType="spot"
  InstanceMarketOptions.SpotOptions.MaxPrice="0.25"

Merge request reports