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

Run google-cloud-python system tests against new runtime images. #39

Merged
merged 13 commits into from
Nov 3, 2016
2 changes: 2 additions & 0 deletions system_tests/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
data/
secrets.tar
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: missing newline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

40 changes: 40 additions & 0 deletions system_tests/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
FROM google/python
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this assume you've just built an image called "google/python" locally?

Choose a reason for hiding this comment

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

Yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice to make this an invalid image name then. If you forgot to build your image or built it with a different name this would give you false positives, since google/python does exist on Dockerhub, but isn't updated at all IIRC.

Choose a reason for hiding this comment

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

Let's file a bug and fix that separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed as #38


# Inject secrets
# TODO: Encrypt/decrypt secrets
ADD data/ data/
ENV GOOGLE_APPLICATION_CREDENTIALS /home/vmagent/app/data/cloud-python-runtime-qa-credentials.json
Copy link
Contributor

Choose a reason for hiding this comment

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

I think /home/vmagent/app is symlinked to /app. It might be easier to just use that path.

Choose a reason for hiding this comment

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

It is indeed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


# Get the source.
RUN git clone https://github.com/GoogleCloudPlatform/google-cloud-python.git
WORKDIR google-cloud-python

# Checkout the latest release.
RUN git checkout $(git describe --tags --abbrev 0)

# Install gcloud so we can do test setup
RUN apt-get update && apt-get install curl
RUN curl https://sdk.cloud.google.com | bash
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to follow these steps instead: https://cloud.google.com/sdk/downloads#versioned

These will ensure you always get the same version of gcloud, which is especially important when you're using commands in the "preview" group.

Choose a reason for hiding this comment

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

Sounds like a good way to end up with a test that constantly runs an outdated SDK. Assuming we run these tests regularly, is there any reason we shouldn't just target the latest?

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as your goal isn't to actually test the SDK it should be fine. I doubt these commands are going to change on you much in practice though.

One other benefit of fetching these tarballs is that the dl.google.com server which is used by the install script has no SLO and will cause flakes eventually. The versioned tarballs are published to GCS, which will have far fewer flakes.

The App Engine team just switched some internal tests over because of flakiness introduced by dl.google.com.

Choose a reason for hiding this comment

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

Actually, @duggelz if you drop the gcloud preview datastore create-indexes system_tests/data/index.yaml you should be able to get by with not installing the SDK at all. Setting GOOGLE_APPLICATION_CREDENTIALS and GCLOUD_PROJECT are sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #40 and GoogleContainerTools/base-images-docker#50 .

Meanwhile, this is good enough for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

ENV PATH ${PATH}:/root/google-cloud-sdk/bin

# System test setup as per google-cloud-python/CONTRIBUTING.rst
RUN gcloud config set project cloud-python-runtime-qa
RUN gcloud components install app-engine-python

Choose a reason for hiding this comment

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

This shouldn't be needed at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deferred as #40

Choose a reason for hiding this comment

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

I'd actually prefer if we went ahead and removed this in this PR. Did you have trouble with just setting GOOGLE_APPLICATION_CREDENTIALS and GCLOUD_PROJECT?

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

RUN gcloud auth activate-service-account \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if you run this docker build inside cloudbuild you might be able to skip all authentication since it'll be running inside GCE and gcloud can auth itself automatically.

Choose a reason for hiding this comment

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

Does cloudcloud give credentials for the current project? What about scopes? Using a service account is predictable auth, so maybe there's some benefit there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the credentials used are a service account in your project. The scopes are cloud-platform I believe, and you can use IAM to grant more permissions to that service account.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deferring this as #40

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

--key-file=${GOOGLE_APPLICATION_CREDENTIALS}
RUN gcloud preview datastore create-indexes system_tests/data/index.yaml
# TODO: python system_tests/clear_datastore.py
# TODO: python system_tests/populate_datastore.py

# Install tox for running the system tests
RUN pip install --upgrade tox

# List of system tests to run. Ideally would be all of them. But
# some of them get permission or other errors that are probably
# related to incomplete test setup.
ENV MODULES="datastore storage speech bigquery pubsub language bigtable"

# Run Python 2.7, 3.4 system tests
# TODO(https://github.com/GoogleCloudPlatform/google-cloud-python/issues/2670)
RUN GOOGLE_CLOUD_TESTS_API_KEY=$(cat /home/vmagent/app/data/cloud-python-runtime-qa-api-key.txt) \
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice to make this a "CMD" instead of a RUN, that way you can build this image once and then run tests several times.

Copy link
Contributor Author

@duggelz duggelz Nov 3, 2016

Choose a reason for hiding this comment

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

I was doing 'CACHE= make' to get the same effect from Docker layer caching. Hmm, the name made more sense in my head ("CACHE=" enables caching?!)

tox -e system-tests,system-tests3 -- ${MODULES}
7 changes: 7 additions & 0 deletions system_tests/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Use no-cache to prevent layer caching because there is a layer that does
# a `git clone` which can not be cached.
CACHE ?= --no-cache

.PHONY: all
all:
docker build $(CACHE) .