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

Use Docker for testing on Travis, start on 16.04 support #649

Merged
merged 7 commits into from May 5, 2017

Conversation

@aneeshusa
Copy link
Member

aneeshusa commented Apr 27, 2017

Use pinned Docker images to test our states on multiple OSes.
Currently, only servo-linux1 is tested, on Ubuntu 14.04 and 16.04.
Also note that some states are not tested since they don't make
sense in a Docker container, mainly the service states;
these still need to be updated for systemd.

This starts updating the Salt states for Ubuntu 16.04 compatibility,
and thus helps with #462.

Also, make the tests run on Python 3.4, which is the default Python 3 on 14.04.
See commit messages for some other miscellaneous changes.


This change is Reviewable

aneeshusa added 3 commits Apr 17, 2017
This can potentially cause security issues.
Ubuntu 16.04 Docker images don't come with sudo.
@aneeshusa
Copy link
Member Author

aneeshusa commented Apr 27, 2017

Copy link
Contributor

metajack left a comment

Looks good overall.

- SALT_DOCKER_IMAGE=ubuntu@sha256:f3a61450ae43896c4332bda5e78b453f4a93179045f20c8181043b26b5e79028
os: linux
sudo: required
dist: trusty

This comment has been minimized.

@metajack

metajack Apr 27, 2017

Contributor

It's no longer Trusty but Xenial, no? Or are we still doing xenial testing within docker on trusty travis machines?

This comment has been minimized.

@aneeshusa

aneeshusa Apr 29, 2017

Author Member

Travis doesn't have Xenial machines, so we're using Xenial Docker containers on their Trusty VMs.

}

SUDO=""

This comment has been minimized.

@metajack

metajack Apr 27, 2017

Contributor

Instead of duplicating this code, maybe we need a common.bash that these files include.

This comment has been minimized.

@aneeshusa

aneeshusa Apr 29, 2017

Author Member

I thought about doing that, but I don't think it's worth it (yet) since it's only a few lines of duplication.
Having another file also breaks the curl install.sh | bash workflow, which is nice to setup another machine.

test.py Outdated
python_scripts = filter(is_python_script, os.scandir(test_dir))
tests = sorted([entry.name for entry in python_scripts])

# TODO(aneeshusa): switch back to scandirr

This comment has been minimized.

@metajack

metajack Apr 27, 2017

Contributor

I'd probably add a ntoe that this is needed because scandir is from python 3.5. Otherwise this knowledge of what the TODO item is will be lost.

@@ -14,6 +14,7 @@ buildbot-master:
- twisted == 16.6.0 # NOTE: keep in sync with buildbot-slave sls
- require:
- pkg: pip
{% if grains.get('virtual_subtype', '') != 'Docker' %}

This comment has been minimized.

@metajack

metajack Apr 27, 2017

Contributor

Why can't we get systemd running inside docker? Not having service.running seems not great, and certainly pollutes the states.

This comment has been minimized.

@aneeshusa

aneeshusa Apr 29, 2017

Author Member

The service.running states underneath call various commands that seem to want to do IPC to a running instance of the int system (Upstart or Systemd). In the container, there is no init process running (just the dispatch.sh inside bash), so the states fail to run. There are purportedly ways to make it all work, but I haven't found a clear guide yet, and I'd like to add at least a basic smoke test now with the option to add service.running support later.

I agree this isn't ideal, so I'm open to other ways to handle this.

aneeshusa added 4 commits Apr 26, 2017
Ubuntu Trusty's Python 3 package supplies 3.4,
so this makes it possibly to run the tests on Trusty machines
without manually installing Python 3.5.

To allow running inside Docker,
create our own virtualenv for Python test dependencies (e.g. `jinja2`).
Don't use the virtualenvs supplied by Travis.
Make sure to install some extra Python bits
that aren't installed by default for Docker.

Rewrite testing code to avoid APIs introduced in Python 3.5.
Use block indent and early return in tests.
More work is needed to support the cross build states.
`service.running` requires talking to init,
which is not running inside Docker.

`timezone.system` on systemd distros uses timedatectl
which also wants to talk to a running systemd,
so skip that on Docker as well.
Use pinned Docker images for 14.04 and 16.04.
Re-exec the dispatch script inside a running container
to simulate running the tests on other OSes.

This is not perfect since various states do
not function properly in containers
(mainly service-related states),
but is still better than nothing
and tests package installation, etc.
@aneeshusa aneeshusa force-pushed the aneeshusa:use-docker-for-testing-on-travis branch from 96fc469 to e7698df Apr 29, 2017
@metajack
Copy link
Contributor

metajack commented May 5, 2017

@bors-servo r+

Please file a bug about fixing the systemd issue. I'd like to make sure we eventually remove all those ifdefs :)

@bors-servo
Copy link
Contributor

bors-servo commented May 5, 2017

📌 Commit e7698df has been approved by metajack

@bors-servo
Copy link
Contributor

bors-servo commented May 5, 2017

Testing commit e7698df with merge 85d4b83...

bors-servo added a commit that referenced this pull request May 5, 2017
…tajack

Use Docker for testing on Travis, start on 16.04 support

Use pinned Docker images to test our states on multiple OSes.
Currently, only `servo-linux1` is tested, on Ubuntu 14.04 and 16.04.
Also note that some states are not tested since they don't make
sense in a Docker container, mainly the `service` states;
these still need to be updated for systemd.

This starts updating the Salt states for Ubuntu 16.04 compatibility,
and thus helps with #462.

Also, make the tests run on Python 3.4, which is the default Python 3 on 14.04.
See commit messages for some other miscellaneous changes.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/649)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 5, 2017

☀️ Test successful - status-travis
Approved by: metajack
Pushing 85d4b83 to master...

@bors-servo bors-servo merged commit e7698df into servo:master May 5, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.