Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Separate E2E and operator tests #1540

Merged
merged 4 commits into from
May 28, 2020
Merged

Separate E2E and operator tests #1540

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2a06d7a
Separate E2E and operator tests
May 27, 2020
b5dab3d
Extract 'sed' call
May 27, 2020
254199f
consolidation of test script setup
kensipe May 27, 2020
78879d1
adding documentation
kensipe May 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ jobs:
- store_artifacts:
path: kind-logs.tar.bz2

operator-test:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
operator-test:
operators-test:

machine: true
steps:
- checkout
- run: echo 'export INTEGRATION_OUTPUT_JUNIT="true"' >> $BASH_ENV
- run: ./test/run_tests.sh operator-test
- store_test_results:
path: reports/
- store_artifacts:
path: kind-logs.tar.bz2

lint:
docker:
- image: kudobuilder/golang:1.14
Expand Down Expand Up @@ -50,3 +61,6 @@ workflows:
e2e-test:
jobs:
- e2e-test
operator-test:
jobs:
- operator-test
4 changes: 4 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ e2e-test: cli-fast manager-fast
integration-test: cli-fast manager-fast
./hack/run-integration-tests.sh

.PHONY: operator-test
operator-test: cli-fast manager-fast
./hack/run-operator-tests.sh

.PHONY: test-clean
# Clean test reports
test-clean:
Expand Down
19 changes: 0 additions & 19 deletions hack/run-e2e-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,8 @@ then
| tee /dev/fd/2 \
| go-junit-report -set-exit-code \
> reports/kudo_e2e_test_report.xml

# Operators tests
rm -rf operators
git clone https://github.com/kudobuilder/operators
mkdir operators/bin/
cp ./bin/kubectl-kudo operators/bin/
sed "s/%version%/$KUDO_VERSION/" operators/kudo-test.yaml.tmpl > operators/kudo-test.yaml
cd operators && ./bin/kubectl-kudo test --artifacts-dir /tmp/kudo-e2e-test 2>&1 \
| tee /dev/fd/2 \
| go-junit-report -set-exit-code \
> ../reports/kudo_operators_test_report.xml
else
echo "Running E2E tests without junit output"

./bin/kubectl-kudo test --config kudo-e2e-test.yaml

# Operators tests
rm -rf operators
git clone https://github.com/kudobuilder/operators
mkdir operators/bin/
cp ./bin/kubectl-kudo operators/bin/
sed "s/%version%/$KUDO_VERSION/" operators/kudo-test.yaml.tmpl > operators/kudo-test.yaml
cd operators && ./bin/kubectl-kudo test --artifacts-dir /tmp/kudo-e2e-test
fi
39 changes: 39 additions & 0 deletions hack/run-operator-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail
set -o xtrace

INTEGRATION_OUTPUT_JUNIT=${INTEGRATION_OUTPUT_JUNIT:-false}
KUDO_VERSION=${KUDO_VERSION:-test}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need documentation in these scripts that there are preconditions / expectations..
I don't understand the default here... if no KUDO_VERSION is set, shouldn't the script fail? what are we testing if this value isn't properly provided?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to allow users to override KUDO_VERSION. The default value of test is used as a tag for the Docker image that is build and injected to the kind cluster. This is to avoid clashes with existing Docker images as test is only used in this context.


docker build . \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is there a docker build in this script? shouldn't this be part of the precondition of this script? don't we already have a build for docker in make?
docker build is now in 3+ locations now.. can we consolidate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps this docker build is slightly different... where it includes run-test-* scripts. making a need for a different build.
there is a lot of commonality between the run-test scripts and even in 1 script there is duplication that could be cleaned up. It is hard to see if consolidation will make things better or not... I would hate jumping from file to file to track down something simple... today that could mean 1. circle-ci file, 2. makefile, 3. run-script file

The line sed "s/%version%/$KUDO_VERSION/" kudo-e2e-test.yaml.tmpl > kudo-e2e-test.yaml why it has different names across these files... it doesn't need to and they all do the same thing.

I don't see any reason to change the file name to match "operator" tests vs e2e. "operator" tests is another grouping of e2e tests with a different purpose is what I imagine without more clarification.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to do with the test setup that is working around limitations in CircleCI. run-tests.sh starts a Docker container that can run Docker-in-Docker. This container then runs a test script in hack/, e.g. hack/run-operator-tests.sh in this case. The script builds a Docker image of the controller containing the changes of the PR under test and injects this into the kind cluster.
I don't like this complicated test setup and tried to refactor it to be simpler but ran into limitations in CircleCI: We have to run the tests in a VM environment to be able to start privileged containers in this VM. But we also need to install prerequisites for our tests in this VM and the easiest way is trough Docker, using the test/Dockerfile. That's why we have this complicated Docker-in-Docker setup here.

--build-arg ldflags_arg="" \
-t "kudobuilder/controller:$KUDO_VERSION"

if [ "$INTEGRATION_OUTPUT_JUNIT" == true ]
then
echo "Running operator tests with junit output"
mkdir -p reports/
go get github.com/jstemmer/go-junit-report

rm -rf operators
git clone https://github.com/kudobuilder/operators
mkdir operators/bin/
cp ./bin/kubectl-kudo operators/bin/
sed "s/%version%/$KUDO_VERSION/" operators/kudo-test.yaml.tmpl > operators/kudo-test.yaml
cd operators && ./bin/kubectl-kudo test --artifacts-dir /tmp/kudo-e2e-test 2>&1 \
| tee /dev/fd/2 \
| go-junit-report -set-exit-code \
> ../reports/kudo_operators_test_report.xml
else
echo "Running operator tests without junit output"

rm -rf operators
git clone https://github.com/kudobuilder/operators
mkdir operators/bin/
cp ./bin/kubectl-kudo operators/bin/
sed "s/%version%/$KUDO_VERSION/" operators/kudo-test.yaml.tmpl > operators/kudo-test.yaml
nfnt marked this conversation as resolved.
Show resolved Hide resolved
cd operators && ./bin/kubectl-kudo test --artifacts-dir /tmp/kudo-e2e-test
fi
1 change: 1 addition & 0 deletions test/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ COPY pkg/ pkg/
COPY cmd/ cmd/
COPY hack/run-integration-tests.sh hack/run-integration-tests.sh
COPY hack/run-e2e-tests.sh hack/run-e2e-tests.sh
COPY hack/run-operator-tests.sh hack/run-operator-tests.sh
COPY test/ test/
COPY kudo-test.yaml kudo-test.yaml
COPY kudo-e2e-test.yaml.tmpl kudo-e2e-test.yaml.tmpl
2 changes: 1 addition & 1 deletion test/run_tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ INTEGRATION_OUTPUT_JUNIT=${INTEGRATION_OUTPUT_JUNIT:-false}

function archive_logs() {
# Archive test harness artifacts
if [ "$TARGET" == "e2e-test" ]; then
if [ "$TARGET" == "e2e-test" ] || [ "$TARGET" == "operator-test" ]; then
tar -cjvf kind-logs.tar.bz2 kind-logs/
fi
}
Expand Down