Helm integration test framework #237

yorinasub17 · 2019-02-17T00:22:46Z

This introduces helm integration tests by providing helm.Install and helm.Delete functions. Additionally, this extends the k8s module with additional functions that map better to helm. Specifically, when you install a chart, it is not obvious what name is chosen for the chart. However, you are always guaranteed to have a bunch of labels defined on the resources, so it is more common to query Kubernetes resources using the labels than directly by name with helm.

Here is the list of new functions introduced:

helm.Install: Install a helm chart onto a Kubernetes cluster
helm.Delete: Delete a deployed release from Tiller
k8s.ListPods: List pods in a given namespace
k8s.ListServices: List services in a given namespace
k8s.WaitUntilNumPodsCreated: Waits until the number of pods matching the given filters reaches the desired count.
k8s.Tunnel: A struct that can be used to create and manage port forward tunnel to a Pod.

Finally, I had to further separate out the helm tests because Tiller overloaded minikube when running with the kubernetes tests.

yorinasub17 · 2019-02-17T00:24:01Z

modules/helm/cmd.go

+// RunHelmCommandAndGetOutputE runs helm with the given arguments and options and returns stdout/stderr.
+func RunHelmCommandAndGetOutputE(t *testing.T, options *Options, cmd string, additionalArgs ...string) (string, error) {
+ args := []string{cmd}
+ args = getCommonArgs(options, args...)


This refactor was necessary because only install and upgrade take the --set, --set-file, and -f options.

brikis98 · 2019-02-18T11:52:57Z

.circleci/config.yml

@@ -223,10 +269,18 @@ workflows:
 tags:
 only: /^v.*/

+ - helm_test:


Will this run in parallel?

Yes. See https://circleci.com/workflow-run/3daf8d7f-f7fe-4daa-a1a7-0679026b0a7e

brikis98 · 2019-02-18T11:54:06Z

examples/helm-basic-example/README.md

+**NOTE**: we have build tags to differentiate kubernetes tests from non-kubernetes tests, and further differentiate helm
+tests. This is done because minikube is heavy and can interfere with docker related tests in terratest. Similarly, helm
+can overload the minikube system and thus interfere with the other kubernetes tests. To avoid overloading the system, we
+run the kubernetes tests and helm tests separately from the others.


What does "overload the system" mean here? Too much CPU or memory consumption? What are the symptoms? Is it just overloading it in CircleCI, and would work OK on larger servers?

I'll update the comment, but here is an example failing test: https://circleci.com/gh/gruntwork-io/terratest/869

Of the 28 tests, 19 fail. They all fail with:

Post https://10.142.2.94:8443/apis/authorization.k8s.io/v1/selfsubjectaccessreviews: dial tcp 10.142.2.94:8443: connect: connection refused

This indicates that either:

minikube crashed

minikube failed to respond

I don't think this would be a problem on beefy machines, as this does NOT happen when I run the kubernetes+helm tests locally, so I believe this is an issue with CircleCI. My best guess is memory. The executors have 8GB of RAM, but my laptop is 16GB. I don't think it's CPU because I am on a 13inch no touchbar model, which is a dual core machine.

New verbiage:

// NOTE: we have build tags to differentiate kubernetes tests from non-kubernetes tests. This is done because minikube // is heavy and can interfere with docker related tests in terratest. Specifically, many of the tests start to fail with // `connection refused` errors from `minikube`. To avoid overloading the system, we run the kubernetes tests and helm // tests separately from the others. This may not be necessary if you have a sufficiently powerful machine. We // recommend at least 4 cores and 16GB of RAM if you want to run all the tests together.

brikis98 · 2019-02-18T11:55:22Z

modules/helm/install.go

+// Install will install the selected helm chart with the provided options under the given release name. This will fail
+// the test if there is an error.
+func Install(t *testing.T, options *Options, chartDir string, releaseName string) {
+ require.NoError(t, InstallE(t, options, chartDir, releaseName))


I like this clean, one-line approach 👍

brikis98 · 2019-02-18T11:56:06Z

modules/k8s/kubectl_options.go

+
+// GetConfigPath will return a sensible default if the config path is not set on the options.
+func (kubectlOptions *KubectlOptions) GetConfigPath(t *testing.T) (string, error) {
+ var err error


Why declare this here, separately?

I'll add comment, but this is so that I can update kubeConfigPath in the if block below. Otherwise, it complains saying err is undefined.

brikis98 · 2019-02-18T11:57:08Z

modules/k8s/pod.go

+ t,
+ statusMsg,
+ 60,
+ 1*time.Second,


Shouldn't you use the passed-in retry settings?

Woops copy pasta from when I was experimenting in helm-kubernetes-services. Will fix.

brikis98 · 2019-02-18T11:58:44Z

modules/k8s/tunnel.go

+ return "svc"
+ default:
+ // This should not happen
+ return ""


Perhaps return a clear error message in the string?

brikis98 · 2019-02-18T12:00:31Z

modules/k8s/tunnel.go

+ ResourceName string
+ Out io.Writer
+ StopChan chan struct{}
+ ReadyChan chan struct{}


Are all these fields are used outside of this package (esp the channels)? If not, you can make them lowercase to make them private.

Ah ok. I didn't know that convention. Will update!

brikis98 · 2019-02-18T12:02:27Z

modules/k8s/tunnel.go

+
+// GetAvailablePortE retrieves an available port on the host machine. This delegates the port selection to the golang net
+// library by starting a server and then checking the port that the server is using.
+func GetAvailablePortE(t *testing.T) (int, error) {


If you run this in a concurrent context, couldn't a race condition lead to the same port being selected? i.e., goroutine one listens on port 12345, it's available, so it returns the port number, and in the defer stops listening. Right after the defer, but before the port number is returned, another goroutine happens to listen on 12345, see it's available, and returns it.

I think you are right. Let me see if there is a way to make this work correctly.

Ok resolved this by changing it so that the available port selection happens in ForwardPortE when local port is 0. This way, we can enforce a mutex lock between getting the available port and spawning the listener. See 7440db7 .

brikis98 · 2019-02-18T12:03:34Z

modules/k8s/tunnel_test.go

+
+ // Open a tunnel to pod from any available port locally
+ localPort := GetAvailablePort(t)
+ tunnel := NewTunnel(options, ResourceTypePod, "nginx-pod", localPort, 80)


Nice, powerful API hiding a lot of complexity 👍

Can't take credit for it since it is copied from helm 😄 But yes I like this API.

brikis98 · 2019-02-18T12:04:35Z

test/helm_basic_example_integration_test.go

+ 30,
+ 10*time.Second,
+ func(statusCode int, body string) bool {
+ return statusCode == 200


Check the body contains the text "nginx"?

yorinasub17 · 2019-02-18T16:53:35Z

Ok I believe I addressed all the comments except for the concurrency issue with GetAvailablePort. I will look into that tomorrow.

yorinasub17 · 2019-02-18T16:56:07Z

Also verified the private var conversion for Tunnel struct still works with helm-kubernetes-services.

yorinasub17 · 2019-02-20T01:31:45Z

Ok I believe I addressed all the review comments, and build is passing, so going to go ahead and merge + release this!

yorinasub17 added 14 commits February 16, 2019 10:56

Framework for helm integration tests

d692222

Fix goimports

cea9c18

Deploy Tiller

e73603a

Additional pod functions

bff3ddd

kube resource names must start with a letter

c71884b

Fix formatting

a870c5f

Support creating port forwards

2993c07

Rename resource types

4ca91d4

TEMP DELETE mE

649031e

Add list services

f4e2c32

Use build tags to further separate helm tests

d3e7e96

Fix the test runner

ad4d27a

Only install helm in helm_test

7a2893f

[skip ci] Add comments to helm.Install and helm.Delete

8cd84d8

yorinasub17 requested review from brikis98, Etiene and robmorgan as code owners February 17, 2019 00:22

yorinasub17 mentioned this pull request Feb 17, 2019

k8s-service basic example: deploying nginx gruntwork-io/helm-kubernetes-services#12

Merged

3 tasks

yorinasub17 commented Feb 17, 2019

View reviewed changes

yorinasub17 requested a review from autero1 February 17, 2019 00:26

brikis98 reviewed Feb 18, 2019

View reviewed changes

yorinasub17 added 3 commits February 18, 2019 08:38

Update boilerplate text on test overloading

bffe32b

Fix copy pasta

1a8c4e0

Address PR comments

8ad1927

Resolve race condition

7440db7

yorinasub17 merged commit 9430200 into master Feb 20, 2019

yorinasub17 deleted the yori-helm-integration branch February 20, 2019 01:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Helm integration test framework #237

Helm integration test framework #237

yorinasub17 commented Feb 17, 2019

yorinasub17 Feb 17, 2019

brikis98 Feb 18, 2019

yorinasub17 Feb 18, 2019

brikis98 Feb 18, 2019

yorinasub17 Feb 18, 2019

yorinasub17 Feb 18, 2019

brikis98 Feb 18, 2019

brikis98 Feb 18, 2019

yorinasub17 Feb 18, 2019

brikis98 Feb 18, 2019

yorinasub17 Feb 18, 2019

brikis98 Feb 18, 2019

yorinasub17 Feb 18, 2019

brikis98 Feb 18, 2019

yorinasub17 Feb 18, 2019

brikis98 Feb 18, 2019

yorinasub17 Feb 18, 2019

yorinasub17 Feb 19, 2019

brikis98 Feb 18, 2019

yorinasub17 Feb 18, 2019

brikis98 Feb 18, 2019

yorinasub17 Feb 18, 2019

yorinasub17 commented Feb 18, 2019

yorinasub17 commented Feb 18, 2019

yorinasub17 commented Feb 20, 2019

Helm integration test framework #237

Helm integration test framework #237

Conversation

yorinasub17 commented Feb 17, 2019

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

yorinasub17 commented Feb 18, 2019

yorinasub17 commented Feb 18, 2019

yorinasub17 commented Feb 20, 2019