feat(aws_instance): add instance market options block

[haidargit]

what

• The PR enables the terraform instance_market_options block for this ec2 module
• add a new tfvars as a consumer/Terratest for the ec2 spot instance inside examples/complete directory

why

• Spot Instances can offer a great discount compared to On-Demand pricing based on AWS documentation

references

• there are no issues related to this PR, this PR is made for the ec2 module improvement

others:

• module documentation is updated (guide: Updating Module Documentation)

• update the versions.tf files in the example directories as a complement (guide: Updating the Module Examples)

• (done) terraform testing (plan+apply) is performed by using examples/complete directory on current PR branch feature/spot-instance-enablement targeting self-owned AWS cloud account

[haidargit]

Hello @hans-d & @jamengual

please review the PR above for the ec2 module improvement any input is welcome 🙂

Thank you

[mergify[bot]]

💥 This pull request now has conflicts. Could you fix it @haidargit? 🙏

[haidargit]

make init && make readme have been re-ran, @joe-niland, Thank you

[nitrocode]

That new example directory isn't getting run by terratest. Was adding the example strictly as a way of documenting the feature?

Here are some options worth considering

1. How come we can't make the existing tfvars test use the new input? This would be ideal.

2. If that's not possible, is it possible to add a new tfvars in the existing examples complete directory instead of adding a new terraform example directory? If so that would be advisable because it would be less code to maintain and update.

3. As-is (example directory, no automated test)

---

Here is the existing tfvars https://github.com/cloudposse/terraform-aws-ec2-instance/blob/main/examples/complete/fixtures.us-east-2.tfvars which could then have an additional tfvars beside it such as fixtures.us-east-2.spot.tfvars.

Here is how the tfvars get inserted into the test https://github.com/cloudposse/terraform-aws-ec2-instance/blob/c4be48095dbccb409a776cd5c08959467696c776/test/src/examples_complete_test.go#L27

[haidargit]

Hi @nitrocode, thanks for the insight

Correct, using the existing example directories is sufficient.

i have updated my code to use the current /complete test directory, these are the changes that may be applicable:

• i have created the fixtures.us-east-2.spot.tfvars as new tvfars. I just left the current tfvars as is inside the examples/complete directory, so the user/terratest able to provision a sample of an EC2 instance with the spot instance disabled. The new tvfars will introduce this new variable instance_market_options_enabled = true

• modified the test/src/examples_complete_test.go to include new tfvars and spot instance request ID output test

for the terratest reference, the output ID prefix is sir-

[nitrocode]

Thanks for addressing the changes.

How come the _enabled flag is still used? I thought it was removed when this comment was raised?

https://github.com/cloudposse/terraform-aws-ec2-instance/pull/202#discussion_r1719397421

[mergify[bot]]

💥 This pull request now has conflicts. Could you fix it @haidargit? 🙏

[nitrocode]

/terratest

[nitrocode]

@haidargit see code annotations and test failures

[nitrocode]

Thanks for resolving some of the issues.

There seem to be a few remaining. Please fix all of them and then resubmit the PR. I moved it to a draft for now.

[haidargit]

Hi @nitrocode, i adjusted the output.tf files on the spot instance part to use length function to fix the tflint warnings

Could you kindly review the latest commit when you get a chance? Much appreciated!

[haidargit]

@nitrocode, this was spot instance testing conducted by terraform apply using fixtures.us-east-2.spot.tfvars on /complete directory. The PR has also set the default to use normal EC2 instance provisioning (commit 0c3e3b3)

case 1:
spot instance enabled (market_type = "spot") and spot options declared (non empty spot_options_attributes)

case 2:
spot instance disabled (market_type = null).
As long as the spot instance is disabled (market_type = null), the spot_options_attributes variable will be ignored regardless of whether its declared or not.

[nitrocode]

I'll cc: @joe-niland here as I need help in reviewing the PR

[haidargit]

Hello @nitrocode, @joe-niland

I've updated the code. Could you help review the latest commit? c184687

the changes included the instance market options variable adjustment, custom validation for market type, and several other minor fix

Thank you 🙏🏻

[joe-niland]

/terratest

[nitrocode]

/terratest

[joe-niland]

/terratest

[mergify[bot]]

💥 This pull request now has conflicts. Could you fix it @haidargit? 🙏

[mergify[bot]]

This PR was closed due to inactivity and merge conflicts. 😭 Please resolve the conflicts and reopen if necessary.