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

Upgrade examples to terraform-google-vm and add test automation #17

Merged

Conversation

zefdelgadillo
Copy link
Contributor

@zefdelgadillo zefdelgadillo commented Sep 25, 2019

  • Upgrade module to work with terraform-google-modules/terraform-google-vm
  • Upgrade simple example to use managed instance group in terraform-google-modules/terraform-google-vm
  • Replace existing test automation with new test automation using kitchen terraform
  • Write a test to hit endpoint created
  • Review README/documentation (having trouble getting make generate_docs to work correctly)

@morgante at long last, here's the (significant) PR that includes the change to use the new terraform-google-vm managed instance group examples. This included some refactor of the module in addition to adding the example. I also added the test automation that we have on the other newer modules, with a simple test for now that checks for the presence of the forwarding rule. We might want to include a test that checks for the presence of the backend.

I'll squash commits on an approval

closes #12
closes #14

@aaron-lane aaron-lane added the enhancement New feature or request label Sep 25, 2019
README.md Outdated Show resolved Hide resolved
examples/basic/main.tf Show resolved Hide resolved
examples/simple/main.tf Outdated Show resolved Hide resolved
examples/simple/main.tf Outdated Show resolved Hide resolved
examples/simple/main.tf Outdated Show resolved Hide resolved
test/fixtures/shared/network.tf Outdated Show resolved Hide resolved
test/fixtures/shared/terraform.tfvars.example Outdated Show resolved Hide resolved
test/fixtures/simple/variables.tf Outdated Show resolved Hide resolved
test/setup/make_source.sh Outdated Show resolved Hide resolved
test/task_helper_functions.sh Outdated Show resolved Hide resolved
@zefdelgadillo
Copy link
Contributor Author

Requested changes I think should be resolved, and I opened a few additional issues where there are external dependencies. Added an additional test for presence of load balancer endpoint that gets created. Also updated README documentation to the same standards as some of the other CFT modules.

I'll touch base separately about where I am on adding CI.

@zefdelgadillo
Copy link
Contributor Author

The integration test I had to test the presence of the load balancer by hitting the endpoint fails for what it seems like the failure of the docker build container to access the internet. The basic functionality of the load balancer in GCP tests for the presence of the backend, but I think to complete an integration test we'd want to test the presence of the front end and forwarding rules.

Are we doing any tests for Internet-facing endpoints like this anywhere else?

control "http" do
  title "http"
    describe http("http://#{attribute("load_balancer_ip")}") do
      its('status') { should eq 200 }
  end
end
Step #3 - "verify":   ×  http: http
Step #3 - "verify":      ×  HTTP GET on http://34.70.209.72 status 
Step #3 - "verify":      execution expired

@Jberlinsky
Copy link

@zefdelgadillo We had some full integration tests like what you're describing in other modules, but they've since (to my knowledge, entirely) been removed or commented out because they were "flappy" -- even when they worked, there was no way to guarantee they worked consistently.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
test/integration/basic/controls/gcloud.rb Show resolved Hide resolved
@zefdelgadillo
Copy link
Contributor Author

@morgante or @Jberlinsky thank you both again for the reviews. would like to get a final once over and an approval if all looks good

README.md Show resolved Hide resolved
examples/basic/README.md Show resolved Hide resolved
examples/basic/variables.tf Outdated Show resolved Hide resolved
examples/basic/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

Some tiny nits, almost there.

CHANGELOG.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
…ed testing

- Add integration tests
- Add lint tests
- Use terraform-google-vm for basic example
- Update changelog
@zefdelgadillo zefdelgadillo requested a review from morgante October 6, 2019 16:01
@zefdelgadillo zefdelgadillo changed the title [WIP] Upgrade examples to terraform-google-vm and add test automation Upgrade examples to terraform-google-vm and add test automation Oct 7, 2019
@morgante morgante merged commit 3b8078e into terraform-google-modules:master Oct 7, 2019
@zefdelgadillo zefdelgadillo mentioned this pull request Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants