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.

Sign up for GitHub

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

Jump to bottom

Add support for min_cpu_platform in google_compute_instance. #349

Merged
merged 3 commits into from
Aug 30, 2017
Merged

Add support for min_cpu_platform in google_compute_instance. #349

merged 3 commits into from
Aug 30, 2017

Conversation

rosbo
Copy link
Contributor

@rosbo rosbo commented Aug 22, 2017

Fixes #106

@rosbo rosbo requested a review from selmanj August 22, 2017 20:15
selmanj
selmanj reviewed Aug 24, 2017
View reviewed changes
Copy link
Contributor

@selmanj selmanj left a comment

Choose a reason for hiding this comment

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

Wouldn't it also make sense to add cpu_platform as a computed attribute?

google/resource_compute_instance_test.go Outdated
func testAccCheckComputeInstanceHasMinCpuPlatform(instance *computeBeta.Instance, minCpuPlatform string) resource.TestCheckFunc {
return func(s *terraform.State) error {
if instance.MinCpuPlatform != minCpuPlatform {
return fmt.Errorf("Wrong minimum CPU platform: expected %d, got %d", minCpuPlatform, instance.MinCpuPlatform)
Copy link
Contributor

Choose a reason for hiding this comment

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

Go vet caught an error here about using %d instead of %s

rosbo
rosbo commented Aug 30, 2017
View reviewed changes
Copy link
Contributor Author

@rosbo rosbo left a comment

Choose a reason for hiding this comment

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

@selmanj It would not make sense to have min_cpu_platform computed because the value returned by the API when not specified is an empty string.

@selmanj
Copy link
Contributor

selmanj commented Aug 30, 2017

I meant expose cpu_platform as well rather than make min_cpu_platform computed

rosbo
rosbo commented Aug 30, 2017
View reviewed changes
Copy link
Contributor Author

@rosbo rosbo left a comment

Choose a reason for hiding this comment

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

Nice. I did not realize the cpu_platform field existed. Added!

@hamann
Copy link

hamann commented Aug 30, 2017 •
edited
Loading

Just for interest, where did you find cpu_platform? Or is that just a output param?

Edit - found it

cpuPlatform | string | [Output Only] The CPU platform used by this instance.

@rosbo rosbo merged commit c2399f7 into hashicorp:master Aug 30, 2017
@rosbo rosbo deleted the compute-instance-cpu-platform branch August 30, 2017 21:25
negz pushed a commit to negz/terraform-provider-google that referenced this pull request Oct 17, 2017
@rosbo
Add support for min_cpu_platform in google_compute_instance. (hashic…
f86ffc0 
…orp#349)
@munnerz
Copy link

munnerz commented Nov 15, 2017

I'm trying to create a managed instance group on GCE - I can't see any way to specify the min_cpu_platform on there however, either through the google_compute_instance_group_manager resource or google_compute_instance_template.

Am I missing something, or does this need updating on the resource type? 😄

@rosbo
Copy link
Contributor Author

rosbo commented Nov 15, 2017

This issue was about adding support for min_cpu_platform only for the google_compute_instance. Issue #516 is for adding it to google_compute_instance_template.

Once #639 is in. I will tackled #516

chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
@rosbo
Add support for min_cpu_platform in google_compute_instance. (hashic…
58293de 
…orp#349)
luis-silva pushed a commit to luis-silva/terraform-provider-google that referenced this pull request May 21, 2019
@modular-magician @rileykarson
Removing "input=true" from ignoredFiles and includedFiles (hashicorp#349
d988280 
)

<!-- This change is generated by MagicModules. -->
/cc @naseemkullah
@ghost
Copy link

ghost commented Mar 30, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@selmanj selmanj selmanj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add min_cpu_platform to google_compute_instance resource
4 participants
@rosbo @selmanj @hamann @munnerz