Unit and integration test enhancements

.github/workflows/pull_request.yaml

@@ -24,7 +24,7 @@ jobs:
     - name: Unit Tests
       run: make test
     - name: Codecov
-      uses: codecov/codecov-action@v1
+      uses: codecov/codecov-action@v2
       with:
         file: cover-unit.out
         flags: unit-tests
@@ -66,9 +66,9 @@ jobs:
         kubectl wait --for=condition=ready --timeout=30s node --all
         kubectl get nodes -o wide
     - name: Run integration tests
-      run: make integration EXTRA_GO_TEST_FLAGS=-v
+      run: make integration TEST_FLAGS=-v
     - name: Gather integration coverage results
-      uses: codecov/codecov-action@v1
+      uses: codecov/codecov-action@v2
       with:
         file: cover-int.out
         flags: integration-tests
@@ -110,9 +110,9 @@ jobs:
         kubectl get nodes -o wide
 
     - name: Run integration tests
-      run: make integration EXTRA_GO_TEST_FLAGS=-v
+      run: make integration TEST_FLAGS=-v
     - name: Gather integration coverage results
-      uses: codecov/codecov-action@v1
+      uses: codecov/codecov-action@v2
       with:
         file: cover-int.out
         flags: integration-tests

CONTRIBUTING.md

@@ -48,7 +48,7 @@ make integration
 
 If you want to run a single suite of tests while working on a specific area of the tests or main code, use something like this:
 ```
-make integration EXTRA_GO_TEST_FLAGS="-run TestConfigMapSuite -v"
+make integration TEST_FLAGS="-run TestConfigMapSuite -v"
 ```
 Hint: find the current test suites with `grep "func Test" integration/suites/*.go`
 

Makefile

@@ -1,8 +1,7 @@
 # Copyright (C) 2020 VMware, Inc.
 # SPDX-License-Identifier: Apache-2.0
 
-VERSION?=$(shell git describe --tags --first-parent --abbrev=7 --long --dirty --always)
-TEST_KUBECONFIG?=$(HOME)/.kube/config
+VERSION?=dev
 TEST_TIMEOUT=600s
 TEST_PARALLELISM=3
 
@@ -45,6 +44,8 @@ GO_DEPS=$(foreach dir,$(shell go list -deps -f '{{.Dir}}' ./cmd/kubectl-buildkit
 REVISION=$(shell git describe --match 'v[0-9]*' --always --dirty --tags)
 GO_FLAGS=-ldflags "-X $(GO_MOD_NAME)/version.Version=${VERSION} -X $(GO_MOD_NAME)/version.DefaultHelperImage=$(BUILDKIT_PROXY_IMAGE)" -mod=vendor
 GO_COVER_FLAGS=-cover -coverpkg=./... -covermode=count
+UNIT_TEST_PACKAGES=$(shell go list ./... | grep -v "/integration/")
+COVERAGE_FILTERS=grep -v "\.pb\.go" | grep -v "/integration/"
 
 .PHONY: help
 help:
@@ -105,28 +106,37 @@ $(BIN_DIR)/%.tgz: $(BIN_DIR)/%/*
 generate:
 	go generate ./...
 
+
 .PHONY: test
 test:
-	go test $(GO_FLAGS) $(GO_COVER_FLAGS) -coverprofile=./cover-unit.out ./...
+	go test $(GO_FLAGS) \
+		-parallel $(TEST_PARALLELISM) \
+		$(TEST_FLAGS)  \
+		$(GO_COVER_FLAGS) -coverprofile=./cover-unit-full.out \
+		$(UNIT_TEST_PACKAGES)
+	cat ./cover-unit-full.out | $(COVERAGE_FILTERS) > ./cover-unit.out
+	rm -f ./cover-unit-full.out
+
 
 .PHONY: integration
 integration:
-	@echo "Running integration tests with $(TEST_KUBECONFIG)"
+	@echo "Running integration tests"
 	@kubectl config get-contexts
-	TEST_KUBECONFIG=$(TEST_KUBECONFIG) go test -timeout $(TEST_TIMEOUT) $(GO_FLAGS) \
+	go test -timeout $(TEST_TIMEOUT) $(GO_FLAGS) \
 		-parallel $(TEST_PARALLELISM) \
-		$(EXTRA_GO_TEST_FLAGS)  \
-		$(GO_COVER_FLAGS) -coverprofile=./cover-int.out \
+		$(TEST_FLAGS)  \
+		$(GO_COVER_FLAGS) -coverprofile=./cover-int-full.out \
 		./integration/...
-
+	cat ./cover-int-full.out | $(COVERAGE_FILTERS) > ./cover-int.out
+	rm -f ./cover-int-full.out
 	go tool cover -html=./cover-int.out -o ./cover-int.html
 
 .PHONY: coverage
 coverage: cover.html
 
 cover.html: cover-int.out cover-unit.out
-	cp cover-int.out cover.out
-	tail +2 cover-unit.out >> cover.out
+	cat cover-int.out > cover.out
+	tail +2 cover-unit.out | $(COVERAGE_FILTERS) >> cover.out
 	go tool cover -html=./cover.out -o ./cover.html
 	go tool cover -func cover.out | grep total:
 	open ./cover.html

integration/suites/configmap_test.go → integration/builders/configmap/configmap_test.go

@@ -1,6 +1,6 @@
 // Copyright (C) 2020 VMware, Inc.
 // SPDX-License-Identifier: Apache-2.0
-package suites
+package configmap
 
 import (
 	"context"
@@ -206,7 +206,6 @@ func (s *configMapSuite) TestPreExistingWithCustomCreate() {
 }
 
 func TestConfigMapSuite(t *testing.T) {
-	common.Skipper(t)
 	t.Parallel()
 	suite.Run(t, &configMapSuite{
 		Name:        "configmaptest",

integration/builders/configmap/doc.go

@@ -0,0 +1 @@
+package configmap

integration/suites/default_test.go → integration/builders/default/default_test.go

@@ -1,6 +1,6 @@
 // Copyright (C) 2020 VMware, Inc.
 // SPDX-License-Identifier: Apache-2.0
-package suites
+package dflt
 
 import (
 	"bytes"
@@ -26,8 +26,11 @@ func (s *DefaultSuite) TestVersion() {
 }
 
 func TestDefaultSuite(t *testing.T) {
-	common.Skipper(t)
-	t.Parallel()
+	// Clean up any pre-existing default builder before proceeding
+	_ = common.RunBuildkit("rm", []string{
+		"buildkit",
+	}, common.RunBuildStreams{})
+
 	suite.Run(t, &DefaultSuite{
 		BaseSuite: common.BaseSuite{
 			Name: "buildkit", // TODO pull this from the actual default name

integration/builders/default/doc.go

@@ -0,0 +1 @@
+package dflt

integration/suites/parallel_default_test.go → integration/builders/default/parallel_default_test.go

@@ -1,6 +1,6 @@
 // Copyright (C) 2020 VMware, Inc.
 // SPDX-License-Identifier: Apache-2.0
-package suites
+package dflt
 
 import (
 	"context"
@@ -98,8 +98,12 @@ func (s *parallelDefaultSuite) TestParallelDefaultBuilds() {
 }
 
 func TestParallelDefaultBuildSuite(t *testing.T) {
-	common.Skipper(t)
 	// We don't parallelize with other tests, since we use the default builder name
+	// Clean up any pre-existing default builder before proceeding
+	_ = common.RunBuildkit("rm", []string{
+		"buildkit",
+	}, common.RunBuildStreams{})
+
 	suite.Run(t, &parallelDefaultSuite{
 		Name:        "buildkit",
 		CreateFlags: []string{"--buildkitd-flags=--debug"},

integration/builders/localregistry/doc.go

@@ -0,0 +1 @@
+package localregistry

integration/suites/localregistry_test.go → integration/builders/localregistry/localregistry_test.go

@@ -1,6 +1,6 @@
 // Copyright (C) 2020 VMware, Inc.
 // SPDX-License-Identifier: Apache-2.0
-package suites
+package localregistry
 
 import (
 	"bytes"
@@ -29,6 +29,13 @@ const (
 	RegistryImageName = "docker.io/registry:2.7"
 )
 
+var (
+	FastPlatforms = []string{"linux/386", "linux/amd64", "linux/arm/v6", "linux/arm/v7", "linux/arm64"}
+
+	// Run with -short to skip these slower platforms (larger base images)
+	SlowPlatforms = []string{"windows/amd64"}
+)
+
 type localRegistrySuite struct {
 	suite.Suite
 	Name        string
@@ -49,7 +56,6 @@ type localRegistrySuite struct {
 }
 
 func (s *localRegistrySuite) SetupSuite() {
-	// s.skipTeardown = true
 	var err error
 	ctx := context.Background()
 	s.ClientSet, s.Namespace, err = common.GetKubeClientset()
@@ -260,8 +266,18 @@ func (s *localRegistrySuite) TearDownSuite() {
 
 		ctx := context.Background()
 
+		// Grab logs from the registry in case there are any interesting failures
+		pod, err := s.podClient.Get(ctx, s.registryName, metav1.GetOptions{})
+		if err != nil {
+			logrus.Warnf("failed to get registry pod for logs %s: %s", s.registryName, err)
+		} else {
+			logrus.Infof("--- BEGIN REGISTRY LOGS ---")
+			common.LogPodLogs(ctx, []corev1.Pod{*pod}, s.Namespace, s.ClientSet)
+			logrus.Infof("--- END REGISTRY LOGS ---")
+		}
+
 		// Clean everything up...
-		err := s.podClient.Delete(ctx, s.registryName, metav1.DeleteOptions{})
+		err = s.podClient.Delete(ctx, s.registryName, metav1.DeleteOptions{})
 		if err != nil {
 			logrus.Warnf("failed to clean up pod %s: %s", s.registryName, err)
 		}
@@ -429,11 +445,18 @@ func main() {
 	require.NoError(s.T(), err, "%s: config file creation", s.Name)
 	defer cleanup()
 	imageName := s.registryFQDN + "/" + s.Name + "multiarchcrosscompile:latest"
+	var platforms []string
+	if testing.Short() {
+		platforms = FastPlatforms[0:2]
+	} else {
+		platforms = append(FastPlatforms, SlowPlatforms...)
+	}
+
 	args := []string{"--progress=plain",
 		"--builder", s.Name,
 		"--push",
 		"--tag", imageName,
-		"--platform", "linux/386,linux/amd64,linux/arm/v6,linux/arm/v7,linux/arm64,windows/amd64",
+		"--platform", strings.Join(platforms, ","),
 		dir,
 	}
 	err = common.RunBuild(args, common.RunBuildStreams{})
@@ -455,12 +478,17 @@ func (s *localRegistrySuite) TestVersion() {
 }
 
 func TestLocalRegistrySuite(t *testing.T) {
-	common.Skipper(t)
 	// TODO this testcase should be safe to run parallel, but I'm seeing failures in CI that look
 	// like containerd runtime concurrency problems.  They don't seem related to this particular change though
 	//t.Parallel()
+	skipTeardown := false
+	skipTeardownStr := os.Getenv("SKIP_TEARDOWN")
+	if skipTeardownStr != "" {
+		skipTeardown = true
+	}
 	suite.Run(t, &localRegistrySuite{
-		Name: "regtest",
+		Name:         "regtest",
+		skipTeardown: skipTeardown,
 		// Debug set in the config file
 	})
 }

integration/builders/randomlb/doc.go

@@ -0,0 +1 @@
+package randomlb

integration/suites/randomlb_test.go → integration/builders/randomlb/randomlb_test.go

@@ -1,6 +1,6 @@
 // Copyright (C) 2020 VMware, Inc.
 // SPDX-License-Identifier: Apache-2.0
-package suites
+package randomlb
 
 import (
 	"testing"
@@ -15,7 +15,6 @@ type RandomLBSuite struct{ common.BaseSuite }
 // TODO - add some conditional test cases to varify random scheduling is actually working
 
 func TestRandomLBSuite(t *testing.T) {
-	common.Skipper(t)
 	t.Parallel()
 	suite.Run(t, &RandomLBSuite{
 		BaseSuite: common.BaseSuite{

integration/builders/readme.md

@@ -0,0 +1,3 @@
+# End to End tests
+
+Each directory contains tests based on a builder configuration pattern.

integration/builders/rootless/doc.go

@@ -0,0 +1 @@
+package rootless

integration/suites/rootless_test.go → integration/builders/rootless/rootless_test.go

@@ -1,6 +1,6 @@
 // Copyright (C) 2020 VMware, Inc.
 // SPDX-License-Identifier: Apache-2.0
-package suites
+package rootless
 
 import (
 	"testing"
@@ -13,7 +13,6 @@ import (
 type rootlessSuite struct{ common.BaseSuite }
 
 func TestRootlessSuite(t *testing.T) {
-	common.Skipper(t)
 	//t.Parallel() // TODO - tests fail if run in parallel, may be actual race bug
 	suite.Run(t, &rootlessSuite{
 		BaseSuite: common.BaseSuite{

integration/common/basesuites.go

@@ -156,7 +156,7 @@ func (s *BaseSuite) TestLs() {
 
 func (s *BaseSuite) TestBuildWithSecret() {
 	logrus.Infof("%s: Build with Secret", s.Name)
-	proxyImage := os.Getenv("TEST_IMAGE_BASE")
+	proxyImage := GetTestImageBase()
 
 	dir, cleanup, err := NewBuildContext(map[string]string{
 		"Dockerfile": fmt.Sprintf(`# syntax=docker/dockerfile:experimental
@@ -205,7 +205,7 @@ func (s *BaseSuite) TestBuildWithSSHKey() {
 	s.T().Skip("Skipping SSH test as it's too flaky")
 
 	logrus.Infof("%s: Build with SSH key", s.Name)
-	proxyImage := os.Getenv("TEST_IMAGE_BASE")
+	proxyImage := GetTestImageBase()
 
 	// Note: if the key is not valid, the socket will not be created, so simply checking for
 	// its existence is sufficient to verify the ssh plumbing is working.
@@ -261,3 +261,11 @@ RUN --mount=type=ssh,id=myssh echo "SSH_AUTH_SOCK=${SSH_AUTH_SOCK}" && ls -l ${S
 
 	require.NoError(s.T(), err, "build failed")
 }
+
+func GetTestImageBase() string {
+	base := os.Getenv("TEST_IMAGE_BASE")
+	if base == "" {
+		base = "busybox"
+	}
+	return base
+}

integration/common/buildcontext.go

@@ -38,10 +38,7 @@ func NewBuildContext(payloads map[string]string) (string, func(), error) {
 
 // NewSimpleBuildContext creates a very simple Dockerfile for exercising the builder
 func NewSimpleBuildContext() (string, func(), error) {
-	proxyImage := os.Getenv("TEST_IMAGE_BASE")
-	if proxyImage == "" {
-		return "", nil, fmt.Errorf("TEST_IMAGE_BASE env var unset")
-	}
+	proxyImage := GetTestImageBase()
 	return NewBuildContext(map[string]string{
 		"Dockerfile": fmt.Sprintf(`FROM %s
 RUN echo "#!/bin/sh" > /run

integration/common/kubeclient.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"k8s.io/client-go/kubernetes"
@@ -30,6 +31,18 @@ func GetKubeClientset() (*kubernetes.Clientset, string, error) {
 		return nil, "", err
 	}
 	clientset, err := kubernetes.NewForConfig(restClientConfig)
+	if err != nil {
+		return nil, "", err
+	}
+
+	// Verify the cluster is accessible so we can fail fast
+	content, err := clientset.Discovery().RESTClient().Get().AbsPath("/livez").DoRaw(context.Background())
+	if err != nil {
+		return nil, "", errors.Wrap(err, "kubernetes cluster is unhealthy or inaccessible")
+	}
+	if !strings.Contains(string(content), "ok") {
+		return nil, "", fmt.Errorf("kubernetes cluster is unhealthy or inaccessible: %s", string(content))
+	}
 	return clientset, ns, err
 }
 
@@ -196,6 +209,11 @@ func LogBuilderLogs(ctx context.Context, name, namespace string, clientset *kube
 	}
 	logrus.Infof("Detected %d pods for builder %s - gathering logs", len(pods), name)
 	logrus.Infof("--- BEGIN BUILDER LOGS ---")
+	LogPodLogs(ctx, pods, namespace, clientset)
+	logrus.Infof("--- END BUILDER LOGS ---")
+}
+
+func LogPodLogs(ctx context.Context, pods []v1.Pod, namespace string, clientset *kubernetes.Clientset) {
 	podClient := clientset.CoreV1().Pods(namespace)
 	for _, pod := range pods {
 		for _, ctr := range pod.Spec.Containers {
@@ -211,6 +229,5 @@ func LogBuilderLogs(ctx context.Context, name, namespace string, clientset *kube
 			}
 		}
 	}
-	logrus.Infof("--- END BUILDER LOGS ---")
 
 }