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.

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

Drop Dockerfile in test images #1792

Merged
merged 10 commits into from
Aug 10, 2018
Merged
Show file tree
Hide file tree
Changes from 8 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
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
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 -P "github.com/knative/serving/test/test_images/$(basename ${image_dir})"
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it also necessary in the upload-test-images.sh script?

Copy link
Member

Choose a reason for hiding this comment

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

It is notable that this will not work with DockerHub (and possible other registries), which is why I'd changed the ko default behavior :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall I fix this separately?

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
2 changes: 1 addition & 1 deletion test/e2e/helloworld_shell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
)

const (
appYaml = "test_images/helloworld/helloworld.yaml"
appYaml = "../test_images/helloworld/helloworld.yaml"
yamlImagePlaceholder = "github.com/knative/serving/test_images/helloworld"
ingressTimeout = 5 * time.Minute
servingTimeout = 2 * time.Minute
Expand Down
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
12 changes: 4 additions & 8 deletions test/upload-test-images.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,11 @@

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"}

DOCKER_FILES="$(find $@ -name Dockerfile)"
: ${DOCKER_FILES:?"No subdirectories with Dockerfile files found in $@"}
export KO_DOCKER_REPO=${DOCKER_REPO_OVERRIDE}
IMAGE_DIRS="$(find $(dirname $0)/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
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