Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implementation of the cpu_options block and addition of support for AMD SEV-SNP #334

Merged
merged 8 commits into from
May 30, 2023

Conversation

schniber
Copy link
Contributor

@schniber schniber commented May 6, 2023

Description

I have implemented the fix for the deprecation warning of cpu_core_count and cpu_threads_per_core attributes. Those can now be specified using the cpu_options block.

In addition, this PR adds the support for AMD SEV-SNP which is also a new attribute that can be set via the cpu_options block.

Motivation and Context

Breaking Changes

Yes, customers can no longer use the cpu_core_count and cpu_threads_per_core variables anymore.
Those have been removed in favor of a new variable cpu_options that can be used as below:

cpu_options = {
    core_count       = 2
    threads_per_core = 1
}

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

main.tf Outdated Show resolved Hide resolved
Copy link
Member

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets keep the current arguments since those are still supported by the provider and this would cause a breaking change here

@schniber schniber requested a review from bryantbiggs May 17, 2023 15:22
main.tf Outdated
@@ -177,11 +187,9 @@ resource "aws_instance" "this" {
resource "aws_instance" "ignore_ami" {
count = local.create && var.ignore_ami_changes && !var.create_spot_instance ? 1 : 0

ami = try(coalesce(var.ami, nonsensitive(data.aws_ssm_parameter.this[0].value)), null)
instance_type = var.instance_type
cpu_core_count = var.cpu_core_count
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these need to be re-added back

main.tf Outdated
@@ -341,11 +359,9 @@ resource "aws_instance" "ignore_ami" {
resource "aws_spot_instance_request" "this" {
count = local.create && var.create_spot_instance ? 1 : 0

ami = try(coalesce(var.ami, nonsensitive(data.aws_ssm_parameter.this[0].value)), null)
instance_type = var.instance_type
cpu_core_count = var.cpu_core_count
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same with these

@schniber schniber requested a review from bryantbiggs May 30, 2023 22:54
@bryantbiggs bryantbiggs merged commit 6a123ad into terraform-aws-modules:master May 30, 2023
antonbabenko pushed a commit that referenced this pull request May 30, 2023
## [5.1.0](v5.0.0...v5.1.0) (2023-05-30)

### Features

* Implementation of the cpu_options block and addition of support for AMD SEV-SNP ([#334](#334)) ([6a123ad](6a123ad))
@antonbabenko
Copy link
Member

This PR is included in version 5.1.0 🎉

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants