Add Terratest Example for Compute Module #66

foreverXZC · 2018-07-09T02:08:51Z

This example uses Terratest to test compute module. All introduction and usages are included in Terratest/README.md.

* build test * build test * build test * build test * build test * build test * Add Terratest * Add Terratest * Add Terratest

foreverXZC · 2018-07-13T14:41:02Z

Removed kitchen terraform and added terratest as travis CI testing.
Trivial things still need to be modified, including readme as well as some pieces of code and configuration.

foreverXZC · 2018-07-16T08:00:24Z

After hiding environment variables, the build fails because the log demonstrates that encrypted environment variables have been removed for security reasons. Although it passes in my own repo, it seems impossible to pass here because pull request from a fork is considered not trustful and thus cannot use encrypted environment variables such as subscription ID, etc.

metacpp

Please address the issues in the PR. I would like to go through the left of this PR with you to save time in typing comment.

metacpp · 2018-07-18T22:31:56Z

.travis.yml

@@ -27,5 +27,5 @@ jobs:
      if: type = push AND branch = master
      install: true
      script:
-      - docker build --build-arg BUILD_TERRAFORM_VERSION=${TERRAFORM_VERSION} --build-arg BUILD_ARM_SUBSCRIPTION_ID=$ARM_SUBSCRIPTION_ID --build-arg BUILD_ARM_CLIENT_ID=$ARM_CLIENT_ID --build-arg BUILD_ARM_CLIENT_SECRET=$ARM_CLIENT_SECRET --build-arg BUILD_ARM_TENANT_ID=$ARM_TENANT_ID -t ${IMAGE_NAME} .
+      - docker build --build-arg BUILD_TERRAFORM_VERSION=${TERRAFORM_VERSION} --build-arg BUILD_ARM_SUBSCRIPTION_ID=$ARM_SUBSCRIPTION_ID --build-arg BUILD_ARM_CLIENT_ID=$ARM_CLIENT_ID --build-arg BUILD_ARM_CLIENT_SECRET=$ARM_CLIENT_SECRET --build-arg BUILD_ARM_TENANT_ID=$ARM_TENANT_ID -t ${IMAGE_NAME} . 


extra white space at the end of line.

Will delete.

metacpp · 2018-07-18T22:32:20Z

Dockerfile

@@ -1,5 +1,5 @@
 # Pull the base image with given version.
-ARG BUILD_TERRAFORM_VERSION="0.11.1"
+ARG BUILD_TERRAFORM_VERSION="0.11.3"


Should be 0.11.7 now.

However, 0.11.7 is not released in docker hub, which means if we set terraform version as 0.11.7 here, an error occurs showing that no such kind of docker image.

0.11.7 was pushed, go ahead to use it.

metacpp · 2018-07-18T22:33:27Z

Dockerfile

 RUN ssh-keygen -q -t rsa -b 4096 -f $HOME/.ssh/id_rsa

-WORKDIR /usr/src/${MODULE_NAME}
+# Install new version of terraform and golang


It's already contained in base image, we don't need it any more.

Will remove.

Can you remove them?

Yes after I guarantee that the new version of docker image runs properly,

It does not matter coz I will not merge it until you verified everything from E2E.

metacpp · 2018-07-18T22:38:43Z

Dockerfile

+RUN mv terraform /usr/local/bin
+
+# Install required go packages
+ENV GOPATH $HOME/terratest/ssh


The environment variables related to Go should be set while baking base image.

Will put in base image.

metacpp · 2018-07-18T22:40:36Z

README.md


-Deploys 1+ Virtual Machines to your provided VNet
-=================================================
+[![Build Status](https://travis-ci.org/foreverXZC/terraform-azurerm-compute.svg?branch=master)](https://travis-ci.org/foreverXZC/terraform-azurerm-compute)


The badge should not be yours. Please keep the original one.

Will change.

Can you please change it?

Yes after it passes the build.

Validation should always happen after you make any change. And merge will never happen if you don't do any validation.

metacpp · 2018-07-18T22:43:17Z

README.md

 ```

 ### Docker

 We provide a Dockerfile to build a new image based `FROM` the `microsoft/terraform-test` Docker hub image which adds additional tools / packages specific for this module (see Custom Image section).  Alternatively use only the `microsoft/terraform-test` Docker hub image [by using these instructions](https://github.com/Azure/terraform-test).

-#### Prerequisites
+#### Prerequisites (Docker)


It's already under Docker section, don't need to have Docker any more.

VS Code considers it as the same heading as the previous one. Thus it thinks the heading is duplicated.

OK I will ignore that.

metacpp · 2018-07-18T22:44:16Z

README.md

@@ -161,6 +159,7 @@ We provide 2 ways to build, run, and test the module on a local development mach
 ### Native (Mac/Linux)

 #### Prerequisites
+
 - [Ruby **(~> 2.3)**](https://www.ruby-lang.org/en/downloads/)
 - [Bundler **(~> 1.15)**](https://bundler.io/)
 - [Terraform **(~> 0.11.0)**](https://www.terraform.io/downloads.html)


Will change.

metacpp · 2018-07-18T22:44:28Z

README.md

@@ -161,6 +159,7 @@ We provide 2 ways to build, run, and test the module on a local development mach
 ### Native (Mac/Linux)

 #### Prerequisites
+
 - [Ruby **(~> 2.3)**](https://www.ruby-lang.org/en/downloads/)
 - [Bundler **(~> 1.15)**](https://bundler.io/)
 - [Terraform **(~> 0.11.0)**](https://www.terraform.io/downloads.html)


metacpp · 2018-07-18T22:45:16Z

README.md

@@ -170,21 +169,22 @@ We provide 2 ways to build, run, and test the module on a local development mach
 We provide simple script to quickly set up module development environment:

 ```sh
-$ curl -sSL https://raw.githubusercontent.com/Azure/terramodtest/master/tool/env_setup.sh | sudo bash
+curl -sSL https://raw.githubusercontent.com/Azure/terramodtest/master/tool/env_setup.sh | sudo bash


Did you verify if this script needs to be updated for new testing environment?

Yes the script should be modified if we introduce Go SDK.

Unfortunately, "export" commands in script only affects environment variables inside the script, which means users have to set global environment variables by themselves. Will add export commands here.

If that's the case, you can try source command in linux.

metacpp · 2018-07-18T22:46:55Z

Rakefile

-  end
-  task :destroy do
-    kitchen_destroy
+    success = system ("go test -v terratest/ssh/terraform_ssh_example_test.go -timeout 20m -args azureuser ~/.ssh/id_rsa")


test_folder_name/**/*_test.go?

It seems that this format using * does not work...

metacpp · 2018-07-19T03:55:51Z

terratest/README.md

@@ -0,0 +1,57 @@
+# Compute Example


Please merge it with README in root folder, remove unnecessary content like how to install go and terraform.

Yes after I ensure that the script runs correctly,

metacpp · 2018-07-19T03:56:21Z

terratest/compute/main.tf

@@ -0,0 +1,41 @@
+provider "random" {


Put all the tf code for testing into ./test/fixture

Will change.

metacpp · 2018-07-19T03:56:41Z

terratest/ssh/public_key_file_deprecated.go

+package test
+
+import (
+	"io/ioutil"


Use 2 whitespaces as indention.

OK will modify. Nevertheless, although most programming languages use a tab or four white spaces as indentation, terraform format uses two white spaces, which seems quite interesting but a bit weird.

That's not true. Many languages(C++, JS) uses 2 whitespaces. Anyway, let's be consistent here.

metacpp · 2018-07-19T03:57:22Z

@foreverXZC please remove the .testdata folder and add it into .gitignore.

metacpp · 2018-07-26T06:36:56Z

Dockerfile

@@ -1,5 +1,5 @@
 # Pull the base image with given version.
-ARG BUILD_TERRAFORM_VERSION="0.11.1"
+ARG BUILD_TERRAFORM_VERSION="0.11.3"


0.11.7 was pushed, go ahead to use it.

metacpp · 2018-07-26T06:38:14Z

Dockerfile

 RUN ssh-keygen -q -t rsa -b 4096 -f $HOME/.ssh/id_rsa

-WORKDIR /usr/src/${MODULE_NAME}
+# Install new version of terraform and golang


Can you remove them?

metacpp · 2018-07-26T06:41:39Z

Dockerfile

+RUN mv terraform /usr/local/bin
+
+# Install required go packages
+ENV GOPATH $HOME/go


You should remove the steps contained in base image.

Will remove.

metacpp · 2018-07-26T06:42:00Z

Dockerfile

+# Install required go packages
+ENV GOPATH $HOME/go
+ENV PATH /usr/local/go/bin:$PATH
+RUN /bin/bash -c "go get github.com/gruntwork-io/terratest/modules/ssh"


these pkgs should be restored through go dep.

Unfortunately we do not have go dep installed in base image, so now maybe I have to install go dep here in Dockerfile.

You can install dep in this Dockerfile, that's OK. But you should remove the Go packages in Dockerfile, they should be restored by 'dep ensure'.

metacpp · 2018-07-26T06:42:36Z

README.md


-Deploys 1+ Virtual Machines to your provided VNet
-=================================================
+[![Build Status](https://travis-ci.org/foreverXZC/terraform-azurerm-compute.svg?branch=master)](https://travis-ci.org/foreverXZC/terraform-azurerm-compute)


Can you please change it?

metacpp · 2018-07-26T06:48:57Z

Rakefile

-  end
-  task :destroy do
-    kitchen_destroy
+    success = system ("go test -v ./test/compute/ -timeout 20m -args azureuser ~/.ssh/id_rsa")


./test should be enough here, compute subfolder is meaningless in compute project.

OK I will move these go files outside.

metacpp · 2018-07-26T06:49:46Z

test/README.md

@@ -0,0 +1,61 @@
+# Compute Example


Why did you still keep this README? You should remove merge the testing related part into README.md under root folder.

The main README seems enough, so now I think this one can be removed.

metacpp · 2018-07-26T06:50:55Z

test/compute/Gopkg.lock

@@ -0,0 +1,158 @@
+# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'.


Please don't use compute folder under test folder, this project is for compute only.

Yes will move go files outside.

metacpp · 2018-07-26T06:57:37Z

test/compute/Gopkg.lock

@@ -0,0 +1,158 @@
+# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'.
+
+


Gopkg.lock and Gopkg.toml can be directly put under root folder.

Makes sense.

metacpp · 2018-07-26T06:58:15Z

.gitignore

@@ -9,7 +9,7 @@ terraform.tfvars

 # Module directory
 .terraform/
-
+.test-data/


Missed vendor folder.

metacpp

LGTM.

metacpp · 2018-07-27T22:44:07Z

@foreverXZC let me know if e2e validation is successful.

foreverXZC · 2018-07-28T08:25:28Z

After I merged, the build passed.

foreverXZC added 2 commits July 9, 2018 09:55

Add Terratest Example for Compute Module

ebb6cf4

Add Terratest Example for Compute Module

32a231c

JunyiYi added the review label Jul 9, 2018

foreverXZC added 3 commits July 9, 2018 10:20

style problem

6cc3cb8

style problem

b280410

style problem

f66f021

metacpp self-requested a review July 10, 2018 00:44

build test (#2)

c714ed1

* build test * build test * build test * build test * build test * build test * Add Terratest * Add Terratest * Add Terratest

foreverXZC added 4 commits July 16, 2018 14:26

Update Files

cb199d0

Hide Environment Variables

53434c0

Allow Pull Requests

ae08618

Update Files

625ffa2

foreverXZC and others added 3 commits July 18, 2018 09:36

rake build

b3a916f

Merge branch 'master' into master

0711017

Update .travis.yml

9c07cf2

metacpp suggested changes Jul 18, 2018

View reviewed changes

metacpp reviewed Jul 19, 2018

View reviewed changes

foreverXZC added 6 commits July 19, 2018 17:50

Required Changes

fdbfd38

Required Changes

64ba0c1

Update README

83526a0

Add dep ensure

32b085c

Add dep init

fd150c7

Update README

eb49a27

metacpp suggested changes Jul 26, 2018

View reviewed changes

Update Docker Image

ce59e9c

foreverXZC added 3 commits July 26, 2018 17:21

Use dep ensure in Dockerfile

24e50ca

Dep Ensure in Rakefile

8490781

Update README

7ddfd6a

metacpp approved these changes Jul 27, 2018

View reviewed changes

foreverXZC merged commit 627c6c2 into Azure:master Jul 28, 2018

		@@ -0,0 +1,158 @@
		# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'.

Add Terratest Example for Compute Module #66

Add Terratest Example for Compute Module #66

Conversation

foreverXZC commented Jul 9, 2018

foreverXZC commented Jul 13, 2018

foreverXZC commented Jul 16, 2018

metacpp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metacpp commented Jul 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metacpp left a comment

Choose a reason for hiding this comment

metacpp commented Jul 27, 2018

foreverXZC commented Jul 28, 2018