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

[joe-niland]

Looks good @haidargit - a couple of comments

[mergify]

💥 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

terraform-aws-ec2-instance/test/src/examples_complete_test.go

Line 27 in c4be480

VarFiles: []string{"fixtures.us-east-2.tfvars"},

[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?

#202 (comment)

[mergify]

💥 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]

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]

It's failing on this condition

TestExternalEniComplete 2024-10-15T22:07:13Z logger.go:66: │ Error: Attempt to get attribute from null value
TestExternalEniComplete 2024-10-15T22:07:13Z logger.go:66: │ 
TestExternalEniComplete 2024-10-15T22:07:13Z logger.go:66: │   on ../../variables.tf line 65, in variable "instance_market_options":
TestExternalEniComplete 2024-10-15T22:07:13Z logger.go:66: │   65:     condition     = contains(["spot", "capacity-block"], var.instance_market_options.market_type)
TestExternalEniComplete 2024-10-15T22:07:13Z logger.go:66: │     ├────────────────
TestExternalEniComplete 2024-10-15T22:07:13Z logger.go:66: │     │ var.instance_market_options is null
TestExternalEniComplete 2024-10-15T22:07:13Z logger.go:66: │ 
TestExternalEniComplete 2024-10-15T22:07:13Z logger.go:66: │ This value is null, so it does not have any attributes.
TestExternalEniComplete 2024-10-15T22:07:13Z logger.go:66: ╵
TestExternalEniComplete 2024-10-15T22:07:13Z retry.go:99: Returning due to fatal error: FatalError{Underlying: error while running command: exit status 1; ╷
│ Error: Attempt to get attribute from null value
│ 
│   on ../../variables.tf line 65, in variable "instance_market_options":
│   65:     condition     = contains(["spot", "capacity-block"], var.instance_market_options.market_type)
│     ├────────────────
│     │ var.instance_market_options is null
│ 
│ This value is null, so it does not have any attributes.
╵}
    destroy.go:11: 
        	Error Trace:	destroy.go:11
        	            				panic.go:523
        	            				testing.go:999
        	            				apply.go:15
        	            				examples_complete_test.go:109
        	Error:      	Received unexpected error:
        	            	FatalError{Underlying: error while running command: exit status 1; ╷
        	            	│ Error: Attempt to get attribute from null value
        	            	│ 
        	            	│   on ../../variables.tf line 65, in variable "instance_market_options":
        	            	│   65:     condition     = contains(["spot", "capacity-block"], var.instance_market_options.market_type)
        	            	│     ├────────────────
        	            	│     │ var.instance_market_options is null
        	            	│ 
        	            	│ This value is null, so it does not have any attributes.
        	            	╵}
        	Test:       	TestExternalEniComplete

[haidargit]

Hi @nitrocode,

i have put the fix for the error and tried to run the terraform plan on examples/external-eni in my local, it succeeded.

please review for my latest changes. Thanks for the feedback!

[nitrocode]

/terratest

[joe-niland]

/terratest