Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
12 changes: 4 additions & 8 deletions test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ go test -v -tags=e2e -count=1 ./test/e2e --resolvabledomain

### Building the test images

Note: this is only required when you run comformance/e2e tests locally with `go test` commands.

The [`upload-test-images.sh`](./upload-test-images.sh) script can be used to build and push the
test images used by the conformance and e2e tests. It requires:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please note here that this is now only required when running the tests locally.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done


Expand All @@ -110,18 +112,12 @@ test images used by the conformance and e2e tests. It requires:
To run the script for all end to end test images:

```bash
./test/upload-test-images.sh ./test/e2e/test_images ./test/conformance/test_images
./test/upload-test-images.sh
```

### Adding new test images

New test images should be placed in their own subdirectories. Be sure to to include a `Dockerfile`
for building and running the test image.

The new test images will also need to be uploaded to the e2e tests Docker repo. You will need one
of the owners found in [`/test/OWNERS`](OWNERS) to do this.

Because the test images are uploaded to the same folder, they **must** have different names.
New test images should be placed in `./test/test_images`.

## Flags

Expand Down
27 changes: 0 additions & 27 deletions test/conformance/test_images/pizzaplanetv1/Dockerfile

This file was deleted.

27 changes: 0 additions & 27 deletions test/conformance/test_images/pizzaplanetv2/Dockerfile

This file was deleted.

12 changes: 10 additions & 2 deletions test/e2e-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ function dump_extra_cluster_state() {
kubectl logs $(get_app_pod controller knative-serving)
}

function publish_test_images() {
image_dirs="$(find ${REPO_ROOT_DIR}/test/test_images -mindepth 1 -maxdepth 1 -type d)"
for image_dir in ${image_dirs}; do
ko publish "github.com/knative/serving/test/test_images/$(basename ${image_dir})"
done
}

# Script entry point.

initialize $@
Expand All @@ -121,6 +128,8 @@ header "Building and starting Knative Serving"
export KO_DOCKER_REPO=${DOCKER_REPO_OVERRIDE}
create_everything

publish_test_images

# Handle test failures ourselves, so we can dump useful info.
set +o errexit
set +o pipefail
Expand All @@ -138,7 +147,6 @@ options=""
report_go_test \
-v -tags=e2e -count=1 -timeout=20m \
./test/conformance ./test/e2e \
${options} \
-dockerrepo gcr.io/knative-tests/test-images/knative-serving || fail_test
${options} || fail_test

success
7 changes: 0 additions & 7 deletions test/e2e/test_images/README.md

This file was deleted.

6 changes: 0 additions & 6 deletions test/e2e/test_images/autoscale/Dockerfile

This file was deleted.

6 changes: 0 additions & 6 deletions test/e2e/test_images/helloworld/Dockerfile

This file was deleted.

6 changes: 0 additions & 6 deletions test/e2e/test_images/httpproxy/Dockerfile

This file was deleted.

2 changes: 1 addition & 1 deletion test/e2e_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func initializeFlags() *EnvironmentFlags {
flag.StringVar(&f.Cluster, "cluster", defaultCluster,
"Provide the cluster to test against. Defaults to $K8S_CLUSTER_OVERRIDE, then current cluster in kubeconfig if $K8S_CLUSTER_OVERRIDE is unset.")

defaultRepo := os.Getenv("DOCKER_REPO_OVERRIDE")
defaultRepo := path.Join(os.Getenv("DOCKER_REPO_OVERRIDE"), "github.com/knative/serving/test/test_images")
flag.StringVar(&f.DockerRepo, "dockerrepo", defaultRepo,
"Provide the uri of the docker repo you have uploaded the test image to using `uploadtestimage.sh`. Defaults to $DOCKER_REPO_OVERRIDE")

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Test images node

The subdirectories contain the test images used in the conformance tests.
The subdirectories contain the test images used in the conformance and e2e tests.

For details about building and adding new images, see the [section about test
images](/test/README.md#test-images).
Expand Down
13 changes: 5 additions & 8 deletions test/upload-test-images.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,12 @@

set -o errexit

: ${1:?"Pass the directories with the test images as arguments"}
: ${DOCKER_REPO_OVERRIDE:?"You must set 'DOCKER_REPO_OVERRIDE', see DEVELOPMENT.md"}
: ${REPO_ROOT_DIR:?"You must set 'REPO_ROOT_DIR' to knative serving repo root"}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why require users to set this? For developers running local go test ... tests, it seems like a path relative to dirname is more convenient, or at least a reasonable default.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry if I wasn't clear. ${REPO_ROOT_DIR} is a convenience variable set for e2e-tests.sh only. Indeed, use dirname to figure out the location of test_images without adding any further dependency.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. I made the change because you mentioned ${REPO_ROOT_DIR} in this file also.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, my fault, I apologize for the misguidance.


DOCKER_FILES="$(find $@ -name Dockerfile)"
: ${DOCKER_FILES:?"No subdirectories with Dockerfile files found in $@"}
export KO_DOCKER_REPO=${DOCKER_REPO_OVERRIDE}
IMAGE_DIRS="$(find ${REPO_ROOT_DIR}/test/test_images -mindepth 1 -maxdepth 1 -type d)"

for docker_file in ${DOCKER_FILES}; do
image_dir="$(dirname ${docker_file})"
versioned_name="${DOCKER_REPO_OVERRIDE}/knative-serving/$(basename ${image_dir})"
docker build "${image_dir}" -f "${docker_file}" -t "${versioned_name}"
docker push "${versioned_name}"
for image_dir in ${IMAGE_DIRS}; do
ko publish "github.com/knative/serving/test/test_images/$(basename ${image_dir})"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this also have -P to preserve the paths?

done