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(TPG>=5.36)!: add confidential_instance_type to instance_template module #416

Conversation

arthurlapertosa
Copy link
Contributor

add confidential_instance_type to instance_template module, with example and tests

@arthurlapertosa arthurlapertosa marked this pull request as ready for review July 24, 2024 18:20
@arthurlapertosa arthurlapertosa requested a review from a team as a code owner July 24, 2024 18:20
@daniel-cit
Copy link

/gcbrun

@daniel-cit
Copy link

@apeabody Could you PTAL ?

Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @arthurlapertosa!

Please see the notes below:

modules/instance_template/variables.tf Outdated Show resolved Hide resolved
test/fixtures/confidential_instance_template/versions.tf Outdated Show resolved Hide resolved
modules/instance_template/main.tf Show resolved Hide resolved
@apeabody apeabody changed the title feat: add confidential_instance_type to instance_template module feat(TPG>=5.36)!: add confidential_instance_type to instance_template module Jul 31, 2024
@apeabody apeabody self-assigned this Jul 31, 2024
@arthurlapertosa arthurlapertosa requested a review from apeabody July 31, 2024 20:22
@apeabody
Copy link
Contributor

/gcbrun

@apeabody
Copy link
Contributor

FYI @arthurlapertosa

Checking for documentation generation
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=autogen' '--exclude=*.tfvars' '--exclude=*metadata.yaml' /workspace/modules/instance_template/README.md /tmp/tmp.4PKCCzT0D4/generate_docs/workspace/modules/instance_template/README.md
23c23
< | confidential\_instance\_type | Defines the confidential computing technology the instance uses. If this is set to "SEV\_SNP", var.min\_cpu\_platform will be automatically set to "AMD Milan". | `string` | `""` | no |
---
> | confidential\_instance\_type | Defines the confidential computing technology the instance uses. If this is set to "SEV\_SNP", var.min\_cpu\_platform will be automatically set to "AMD Milan". | `string` | `null` | no |
Error: Documentation generation has not been run, please run the
'make docker_generate_docs' command and commit the above changes.

@arthurlapertosa
Copy link
Contributor Author

FYI @arthurlapertosa

Checking for documentation generation
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=autogen' '--exclude=*.tfvars' '--exclude=*metadata.yaml' /workspace/modules/instance_template/README.md /tmp/tmp.4PKCCzT0D4/generate_docs/workspace/modules/instance_template/README.md
23c23
< | confidential\_instance\_type | Defines the confidential computing technology the instance uses. If this is set to "SEV\_SNP", var.min\_cpu\_platform will be automatically set to "AMD Milan". | `string` | `""` | no |
---
> | confidential\_instance\_type | Defines the confidential computing technology the instance uses. If this is set to "SEV\_SNP", var.min\_cpu\_platform will be automatically set to "AMD Milan". | `string` | `null` | no |
Error: Documentation generation has not been run, please run the
'make docker_generate_docs' command and commit the above changes.

Sorry, I forgot to update the documentation after changing the default value to null. Just did it.

@apeabody
Copy link
Contributor

/gcbrun

Copy link
Collaborator

@erlanderlo erlanderlo left a comment

Choose a reason for hiding this comment

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

SEV vs SEV_SNP

@arthurlapertosa
Copy link
Contributor Author

@apeabody could you please trigger the workflows?

@apeabody
Copy link
Contributor

apeabody commented Aug 5, 2024

/gcbrun

@apeabody
Copy link
Contributor

apeabody commented Aug 5, 2024

Thanks @arthurlapertosa! - Can you resolve the appropriate open comments?

@arthurlapertosa
Copy link
Contributor Author

@erlanderlo could you please take a look at the open comments?

Copy link
Collaborator

@erlanderlo erlanderlo left a comment

Choose a reason for hiding this comment

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

lgtm. one minor item

@arthurlapertosa
Copy link
Contributor Author

arthurlapertosa commented Aug 7, 2024

@apeabody all open comments addressed. Please take another look!

@apeabody
Copy link
Contributor

apeabody commented Aug 7, 2024

/gcbrun

@apeabody apeabody merged commit 1073c39 into terraform-google-modules:master Aug 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants