feat: Integration test enhancements

This enhances the integration test suite in a variety of ways: - Both minikube and KiND are now supported, and run in github actions - Tests run in their own namespace, which is intelligently cleaned up - Images are uploaded to a test backend via docker registry API - Buildkit is used to significantly speed up incremental image builds This commit also fixes a bug where failed jobs would not report their failure. Co-authored-by: Aaron Olson <[email protected]> Co-authored-by: Zeeshan Qureshi <[email protected]>
iovisor · Jul 15, 2021 · b41eb65 · b41eb65
1 parent 2f2174a
commit b41eb65
Show file tree

Hide file tree

Showing 13 changed files with 810 additions and 109 deletions.
diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml
@@ -1,6 +1,6 @@
 name: Kubectl trace build and tests
 
-on: [push, pull_request]
+on: [pull_request]
 
 jobs:
   build_and_test:
@@ -9,6 +9,9 @@ jobs:
       matrix:
         os: [ubuntu-16.04, ubuntu-18.04] # 16.04.4 release has 4.15 kernel
                                          # 18.04.3 release has 5.0.0 kernel
+        env:
+        - TEST_KUBERNETES_BACKEND: minikube
+        - TEST_KUBERNETES_BACKEND: kind
     steps:
     - uses: actions/checkout@v2
     - run: git fetch --prune --unshallow # We want tags
@@ -39,13 +42,26 @@ jobs:
 
     - name: Build CI image
       run: |
-       ./build/scripts/ci-build-image.sh ${{ github.ref }}
+       ./build/scripts/ci-build-image.sh ${{ github.head_ref }}
+
+    - name: Install minikube
+      env: ${{matrix.env}}
+      if: ${{ env.TEST_KUBERNETES_BACKEND == 'minikube' }}
+      run: |
+        curl -LO https://storage.googleapis.com/minikube/releases/latest/minikube-linux-amd64
+        sudo install minikube-linux-amd64 /usr/local/bin/minikube
 
     - name: Run integration tests
+      env: ${{matrix.env}}
       run: |
         make integration
 
+#    - name: Debug failure over SSH
+#      if: ${{ failure() }}
+#      uses: mxschmitt/action-tmate@v3
+
     - name: Build cross binaries
+      if: github.ref == 'refs/heads/master'
       run: |
         curl -LO https://github.com/goreleaser/goreleaser/releases/latest/download/goreleaser_amd64.deb && sudo dpkg -i goreleaser_amd64.deb
         make cross
@@ -56,6 +72,7 @@ jobs:
         path: _output/bin/kubectl-trace
 
     - uses: actions/upload-artifact@v1
+      if: github.ref == 'refs/heads/master'
       with:
         name: ${{ matrix.os }}-kubectl-trace-cross-dist
         path: dist
@@ -66,4 +83,4 @@ jobs:
       env:
         QUAY_TOKEN: ${{ secrets.QUAY_TOKEN }}
       run: |
-        ./build/scripts/ci-release-image.sh ${{ github.ref }}
+        ./build/scripts/ci-release-image.sh ${{ github.head_ref }}
diff --git a/Makefile b/Makefile
@@ -4,35 +4,45 @@ GO ?= go
 DOCKER ?= docker
 
 COMMIT_NO := $(shell git rev-parse HEAD 2> /dev/null || true)
-GIT_COMMIT := $(if $(shell git status --porcelain --untracked-files=no),${COMMIT_NO}-dirty,${COMMIT_NO})
+GIT_COMMIT := $(if $(shell git status --porcelain --untracked-files=no 2> /dev/null),${COMMIT_NO}-dirty,${COMMIT_NO})
 GIT_TAG    ?= $(shell git describe 2> /dev/null)
 GIT_BRANCH ?= $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null)
 GIT_BRANCH_CLEAN := $(shell echo $(GIT_BRANCH) | sed -e "s/[^[:alnum:]]/-/g")
 
 GIT_ORG ?= iovisor
 
-IMAGE_NAME_INIT ?= quay.io/$(GIT_ORG)/kubectl-trace-init
-IMAGE_NAME ?= quay.io/$(GIT_ORG)/kubectl-trace-bpftrace
+DOCKER_BUILD_PROGRESS ?= auto
 
-IMAGE_TRACERUNNER_BRANCH := $(IMAGE_NAME):$(GIT_BRANCH_CLEAN)
-IMAGE_TRACERUNNER_COMMIT := $(IMAGE_NAME):$(GIT_COMMIT)
-IMAGE_TRACERUNNER_TAG    := $(IMAGE_NAME):$(GIT_TAG)
-IMAGE_TRACERUNNER_LATEST := $(IMAGE_NAME):latest
+IMAGE_NAME_INITCONTAINER ?= quay.io/$(GIT_ORG)/kubectl-trace-init
+IMAGE_NAME_TRACERUNNER ?= quay.io/$(GIT_ORG)/kubectl-trace-runner
 
-IMAGE_INITCONTAINER_BRANCH := $(IMAGE_NAME_INIT):$(GIT_BRANCH_CLEAN)
-IMAGE_INITCONTAINER_COMMIT := $(IMAGE_NAME_INIT):$(GIT_COMMIT)
-IMAGE_INITCONTAINER_TAG    := $(IMAGE_NAME_INIT):$(GIT_TAG)
-IMAGE_INITCONTAINER_LATEST := $(IMAGE_NAME_INIT):latest
+IMAGE_TRACERUNNER_BRANCH := $(IMAGE_NAME_TRACERUNNER):$(GIT_BRANCH_CLEAN)
+IMAGE_TRACERUNNER_COMMIT := $(IMAGE_NAME_TRACERUNNER):$(GIT_COMMIT)
+IMAGE_TRACERUNNER_TAG    := $(IMAGE_NAME_TRACERUNNER):$(GIT_TAG)
+IMAGE_TRACERUNNER_LATEST := $(IMAGE_NAME_TRACERUNNER):latest
 
-IMAGE_BUILD_FLAGS ?= "--no-cache"
+IMAGE_INITCONTAINER_BRANCH := $(IMAGE_NAME_INITCONTAINER):$(GIT_BRANCH_CLEAN)
+IMAGE_INITCONTAINER_COMMIT := $(IMAGE_NAME_INITCONTAINER):$(GIT_COMMIT)
+IMAGE_INITCONTAINER_TAG    := $(IMAGE_NAME_INITCONTAINER):$(GIT_TAG)
+IMAGE_INITCONTAINER_LATEST := $(IMAGE_NAME_INITCONTAINER):latest
 
-BPFTRACEVERSION ?= "v0.11.1"
+IMAGE_BUILD_FLAGS_EXTRA ?= # convenience to allow to specify extra build flags with env var, defaults to nil
 
-LDFLAGS := -ldflags '-X github.com/iovisor/kubectl-trace/pkg/version.buildTime=$(shell date +%s) -X github.com/iovisor/kubectl-trace/pkg/version.gitCommit=${GIT_COMMIT} -X github.com/iovisor/kubectl-trace/pkg/cmd.ImageName=${IMAGE_NAME} -X github.com/iovisor/kubectl-trace/pkg/cmd.ImageTag=${GIT_COMMIT} -X github.com/iovisor/kubectl-trace/pkg/cmd.InitImageName=${IMAGE_NAME_INIT} -X github.com/iovisor/kubectl-trace/pkg/cmd.InitImageTag=${GIT_COMMIT}'
+IMG_REPO ?= quay.io/iovisor/
+IMG_SHA ?= latest
+
+BPFTRACEVERSION ?= "v0.13.0"
+
+LDFLAGS := -ldflags '-X github.com/iovisor/kubectl-trace/pkg/version.buildTime=$(shell date +%s) -X github.com/iovisor/kubectl-trace/pkg/version.gitCommit=${GIT_COMMIT} -X github.com/iovisor/kubectl-trace/pkg/cmd.ImageName=${IMAGE_NAME_TRACERUNNER} -X github.com/iovisor/kubectl-trace/pkg/cmd.ImageTag=${GIT_COMMIT} -X github.com/iovisor/kubectl-trace/pkg/cmd.InitImageName=${IMAGE_NAME_INITCONTAINER} -X github.com/iovisor/kubectl-trace/pkg/cmd.InitImageTag=${GIT_COMMIT}'
 TESTPACKAGES := $(shell go list ./... | grep -v github.com/iovisor/kubectl-trace/integration)
+TEST_ONLY ?=
 
 kubectl_trace ?= _output/bin/kubectl-trace
 trace_runner ?= _output/bin/trace-runner
+trace_uploader ?= _output/bin/trace-uploader
+
+# ensure variables are available to child invocations of make
+export
 
 .PHONY: build
 build: clean ${kubectl_trace}
@@ -45,11 +55,11 @@ ${trace_runner}:
 
 .PHONY: cross
 cross:
-	IMAGE_NAME=$(IMAGE_NAME) GO111MODULE=on goreleaser --snapshot --rm-dist
+	IMAGE_NAME_TRACERUNNER=$(IMAGE_NAME_TRACERUNNER) GO111MODULE=on goreleaser --snapshot --rm-dist
 
 .PHONY: release
 release:
-	IMAGE_NAME=$(IMAGE_NAME) GO111MODULE=on goreleaser --rm-dist
+	IMAGE_NAME_TRACERUNNER=$(IMAGE_NAME_TRACERUNNER) GO111MODULE=on goreleaser --rm-dist
 
 .PHONY: clean
 clean:
@@ -59,24 +69,30 @@ clean:
 .PHONY: image/build-init
 image/build-init:
 	$(DOCKER) build \
-		$(IMAGE_BUILD_FLAGS) \
+		--progress=$(DOCKER_BUILD_PROGRESS) \
 		-t $(IMAGE_INITCONTAINER_BRANCH) \
-		-f ./build/Dockerfile.initcontainer ./build
-	$(DOCKER) tag $(IMAGE_INITCONTAINER_BRANCH) $(IMAGE_INITCONTAINER_COMMIT)
-	$(DOCKER) tag $(IMAGE_INITCONTAINER_BRANCH) $(IMAGE_INITCONTAINER_TAG)
+		-f ./build/Dockerfile.initcontainer \
+		${IMAGE_BUILD_FLAGS_EXTRA} \
+		./build
+	$(DOCKER) tag "$(IMAGE_INITCONTAINER_BRANCH)" "$(IMAGE_INITCONTAINER_COMMIT)"
+	$(DOCKER) tag "$(IMAGE_INITCONTAINER_BRANCH)" "$(IMAGE_INITCONTAINER_TAG)"
+	$(DOCKER) tag "$(IMAGE_INITCONTAINER_BRANCH)" "$(IMAGE_INITCONTAINER_LATEST)"
 
 .PHONY: image/build
 image/build:
-	$(DOCKER) build \
+	DOCKER_BUILDKIT=1 $(DOCKER) build \
 		--build-arg bpftraceversion=$(BPFTRACEVERSION) \
 		--build-arg GIT_ORG=$(GIT_ORG) \
-		$(IMAGE_BUILD_FLAGS) \
+		--progress=$(DOCKER_BUILD_PROGRESS) \
+		--build-arg BUILDKIT_INLINE_CACHE=1 \
+		--cache-from "$(IMAGE_TRACERUNNER_BRANCH)" \
 		-t "$(IMAGE_TRACERUNNER_BRANCH)" \
-		-f build/Dockerfile.tracerunner .
-	$(DOCKER) tag $(IMAGE_TRACERUNNER_BRANCH) $(IMAGE_TRACERUNNER_COMMIT)
-	$(DOCKER) tag "$(IMAGE_TRACERUNNER_BRANCH)" $(IMAGE_TRACERUNNER_BRANCH)
-	$(DOCKER) tag "$(IMAGE_TRACERUNNER_BRANCH)" $(IMAGE_TRACERUNNER_TAG)
-
+		-f build/Dockerfile.tracerunner \
+		${IMAGE_BUILD_FLAGS_EXTRA} \
+		.
+	$(DOCKER) tag "$(IMAGE_TRACERUNNER_BRANCH)" "$(IMAGE_TRACERUNNER_COMMIT)"
+	$(DOCKER) tag "$(IMAGE_TRACERUNNER_BRANCH)" "$(IMAGE_TRACERUNNER_TAG)"
+	$(DOCKER) tag "$(IMAGE_TRACERUNNER_BRANCH)" "$(IMAGE_TRACERUNNER_LATEST)"
 
 .PHONY: image/push
 image/push:
@@ -99,5 +115,5 @@ test:
 	$(GO) test -v -race $(TESTPACKAGES)
 
 .PHONY: integration
-integration:
-	TEST_KUBECTLTRACE_BINARY=$(shell pwd)/$(kubectl_trace) $(GO) test ${LDFLAGS} -v ./integration/...
+integration: build image/build image/build-init
+	TEST_KUBECTLTRACE_BINARY=$(shell pwd)/$(kubectl_trace) $(GO) test ${LDFLAGS} -failfast -count=1 -v ./integration/... -run TestKubectlTraceSuite $(if $(TEST_ONLY),-testify.m $(TEST_ONLY),)
diff --git a/build/Dockerfile.tracerunner b/build/Dockerfile.tracerunner
@@ -1,23 +1,37 @@
+# syntax = docker/dockerfile:1.2
 ARG bpftraceversion=v0.13.0
 FROM quay.io/iovisor/bpftrace:$bpftraceversion as bpftrace
 
 FROM golang:1.15-buster as gobuilder
 ARG GIT_ORG=iovisor
 ENV GIT_ORG=$GIT_ORG
-RUN apt-get update
-RUN apt-get install -y make bash git
+RUN apt-get update && apt-get install -y make bash git && apt-get clean
 
-ADD . /go/src/github.com/iovisor/kubectl-trace
 WORKDIR /go/src/github.com/iovisor/kubectl-trace
 
-RUN make _output/bin/trace-runner
+# first copy the go mod files and sync the module cache as this step is expensive
+COPY go.* .
+RUN go mod download
+
+# Now copy the rest of the source code one by one
+# note any changes in any of these files or subdirectories is expected to bust the cache
+# We copy only the code directories, makefile, and git directory in order to prevent
+# busting the cache. Due to limitations in docker syntax, this must be done one-per-line
+COPY Makefile .
+COPY cmd cmd
+COPY pkg pkg
+
+# This buildkit feature reduces the build time from ~50s → 5s by preserving the compiler cache
+RUN --mount=type=cache,target=/root/.cache/go-build make _output/bin/trace-runner
 
 FROM ubuntu:20.04
 
-RUN apt-get update
-RUN apt-get install -y xz-utils
+# Install CA certificates
+RUN apt-get update && apt-get install -y ca-certificates && update-ca-certificates && apt-get clean
 
-COPY --from=gobuilder /go/src/github.com/iovisor/kubectl-trace/_output/bin/trace-runner /bin/trace-runner
 COPY --from=bpftrace /usr/bin/bpftrace /usr/bin/bpftrace
+COPY --from=gobuilder /go/src/github.com/iovisor/kubectl-trace/_output/bin/trace-runner /bin/trace-runner
+
+COPY /build/hooks/prestop /bin/hooks/prestop
 
 ENTRYPOINT ["/bin/trace-runner"]
diff --git a/build/hooks/prestop b/build/hooks/prestop
@@ -0,0 +1,29 @@
+#!/bin/bash
+set -uo pipefail
+
+if [[ "$#" -gt "1" ]]; then
+  echo "usage: prestop [SLEEP_SECONDS]"
+  exit 41
+fi
+
+sleep_seconds=0
+if [[ "$#" -eq "1" ]]; then
+  sleep_seconds="$1"
+fi
+
+tpid=`pgrep --oldest trace-runner`
+if [[ -z "$tpid" ]]; then
+  echo "could not find trace-runner"
+  exit 21
+fi
+
+cpid=`pgrep --oldest -P $tpid`
+if [[ -z "$cpid" ]]; then
+  echo "could not find first child of trace-runner"
+  exit 22
+fi
+
+kill -SIGINT $cpid
+
+## Give some time to trace-runner to cleanup before pod kill timeout starts.
+sleep $sleep_seconds
diff --git a/go.mod b/go.mod
@@ -6,11 +6,10 @@ require (
 	github.com/evanphx/json-patch v4.9.0+incompatible
 	github.com/fntlnz/mountinfo v0.0.0-20171106231217-40cb42681fad
 	github.com/kr/pretty v0.2.1 // indirect
-	github.com/pkg/errors v0.9.1
 	github.com/spf13/cobra v1.1.1
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.6.1
-	gotest.tools v2.2.0+incompatible
+	golang.org/x/mod v0.3.0
 	k8s.io/api v0.19.3
 	k8s.io/apimachinery v0.19.3
 	k8s.io/cli-runtime v0.19.3

diff --git a/go.sum b/go.sum
@@ -410,6 +410,7 @@ golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKG
 golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY=
 golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
+golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/net v0.0.0-20170114055629-f2499483f923/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=

diff --git a/integration/cmd_run_test.go b/integration/cmd_run_test.go
@@ -2,19 +2,45 @@ package integration
 
 import (
 	"regexp"
+	"time"
 
 	"github.com/stretchr/testify/assert"
+
+	batchv1 "k8s.io/api/batch/v1"
 )
 
-func (k *KubectlTraceSuite) TestRunNode() {
-	nodes, err := k.provider.ListNodes(k.name)
-	assert.Nil(k.T(), err)
-	assert.Equal(k.T(), 1, len(nodes))
+type outputAsserter func(string, []byte)
 
-	nodeName := nodes[0].String()
+func (k *KubectlTraceSuite) TestRunNode() {
+	nodeName := k.GetTestNode()
 	bpftraceProgram := `kprobe:do_sys_open { printf("%s: %s\n", comm, str(arg1)) }'`
-	out := k.KubectlTraceCmd("run", "-e", bpftraceProgram, nodeName)
-	match, err := regexp.MatchString("trace (\\w+-){4}\\w+ created", out)
-	assert.Nil(k.T(), err)
-	assert.True(k.T(), match)
+	out := k.KubectlTraceCmd("run", "--namespace="+k.namespace(), "--imagename="+k.RunnerImage(), "-e", bpftraceProgram, nodeName)
+	assert.Regexp(k.T(), "trace (\\w+-){4}\\w+ created", out)
+}
+
+func (k *KubectlTraceSuite) TestReturnErrOnErr() {
+	nodeName := k.GetTestNode()
+
+	bpftraceProgram := `kprobe:not_a_real_kprobe { printf("%s: %s\n", comm, str(arg1)) }'`
+	out := k.KubectlTraceCmd("run", "--namespace="+k.namespace(), "--imagename="+k.RunnerImage(), "-e", bpftraceProgram, nodeName)
+	assert.Regexp(k.T(), regexp.MustCompile("trace [a-f0-9-]{36} created"), out)
+
+	var job batchv1.Job
+
+	for {
+		jobs := k.GetJobs().Items
+		assert.Equal(k.T(), 1, len(jobs))
+
+		job = jobs[0]
+		if len(job.Status.Conditions) > 0 {
+			break // on the first condition
+		}
+
+		time.Sleep(1 * time.Second)
+	}
+
+	assert.Equal(k.T(), 1, len(job.Status.Conditions))
+	assert.Equal(k.T(), "Failed", string(job.Status.Conditions[0].Type))
+	assert.Equal(k.T(), int32(0), job.Status.Succeeded, "No jobs in the batch should have succeeded")
+	assert.Greater(k.T(), job.Status.Failed, int32(1), "There should be at least one failed job")
 }