Isolate envtest tests to local processes, rather than requiring a clu…

…ster (#319)

* Run tests with phony K8s API, not existing cluster

This commit changes the envtest (Kubernetes API harness) setup to use a
fake Kubernetes API rather than the (presumed) existing cluster. This
insulates the tests from coincidences -- for example, data that's
required for the test and happens to be present in the test author's
cluster, but nowhere else. It also ensures the code under test is the
code in the working directory, and not the code as it was when last
deployed to the cluster.

* Remove cluster setup from github workflow

* Reduce individual task timeout

The timeout for uses of `Eventually` is 10 minutes, but a whole test
case times out after 10 minutes -- so any failures will 1. take 10
minutes, and 2. tend to trigger the more opaque whole test case timeout,
where you get a process dump rather than a test failure mentioning the
line number.

This commit reduces the timeout for indvidual `Eventually` calls to 1
minute -- if this turns out to be optimistic in specific cases, some can
be increased (or it can be taken as a signal that the test should be
reconsidered).

* Distinguish between k8s and Pulumi op timeouts

Generally we expect Pulumi stacks to take O(minute) to run; but
Kubernetes API operations should be near instanteneous, so if they are
going to fail, they can fail quickly.

Note that deleting Stack objects is a Kubernetes API operation, but will
wait for the controller to run a finalizer, which is a stack operation
-- so these ops get a stack-proportioned timeout.

* Avoid a goroutine leak by ensuring pipes closed

The func runCmd uses io.Pipe for both stderr and stdout of the command
it runs, so writes on either can be echoed to the log as they happen;
and two goroutines for doing that echoing. In process dumps, it's
apparent that these goroutines leak -- they are blocked on Scan, which
is presumably waiting for an EOF.

This commit replaces the explicit pipe plumbing with
`Command.StdoutPipe()` and `Command.StderrPipe()`. The Command machinery
knows to close the readers when the process has finished writing, so
Scan no longer blocks indefinitely.

It's also not necessary to start _two_ new goroutines, since we already
have one -- that in which we're running.

* Avoid deletion with foreground policy in test

Foreground propagation propagation relies on a finalizer, which
interacts with the operator's finalizer in such a way that prevents
deletion indefinitely and for tests to time out (I'm not sure exactly
why to be honest, that's just what I observe). It shouldn't be
necessary, since the test code in question goes on to wait for the
deletion to have taken effect anyway.

* Changelog entry for goroutine leak fix

Signed-off-by: Michael Bridgen <[email protected]>

.github/workflows/master.yaml

@@ -44,15 +44,6 @@ jobs:
         uses: actions/setup-go@v2
         with:
           go-version: 1.19.x
-      - name: Install Ginkgo testing framework
-        run: |
-          # Do the install from outside the code tree to avoid messing with go.sum
-          cd /tmp; go install github.com/onsi/ginkgo/[email protected]
-      - name: Setup gcloud CLI for GKE testing cluster
-        uses: google-github-actions/setup-gcloud@v0
-        with:
-          service_account_key: ${{ secrets.GKE_SA_KEY }}
-          export_default_credentials: true
       - name: Configure AWS credentials to use in AWS Stack tests
         uses: aws-actions/configure-aws-credentials@v1
         with:
@@ -72,15 +63,14 @@ jobs:
       - name: Tests
         run: |
           # Create GKE test cluster to install CRDs and use with the test operator.
-          scripts/ci-cluster-create.sh
+          scripts/ci-infra-create.sh
           # Source the env variables created in the script above
           cat ~/.envfile
           . ~/.envfile
 
           # Run tests
-          make codegen install-crds
           make test
       - name: Cleanup
         if: ${{ always() }}
         run: |
-          scripts/ci-cluster-destroy.sh
+          scripts/ci-infra-destroy.sh

.github/workflows/release.yaml

@@ -23,15 +23,6 @@ jobs:
         uses: actions/setup-go@v2
         with:
           go-version: "1.19.x"
-      - name: Install Ginkgo testing framework
-        run: |
-          # Do the install from outside the code tree to avoid messing with go.sum
-          cd /tmp; go install github.com/onsi/ginkgo/[email protected]
-      - name: Setup gcloud CLI for GKE testing cluster
-        uses: google-github-actions/setup-gcloud@v0
-        with:
-          service_account_key: ${{ secrets.GKE_SA_KEY }}
-          export_default_credentials: true
       - name: Configure AWS credentials to use in AWS Stack tests
         uses: aws-actions/configure-aws-credentials@v1
         with:
@@ -44,25 +35,24 @@ jobs:
           role-to-assume: ${{ secrets.AWS_CI_ROLE_ARN }}
       - name: Install Pulumi CLI
         uses: pulumi/setup-pulumi@v2
-      - name: Set env variables
+      - name: Set env variables and path
         run: |
           echo '$HOME/.pulumi/bin' >> $GITHUB_PATH
           echo "STACK=ci-cluster-$(head /dev/urandom | LC_CTYPE=C tr -dc '[:lower:]' | head -c5)" >> $GITHUB_ENV
       - name: Tests
         run: |
           # Create GKE test cluster to install CRDs and use with the test operator.
-          scripts/ci-cluster-create.sh
+          scripts/ci-infra-create.sh
           # Source the env variables created in the script above
           cat ~/.envfile
           . ~/.envfile
 
           # Run tests
-          make codegen install-crds
           make test
       - name: Cleanup
         if: ${{ always() }}
         run: |
-          scripts/ci-cluster-destroy.sh
+          scripts/ci-infra-destroy.sh
   release:
     needs: [operator-int-tests]
     runs-on: ubuntu-latest

.github/workflows/run-acceptance-tests.yaml

@@ -56,15 +56,6 @@ jobs:
         uses: actions/setup-go@v2
         with:
           go-version: 1.19.x
-      - name: Install Ginkgo testing framework
-        run: |
-          # Do the install from outside the code tree to avoid messing with go.sum
-          cd /tmp; go install github.com/onsi/ginkgo/[email protected]
-      - name: Setup gcloud CLI for GKE testing cluster
-        uses: google-github-actions/setup-gcloud@v0
-        with:
-          service_account_key: ${{ secrets.GKE_SA_KEY }}
-          export_default_credentials: true
       - name: Configure AWS credentials to use in AWS Stack tests
         uses: aws-actions/configure-aws-credentials@v1
         with:
@@ -84,14 +75,14 @@ jobs:
       - name: Tests
         run: |
           # Create GKE test cluster to install CRDs and use with the test operator.
-          scripts/ci-cluster-create.sh
+          scripts/ci-infra-create.sh
           # Source the env variables created in the script above
           cat ~/.envfile
           . ~/.envfile
+
           # Run tests
-          make codegen install-crds
           make test
       - name: Cleanup
         if: ${{ always() }}
         run: |
-          scripts/ci-cluster-destroy.sh
+          scripts/ci-infra-destroy.sh

CHANGELOG.md

@@ -5,6 +5,7 @@ CHANGELOG
 
 - Exit processing early when a stack is ready to be garbage collected
   [#322](https://github.com/pulumi/pulumi-kubernetes-operator/pull/322)
+- Fix a goroutine leak [#319](https://github.com/pulumi/pulumi-kubernetes-operator/pull/319)
 
 ## 1.8.0 (2022-09-01)
 - Use go 1.18 for builds

Makefile

@@ -8,6 +8,10 @@ RELEASE ?= $(shell git describe --abbrev=0 --tags)
 
 default: build
 
+.PHONY: download-test-deps
+download-test-deps:
+	go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
+
 install-crds:
 	kubectl apply -f deploy/crds/pulumi.com_stacks.yaml
 
@@ -40,8 +44,8 @@ build-static:
 push-image:
 	docker push $(IMAGE_NAME):$(VERSION)
 
-test: install-crds
-	ginkgo -v ./test/...
+test: codegen download-test-deps
+	KUBEBUILDER_ASSETS="$(shell setup-envtest --use-env use -p path)" go test -v ./test/...
 
 deploy:
 	kubectl apply -f deploy/yaml/service_account.yaml

pkg/controller/stack/stack_controller.go

@@ -9,7 +9,6 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
-	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -620,38 +619,45 @@ func (sess *reconcileStackSession) runCmd(title string, cmd *exec.Cmd, workspace
 	}
 
 	// Capture stdout and stderr.
-	stdoutR, stdoutW := io.Pipe()
-	stderrR, stderrW := io.Pipe()
-	cmd.Stdout = stdoutW
-	cmd.Stderr = stderrW
+	stdoutR, err := cmd.StdoutPipe()
+	if err != nil {
+		return "", "", err
+	}
+	stderrR, err := cmd.StderrPipe()
+	if err != nil {
+		return "", "", err
+	}
 
 	// Start the command asynchronously.
-	err := cmd.Start()
+	err = cmd.Start()
 	if err != nil {
 		return "", "", err
 	}
 
-	// Kick off some goroutines to stream the output asynchronously. Since Pulumi can take
-	// a while to run, this helps to debug issues that might be ongoing before a command completes.
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
+
+	// We want to echo both stderr and stdout as they are written; so at least one of them must be
+	// in another goroutine.
+	stderrClosed := make(chan struct{})
+	errs := bufio.NewScanner(stderrR)
 	go func() {
-		outs := bufio.NewScanner(stdoutR)
-		for outs.Scan() {
-			text := outs.Text()
-			sess.logger.Debug(title, "Dir", cmd.Dir, "Path", cmd.Path, "Args", cmd.Args, "Stdout", text)
-			stdout.WriteString(text + "\n")
-		}
-	}()
-	go func() {
-		errs := bufio.NewScanner(stderrR)
 		for errs.Scan() {
 			text := errs.Text()
 			sess.logger.Debug(title, "Dir", cmd.Dir, "Path", cmd.Path, "Args", cmd.Args, "Text", text)
 			stderr.WriteString(text + "\n")
 		}
+		close(stderrClosed)
 	}()
 
+	outs := bufio.NewScanner(stdoutR)
+	for outs.Scan() {
+		text := outs.Text()
+		sess.logger.Debug(title, "Dir", cmd.Dir, "Path", cmd.Path, "Args", cmd.Args, "Stdout", text)
+		stdout.WriteString(text + "\n")
+	}
+	<-stderrClosed
+
 	// Now wait for the command to finish. No matter what, return everything written to stdout and
 	// stderr, in addition to the resulting error, if any.
 	err = cmd.Wait()
@@ -1139,9 +1145,15 @@ func (sess *reconcileStackSession) updateResource(o client.Object) error {
 	})
 }
 
-func (sess *reconcileStackSession) updateResourceStatus(o client.Object) error {
+func (sess *reconcileStackSession) updateResourceStatus(o *pulumiv1.Stack) error {
+	name := types.NamespacedName{Name: o.Name, Namespace: o.Namespace}
 	return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
-		return sess.kubeClient.Status().Update(context.TODO(), o)
+		var s pulumiv1.Stack
+		if err := sess.kubeClient.Get(context.TODO(), name, &s); err != nil {
+			return err
+		}
+		s.Status = o.Status
+		return sess.kubeClient.Status().Update(context.TODO(), &s)
 	})
 }
 

scripts/ci-cluster-create.sh → scripts/ci-infra-create.sh

@@ -1,18 +1,6 @@
 #!/usr/bin/env bash
 set -o nounset -o errexit -o pipefail
 
-echo Creating ephemeral Kubernetes cluster for CI testing...
-
-pushd test/ci-cluster
-yarn install --json --verbose >out
-tail -10 out
-pulumi stack init "${STACK}"
-pulumi up --skip-preview --yes
-
-mkdir -p "$HOME/.kube/"
-pulumi stack output kubeconfig --show-secrets >~/.kube/config
-popd
-
 echo Creating S3 backend and KMS key
 
 pushd test/s3backend

scripts/ci-cluster-destroy.sh → scripts/ci-infra-destroy.sh

@@ -1,14 +1,6 @@
 #!/usr/bin/env bash
 set -o nounset -o errexit -o pipefail
 
-echo Deleting ephemeral Kubernetes cluster...
-
-pushd test/ci-cluster
-pulumi stack select "${STACK}" && \
-  pulumi destroy --skip-preview --yes && \
-  pulumi stack rm --yes
-popd
-
 echo Deleting S3 backend and KMS Key...
 pushd test/s3backend
 
@@ -21,4 +13,3 @@ echo Destroying stack
 pulumi destroy --skip-preview --yes && \
   pulumi stack rm --yes
 popd
-