Merge pull request #2382 from estroz/chore/clean-up-test-setup

🌱 chore(test): clean up e2e setup scripts, unit test utils

Author: k8s-ci-robot

.github/workflows/lint.yml

@@ -18,7 +18,7 @@ jobs:
       - name: Run linter
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.37        # Always uses the latest patch version.
+          version: v1.41        # Always uses the latest patch version.
           only-new-issues: true # Show only new issues if it's a pull request
       - name: Report failure
         uses: nashmaniac/[email protected]

CONTRIBUTING.md

@@ -35,13 +35,15 @@ $ git clone [email protected]:<user>/kubebuilder.git $GOPATH/src/sigs.k8s.io/kubebu
 ```
 
 1. Ensure you activate module support before continue (`$ export GO111MODULE=on`)
-1. Run the command `make install` to create a bin with the source code 
+1. Run the command `make install` to create a bin with the source code
 
 **NOTE** In order to check the local environment run `make test-unit`.
 
-## What to do before submitting a pull request 
+## What to do before submitting a pull request
 
-1. Run the script `make generate` to update/generate the mock data used in the e2e test in `$GOPATH/src/sigs.k8s.io/kubebuilder/testdata/` 
+1. Run the script `make generate` to update/generate the mock data used in the e2e test in `$GOPATH/src/sigs.k8s.io/kubebuilder/testdata/`
+1. Run `make test-unit test-e2e-local`
+  - e2e tests use [`kind`][kind] and [`setup-envtest`][setup-envtest]. If you want to bring your own binaries, place them in `$(go env GOPATH)/bin`.
 
 **IMPORTANT:** The `make generate` is very helpful. By using it, you can check if good part of the commands still working successfully after the changes. Also, note that its usage is a pre-requirement to submit a PR.
 
@@ -87,9 +89,9 @@ separately.
 
 ## Where the CI Tests are configured
 
-1. See the [action files](.github/workflows) to check its tests, and the scripts used on it. 
-1. Note that the prow tests used in the CI are configured in [kubernetes-sigs/kubebuilder/kubebuilder-presubmits.yaml](https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes-sigs/kubebuilder/kubebuilder-presubmits.yaml). 
-1. Check that all scripts used by the CI are defined in the project.  
+1. See the [action files](.github/workflows) to check its tests, and the scripts used on it.
+1. Note that the prow tests used in the CI are configured in [kubernetes-sigs/kubebuilder/kubebuilder-presubmits.yaml](https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes-sigs/kubebuilder/kubebuilder-presubmits.yaml).
+1. Check that all scripts used by the CI are defined in the project.
 
 ## How to contribute to docs
 
@@ -102,7 +104,7 @@ The docs are published off of three branches:
   legacy docs
 - `master`:
   [master.book.kubebuilder.io](https://master.book.kubebuilder.io) --
-  "nightly" docs 
+  "nightly" docs
 
 See [VERSIONING.md](VERSIONING.md#book-releases) for more information.
 
@@ -129,3 +131,5 @@ KubeBuilder and the related repositories.  See
 Participation in the Kubernetes community is governed by the [Kubernetes Code of Conduct](code-of-conduct.md).
 
 [golangci]:https://github.com/golangci/golangci-lint
+[kind]:https://kind.sigs.k8s.io/#installation-and-usage
+[setup-envtest]:https://book.kubebuilder.io/reference/envtest

Makefile

@@ -101,13 +101,14 @@ go-apidiff:
 test: test-unit test-integration test-testdata test-book ## Run the unit and integration tests (used in the CI)
 
 .PHONY: test-unit
+TEST_PKGS := ./pkg/... ./test/e2e/utils/...
 test-unit: ## Run the unit tests
-	go test -race -v ./pkg/...
+	go test -race $(TEST_PKGS)
 
 .PHONY: test-coverage
 test-coverage: ## Run unit tests creating the output to report coverage
 	- rm -rf *.out  # Remove all coverage files if exists
-	go test -race -failfast -tags=integration -coverprofile=coverage-all.out -coverpkg="./pkg/cli/...,./pkg/config/...,./pkg/internal/...,./pkg/machinery/...,./pkg/model/...,./pkg/plugin/...,./pkg/plugins/golang" ./pkg/...
+	go test -race -failfast -tags=integration -coverprofile=coverage-all.out -coverpkg="./pkg/cli/...,./pkg/config/...,./pkg/internal/...,./pkg/machinery/...,./pkg/model/...,./pkg/plugin/...,./pkg/plugins/golang" $(TEST_PKGS)
 
 .PHONY: test-integration
 test-integration: ## Run the integration tests

test/common.sh

@@ -27,7 +27,7 @@ if [ -n "$TRACE" ]; then
   set -x
 fi
 
-k8s_version=1.19.2
+tools_k8s_version=1.19.2
 kind_version=0.11.1
 goarch=amd64
 
@@ -59,8 +59,14 @@ function header_text {
   echo "$header$*$reset"
 }
 
+# Certain tools are installed to GOBIN.
+export PATH="$(go env GOPATH)/bin:${PATH}"
+
+# Kubebuilder's bin path should be the last added to PATH such that it is preferred.
 tmp_root=/tmp
 kb_root_dir=$tmp_root/kubebuilder
+mkdir -p "$kb_root_dir"
+export PATH="${kb_root_dir}/bin:${PATH}"
 
 # Skip fetching and untaring the tools by setting the SKIP_FETCH_TOOLS variable
 # in your environment to any value:
@@ -71,66 +77,40 @@ kb_root_dir=$tmp_root/kubebuilder
 # machine, but rebuild the kubebuilder and kubebuilder-bin binaries.
 SKIP_FETCH_TOOLS=${SKIP_FETCH_TOOLS:-""}
 
-# Remove previously built binary and fetched tools if they need to be fetched again
-function prepare_staging_dir {
-  header_text "Preparing staging dir"
-
-  if [ -z "$SKIP_FETCH_TOOLS" ]; then
-    rm -rf "$kb_root_dir"
-  else
-    rm -f "$kb_root_dir/bin/kubebuilder"
-  fi
-}
-
 # Build kubebuilder
 function build_kb {
   header_text "Building kubebuilder"
-  go build -o $kb_root_dir/bin/kubebuilder ./cmd
-  kb=$kb_root_dir/bin/kubebuilder
+
+  go build -o "${kb_root_dir}/bin/kubebuilder" ./cmd
+  kb="${kb_root_dir}/bin/kubebuilder"
 }
 
-# Fetch k8s API gen tools and make it available under kb_root_dir/bin.
+# Fetch k8s API tools and manage them globally with setup-envtest.
 function fetch_tools {
-  if [ -z "$SKIP_FETCH_TOOLS" ]; then
-    header_text "Fetching kb tools"
-    kb_tools_archive_name="kubebuilder-tools-$k8s_version-$goos-$goarch.tar.gz"
-    kb_tools_download_url="https://storage.googleapis.com/kubebuilder-tools/$kb_tools_archive_name"
-
-    kb_tools_archive_path="$tmp_root/$kb_tools_archive_name"
-    if [ ! -f $kb_tools_archive_path ]; then
-      curl -sL ${kb_tools_download_url} -o "$kb_tools_archive_path"
-    fi
-    tar -zvxf "$kb_tools_archive_path" -C "$tmp_root/"
+  if ! is_installed setup-envtest; then
+    header_text "Installing setup-envtest to $(go env GOPATH)/bin"
+
+    go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
   fi
 
-  export KUBEBUILDER_ASSETS=$kb_root_dir/bin/
-}
+  if [ -z "$SKIP_FETCH_TOOLS" ]; then
+    header_text "Installing e2e tools with setup-envtest"
 
-# Check if version of tool is less than or equal a required version
-function is_ver_lessthanequal {
-    [  "$1" = "`echo -e \"$1\n$2\" | sort -V | head -n1`" ]
-}
+    setup-envtest use $tools_k8s_version
+  fi
 
-# Check if version of tool is less than a required version
-function is_ver_lessthan {
-    [ "$1" = "$2" ] && return 1 || is_ver_lessthanequal $1 $2
+  # Export KUBEBUILDER_ASSETS.
+  eval $(setup-envtest use -i -p env $tools_k8s_version)
+  # Downloaded tools should be used instead of counterparts present in the environment.
+  export PATH="${KUBEBUILDER_ASSETS}:${PATH}"
 }
 
-# Installing kind in a temporal dir if no previously installed
+# Installing kind in a temporal dir if no previously installed to GOBIN.
 function install_kind {
-  header_text "Checking if kind is installed"
   if ! is_installed kind ; then
-    header_text "Kind not found, installing kind"
-    pushd $(mktemp -d)
-    GO111MODULE=on go get sigs.k8s.io/kind@v$kind_version
-    popd
-  else
-    if is_ver_lessthan `kind version -q` $kind_version ; then
-      header_text "Kind version less than v$kind_version, updating kind"
-      pushd $(mktemp -d)
-      GO111MODULE=on go get sigs.k8s.io/kind@v$kind_version
-      popd
-    fi
+    header_text "Installing kind to $(go env GOPATH)/bin"
+
+    go install sigs.k8s.io/kind@v$kind_version
   fi
 }
 

test/e2e/setup.sh

@@ -15,7 +15,6 @@
 # limitations under the License.
 
 build_kb
-export PATH=$kb_root_dir/bin:$PATH
 fetch_tools
 install_kind
 

test/e2e/utils/kubectl.go

@@ -112,11 +112,15 @@ func (vi VersionInfo) GetMajorInt() uint64 { return vi.major }
 func (vi VersionInfo) GetMinorInt() uint64 { return vi.minor }
 
 func (vi *VersionInfo) parseVersionInts() (err error) {
-	if vi.major, err = strconv.ParseUint(vi.Major, 10, 64); err != nil {
-		return err
+	if vi.Major != "" {
+		if vi.major, err = strconv.ParseUint(vi.Major, 10, 64); err != nil {
+			return err
+		}
 	}
-	if vi.minor, err = strconv.ParseUint(vi.Minor, 10, 64); err != nil {
-		return err
+	if vi.Minor != "" {
+		if vi.minor, err = strconv.ParseUint(vi.Minor, 10, 64); err != nil {
+			return err
+		}
 	}
 	return nil
 }
@@ -127,24 +131,35 @@ type KubernetesVersion struct {
 	ServerVersion VersionInfo `json:"serverVersion,omitempty"`
 }
 
-func (v *KubernetesVersion) prepare() (err error) {
-	if err = v.ClientVersion.parseVersionInts(); err != nil {
+func (v *KubernetesVersion) prepare() error {
+	if err := v.ClientVersion.parseVersionInts(); err != nil {
+		return err
+	}
+	if err := v.ServerVersion.parseVersionInts(); err != nil {
 		return err
 	}
-	return v.ServerVersion.parseVersionInts()
+	return nil
 }
 
 // Version is a func to run kubectl version command
 func (k *Kubectl) Version() (ver KubernetesVersion, err error) {
-	var out string
-	if out, err = k.Command("version", "-o", "json"); err != nil {
-		return KubernetesVersion{}, err
-	}
-	if err = json.Unmarshal([]byte(out), &ver); err != nil {
+	out, err := k.Command("version", "-o", "json")
+	if err != nil {
 		return KubernetesVersion{}, err
 	}
-	if err = ver.prepare(); err != nil {
+	if err := ver.decode(out); err != nil {
 		return KubernetesVersion{}, err
 	}
 	return ver, nil
 }
+
+func (v *KubernetesVersion) decode(out string) (err error) {
+	dec := json.NewDecoder(strings.NewReader(out))
+	if err := dec.Decode(&v); err != nil {
+		return err
+	}
+	if err := v.prepare(); err != nil {
+		return err
+	}
+	return nil
+}

test/e2e/utils/kubectl_test.go

@@ -0,0 +1,134 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package utils
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Kubectl", func() {
+	var ver KubernetesVersion
+	AfterEach(func() {
+		ver = KubernetesVersion{}
+	})
+	Context("successful 'kubectl version' output", func() {
+		It("decodes both client and server versions", func() {
+			Expect(ver.decode(clientServerOutput)).To(Succeed())
+			Expect(ver.ClientVersion.major).To(BeNumerically("==", 1))
+			Expect(ver.ClientVersion.minor).To(BeNumerically("==", 21))
+			Expect(ver.ServerVersion.major).To(BeNumerically("==", 1))
+			Expect(ver.ServerVersion.minor).To(BeNumerically("==", 21))
+		})
+		It("decodes only client version", func() {
+			Expect(ver.decode(clientOnlyOutput)).To(Succeed())
+			Expect(ver.ClientVersion.major).To(BeNumerically("==", 1))
+			Expect(ver.ClientVersion.minor).To(BeNumerically("==", 21))
+			Expect(ver.ServerVersion.major).To(BeNumerically("==", 0))
+			Expect(ver.ServerVersion.minor).To(BeNumerically("==", 0))
+		})
+	})
+	Context("'kubectl version' output with non-JSON text", func() {
+		It("handles warning logs", func() {
+			Expect(ver.decode(clientServerWithWarningOutput)).To(Succeed())
+			Expect(ver.ClientVersion.major).To(BeNumerically("==", 1))
+			Expect(ver.ClientVersion.minor).To(BeNumerically("==", 21))
+			Expect(ver.ServerVersion.major).To(BeNumerically("==", 1))
+			Expect(ver.ServerVersion.minor).To(BeNumerically("==", 21))
+		})
+	})
+	Context("with error text", func() {
+		It("returns an error", func() {
+			Expect(ver.decode(errorOutput)).NotTo(Succeed())
+		})
+	})
+})
+
+const clientServerOutput = `
+{
+  "clientVersion": {
+    "major": "1",
+    "minor": "21",
+    "gitVersion": "v0.21.0-beta.1",
+    "gitCommit": "0d10c3f72592addce965b9bb34992eb6fc283a3b",
+    "gitTreeState": "clean",
+    "buildDate": "2021-08-31T22:03:33Z",
+    "goVersion": "go1.16.6",
+    "compiler": "gc",
+    "platform": "linux/amd64"
+  },
+  "serverVersion": {
+    "major": "1",
+    "minor": "21",
+    "gitVersion": "v1.21.1",
+    "gitCommit": "5e58841cce77d4bc13713ad2b91fa0d961e69192",
+    "gitTreeState": "clean",
+    "buildDate": "2021-05-18T01:10:20Z",
+    "goVersion": "go1.16.4",
+    "compiler": "gc",
+    "platform": "linux/amd64"
+  }
+}
+`
+
+const clientOnlyOutput = `
+{
+  "clientVersion": {
+    "major": "1",
+    "minor": "21",
+    "gitVersion": "v0.21.0-beta.1",
+    "gitCommit": "0d10c3f72592addce965b9bb34992eb6fc283a3b",
+    "gitTreeState": "clean",
+    "buildDate": "2021-08-31T22:03:33Z",
+    "goVersion": "go1.16.6",
+    "compiler": "gc",
+    "platform": "linux/amd64"
+  }
+}
+`
+
+const clientServerWithWarningOutput = `
+{
+  "clientVersion": {
+    "major": "1",
+    "minor": "21",
+    "gitVersion": "v0.21.0-beta.1",
+    "gitCommit": "0d10c3f72592addce965b9bb34992eb6fc283a3b",
+    "gitTreeState": "clean",
+    "buildDate": "2021-08-31T22:03:33Z",
+    "goVersion": "go1.16.6",
+    "compiler": "gc",
+    "platform": "linux/amd64"
+  },
+  "serverVersion": {
+    "major": "1",
+    "minor": "21",
+    "gitVersion": "v1.21.1",
+    "gitCommit": "5e58841cce77d4bc13713ad2b91fa0d961e69192",
+    "gitTreeState": "clean",
+    "buildDate": "2021-05-18T01:10:20Z",
+    "goVersion": "go1.16.4",
+    "compiler": "gc",
+    "platform": "linux/amd64"
+  }
+}
+WARNING: version difference between client (0.21) and server (1.21) exceeds the supported minor version skew of +/-1
+`
+
+const errorOutput = `
+ERROR: reason blah blah
+`