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

Add support for using a GCP Image Family #8083

Merged
merged 1 commit into from
Aug 15, 2016
Merged

Add support for using a GCP Image Family #8083

merged 1 commit into from
Aug 15, 2016

Conversation

cblecker
Copy link
Contributor

@cblecker cblecker commented Aug 9, 2016

Please excuse any mistakes in this as it's my first attempt at a PR for Terraform :)

When I dug into #7562 a bit more, it looked easier than I initially thought to use the image family api instead of the regular image api.

I haven't updated the documentation or anything yet, but I wanted to ensure I'm on the right track. I tested this with my terraform config, and it seemed to work as expected.

@cblecker
Copy link
Contributor Author

I figured out how to run the acceptance tests, saw they weren't working properly, and then fixed them. I also had to modify the test for these image families, as Google doesn't have/support a Debian 7 image family.

$ make testacc TEST=./builtin/providers/google TESTARGS='-run=TestAccComputeInstance_basic[1-5]'
==> Checking that code complies with gofmt requirements...
/opt/gopath/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/09 20:56:01 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/google -v -run=TestAccComputeInstance_basic[1-5] -timeout 120m
=== RUN   TestAccComputeInstance_basic1
--- PASS: TestAccComputeInstance_basic1 (82.49s)
=== RUN   TestAccComputeInstance_basic2
--- PASS: TestAccComputeInstance_basic2 (72.63s)
=== RUN   TestAccComputeInstance_basic3
--- PASS: TestAccComputeInstance_basic3 (90.19s)
=== RUN   TestAccComputeInstance_basic4
--- PASS: TestAccComputeInstance_basic4 (72.50s)
=== RUN   TestAccComputeInstance_basic5
--- PASS: TestAccComputeInstance_basic5 (72.59s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/google 390.406s

Side note: Should the other acceptance tests in this project be updated to use Debian 8 too?

@cblecker
Copy link
Contributor Author

I've squashed the commits and rebased. Removing WIP tag as I think this might be ready for review/merge.

@cblecker cblecker changed the title [WIP] Add support for using a GCP Image Family Add support for using a GCP Image Family Aug 10, 2016
@stack72
Copy link
Contributor

stack72 commented Aug 15, 2016

@evandbrown would love your thoughts on this PR :)

@evandbrown
Copy link
Contributor

This is awesome. Thanks, @cblecker!

Only one nit: mind adding a comment to the resolveImage func that lists ImageFamily as one of the possibilities?

After that, LGTM. Thanks for taking the time to contribute solid, useful stuff to the Google provider, Christoph!

@cblecker
Copy link
Contributor Author

@evandbrown -- Updated the comments to be a bit more clear. Mind taking another look?

@evandbrown
Copy link
Contributor

LGTM! @stack72, cool if push the merge button on this one? :)

@stack72
Copy link
Contributor

stack72 commented Aug 15, 2016

done! Thanks @cblecker :)

@stack72 stack72 merged commit 8416258 into hashicorp:master Aug 15, 2016
@cblecker cblecker deleted the gcp-image-family branch August 15, 2016 21:30
@loliee
Copy link

loliee commented Aug 16, 2016

awesome @cblecker !

kwilczynski pushed a commit to kwilczynski/terraform that referenced this pull request Aug 18, 2016
@ghost
Copy link

ghost commented Apr 23, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants