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

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

Merged
merged 13 commits into from
Nov 3, 2016
Merged

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

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9aeedd3
Run google-cloud-python system tests against new runtime images.
Nov 3, 2016
3b2abef
Add missing newline
Nov 3, 2016
e3a7cf8
Ignore Emacs autosave files
Nov 3, 2016
87db4d3
Shorten paths (/home/vmagent/app is symlinked to /app)
Nov 3, 2016
c0933d0
Add top-level system-tests target
Nov 3, 2016
5439e04
Fix "checkout latest release", then disable it to always checkout head.
Nov 3, 2016
29c73aa
Enable system tests that work now.
Nov 3, 2016
6960bb4
Revert "Ignore Emacs autosave files"
Nov 3, 2016
2e6f704
Use google-cloud-python from head, not the released version.
Nov 3, 2016
d24cb12
Don't install or use the Google Cloud SDK for system tests.
Nov 3, 2016
d108c5a
Remove cruft
Nov 3, 2016
6ad7fb2
Rename system-tests target to google-cloud-system-tests
Nov 3, 2016
5632d0e
Remove TODOs in favor of opening issues
Nov 3, 2016
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
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Editor
Copy link

Choose a reason for hiding this comment

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

Drop this. Add it to your global gitignore: https://help.github.com/articles/ignoring-files/#create-a-global-gitignore

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.

*~
\#*\#
4 changes: 4 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,7 @@ tests:
.PHONY: benchmarks
benchmarks:
make -C tests benchmarks

.PHONY: system-tests
Copy link

Choose a reason for hiding this comment

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

Go ahead and name this google-cloud-system-tests, as we may want to add system tests for other packages in the future?

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. No doubt we'll need to move files and directories around later when we add more system tests.

system-tests:
make -C system_tests
2 changes: 2 additions & 0 deletions system_tests/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
data/
secrets.tar
41 changes: 41 additions & 0 deletions system_tests/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
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?

Copy link

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.

Copy link

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 /app/data/cloud-python-runtime-qa-credentials.json

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

# Checkout the latest release.
# TODO: Enable once the latest release works
#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.

Copy link

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.

Copy link

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
Copy link

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

Copy link

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.

Copy link

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 /app/data/cloud-python-runtime-qa-api-key.txt) \
tox -e system-tests,system-tests3 -- ${MODULES}
7 changes: 7 additions & 0 deletions system_tests/Makefile
View file
Open in desktop
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) .
3 changes: 2 additions & 1 deletion tests/google-cloud-python/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ 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)
# TODO: Enable once run_unit_tests.py is released
Copy link

Choose a reason for hiding this comment

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

No TODOs in code, please, file an issue instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#41

Do you want to see something like:

# TODO(#41): Enable once run_unit_tests.py is released

Or just not have that comment at all?

Copy link

Choose a reason for hiding this comment

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

Prefer not to have it at all. Put all the needed context in the issue.

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. Asking me to program without TODOs is like asking me to program without my left hand. But I'm always up for a challenge.

#RUN git checkout $(git describe --tags --abbrev=0)

# Run Python 2.7 unit tests
RUN python2.7 scripts/run_unit_tests.py
Expand Down