add preflight checks integration tests #3816

sosiouxme · 2017-03-30T23:10:49Z

Adds some integration tests that can run preflight checks against inventory made up of throwaway docker containers, and test for expected results.

sosiouxme · 2017-03-31T15:02:32Z

@rhcarvalho The golang part of this is really just running playbooks (in parallel), checking for expected outcomes, and displaying the results nicely. It does seem like the sort of thing that could be done with pytest instead, and prettier.

Re boilerplate, I think I've reduced the setup part about as much as possible. It is probably worth exploring a callback plugin for teardown rather than having the exception-handling block in every test.

I don't know that we need to block this PR in order to finish that work though. Just a couple small things I'd like to do first:

rip out the example tests
get TestPackageAvailabilitySucceeds fixed or skipped

rhcarvalho · 2017-03-31T15:19:56Z

The golang part of this is really just running playbooks (in parallel), checking for expected outcomes, and displaying the results nicely. It does seem like the sort of thing that could be done with pytest instead, and prettier.

The experiment to be done is to use pytest + pytest-xdist. And look out for flakiness.

sosiouxme · 2017-03-31T15:52:44Z

So, yeah... did those little things.

sosiouxme · 2017-04-03T00:27:12Z

The tests all pass if I run go test ./test/integration/...

However with tox -e integration all fail immediately with

                 Task:     openshift_sanitize_inventory : Ensure a valid deployment type has been given.
                 Message:  Please set openshift_deployment_type to one of:
                           origin, online, enterprise, atomic-enterprise, openshift-enterprise

I don't understand what the difference is between the two.

sosiouxme · 2017-04-03T23:59:55Z

Digging a little deeper, tox seems to be running its own ansible, which doesn't work quite the same as the system ansible I get from my path when I run ansible-playbook more directly. I get the same result if I run the vendored ansible. I bumped up debugging and had a closer look. The setup step runs a little differently, and afterward the facts seem to be wiped out. Not sure what's happening here:

                TASK [setup] *******************************************************************
                Using module file /root/openshift-ansible/.tox/integration/lib/python2.7/site-packages/ansible/modules/core/system/setup.py
                <openshift_ansible_test_84089141491733> ESTABLISH DOCKER CONNECTION FOR USER: root
                <openshift_ansible_test_84089141491733> EXEC ['/bin/docker', 'exec', '-i', u'openshift_ansible_test_84089141491733', u'/bin/sh', '-c', u"/bin/sh -c 'echo ~ && sleep 0'"]
                <openshift_ansible_test_84089141491733> EXEC ['/bin/docker', 'exec', '-i', u'openshift_ansible_test_84089141491733', u'/bin/sh', '-c', u'/bin/sh -c \'( umask 77 && mkdir -p "` echo /root/.ansible
/tmp/ansible-tmp-1491254488.22-118473355611698 `" && echo ansible-tmp-1491254488.22-118473355611698="` echo /root/.ansible/tmp/ansible-tmp-1491254488.22-118473355611698 `" ) && sleep 0\'']
                <openshift_ansible_test_84089141491733> PUT /tmp/tmpGaXB9Q TO /root/.ansible/tmp/ansible-tmp-1491254488.22-118473355611698/setup.py
                <openshift_ansible_test_84089141491733> EXEC ['/bin/docker', 'exec', '-i', u'openshift_ansible_test_84089141491733', u'/bin/sh', '-c', u"/bin/sh -c 'chmod u+x /root/.ansible/tmp/ansible-tmp-14912
54488.22-118473355611698/ /root/.ansible/tmp/ansible-tmp-1491254488.22-118473355611698/setup.py && sleep 0'"]
                <openshift_ansible_test_84089141491733> EXEC ['/bin/docker', 'exec', '-i', u'openshift_ansible_test_84089141491733', u'/bin/sh', '-c', u'/bin/sh -c \'/usr/bin/python /root/.ansible/tmp/ansible-tm
p-1491254488.22-118473355611698/setup.py; rm -rf "/root/.ansible/tmp/ansible-tmp-1491254488.22-118473355611698/" > /dev/null 2>&1 && sleep 0\'']
                ok: [openshift_ansible_test_84089141491733]
                
                TASK [openshift_sanitize_inventory : Standardize on latest variable names] *****
                task path: /root/openshift-ansible/roles/openshift_sanitize_inventory/tasks/main.yml:2
                ok: [openshift_ansible_test_84089141491733] => {
                    "ansible_facts": {
                        "deployment_type": "", 
                        "openshift_deployment_type": ""
                    }, 
                    "changed": false, 
                    "invocation": {
                        "module_args": {
                            "deployment_type": "", 
                            "openshift_deployment_type": ""
                        }, 
                        "module_name": "set_fact"
                    }
                }

sosiouxme · 2017-04-04T00:17:44Z

Tests run if I modify requirements.txt to have ansible==2.2.1.0 instead of ansible>=2.2 - argh! Ansible regression apparently...

sosiouxme · 2017-04-04T16:09:10Z

requirements.txt

@@ -1,4 +1,6 @@
-ansible>=2.2
+# ansible 2.2.2.0 inexplicably fails integration tests under tox;


I did get tests to run fine from a source checkout of ansible-2.2.2.0 but tox uses what pip gets so it seems there is still some difference.

What's the error you saw?

described at #3816 (comment)

I really don't want this in here so I'll keep fiddling with it as the CI work comes along.

sosiouxme · 2017-04-05T14:55:58Z

@sdodson this adds integration tests to tox. However they're not running in travis and I assume they should be. How do we make that happen? How do we get them running in the jenkins test/merge also/instead?

Also how do we ensure the local requirements of the tests (docker + golang) are available?

rhcarvalho · 2017-04-05T15:32:39Z

However they're not running in travis and I assume they should be. How do we make that happen? How do we get them running in the jenkins test/merge also/instead?

Hey, we will probably not run them on Travis. Our usage of Travis within the openshift/* organization repos should be and stay light, as we have only (IIRC) 2 concurrent builds for all repos.

As integration tests start taking time to run, we don't want to be affecting Travis builds across all projects.

sosiouxme · 2017-04-05T17:58:09Z

Right, makes sense. So I'm curious why travis isn't trying to run integration tests now? It's just another environment in tox.ini added in this PR.

rhcarvalho · 2017-04-05T21:31:39Z

Right, makes sense. So I'm curious why travis isn't trying to run integration tests now? It's just another environment in tox.ini added in this PR.

Can't see to find it... I have a vague memory that tox only run environments matching the list of defaults:
http://tox.readthedocs.io/en/latest/example/basic.html#a-simple-tox-ini-default-environments

So to run an environment "foo" you have to explicitly select it: tox -e foo; tox -e integration.

rhcarvalho · 2017-04-09T15:48:53Z

requirements.txt

@@ -1,4 +1,6 @@
-ansible>=2.2
+# ansible 2.2.2.0 inexplicably fails integration tests under tox;


What's the error you saw?

rhcarvalho · 2017-04-09T15:54:39Z

test/integration/README.md

+or increase concurrency:
+
+```
+go test ./test/integration/... -p 8 -run TestPackageUpdateDepMissing


-p tells go test how many packages to test in parallel... -parallel operates within a single package.

Either way, I'd leave this discussion off the README and go with the defaults (based on number of CPU cores).

did not know that, interesting. Actually the test script will provide this option, so I'll just leave it out of the README.

rhcarvalho · 2017-04-09T15:56:34Z

test/integration/openshift_health_checker/builds/Dockerfile.test-target-base

@@ -0,0 +1,2 @@
+FROM centos/systemd
+RUN yum install -y iproute python-dbus PyYAML yum-utils


I think I never understood why openshift-ansible doesn't use ansible to install those... @sosiouxme do you happen to know?

rhcarvalho · 2017-04-09T16:01:53Z

test/integration/openshift_health_checker/preflight/playbooks/package_update_dep_missing.yml

+            checks: [ 'package_update' ]
+
+      always:  # destroy the container whether check passed or not
+        - include: ../../teardown_container.yml


We need to isolate setup and teardown from the actual test, to avoid copy-pasting this snippet all the time.

Agreed about teardown, I'm pretty sure that can go in a callback plugin.

For setup, you're always going to need some way to specify the various parameters for the host, which is most of what's copy-pasted. It could be a little cleaner if there were a setup action or something like that.

We might have better luck using the test framework for that, not YAML.

rhcarvalho · 2017-04-09T16:04:22Z

test/integration/openshift_health_checker/preflight/preflight_test.go

+import (
+	"testing"
+
+	. ".."


FTR, this is something I wrote and I don't like it; but to use absolute imports we need to have the code checked out appropriately within a GOPATH.

rhcarvalho · 2017-04-13T10:43:28Z

The unit test failures (incl. Travis) were caused by the way we instantiate action plugins in tests, something that manifested itself only in Ansible 2.3 and fixed in #3919.

sosiouxme · 2017-04-13T12:53:22Z

docker-py is needed on the host. thinking of running this under tox after all...

sosiouxme · 2017-04-13T17:54:21Z

So now the integration tests actually run, and fail the same way they do locally for me. So it's not just something nuts about my setup. Progress!

sosiouxme · 2017-04-17T15:02:01Z

The test runner fails under ansible 2.2.2+ because it no longer sets facts on the test hosts the way it's supposed to. I do not think we want to drag the rest of the tests back to ansible 2.2.1 so I created a separate tox config under test/integration until this bug is fixed. It's not ideal but it keeps this PR moving.

sosiouxme · 2017-04-17T15:17:18Z

Tests ran fine at https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible_tox/60/console

rhcarvalho · 2017-04-18T11:46:23Z

test/integration/run-tests.sh

+STARTTIME=$(date +%s)
+source_root=$(dirname "${0}")
+
+prefix="${PREFIX:-openshift-ansible-integration-}"


I noticed we're defaulting this value in multiple places -- 5 occurrences of openshift-ansible-integration- in this PR. What's the case when we would actually use a different prefix?!

That case is when we're building the images separately and pushing them to a shared registry to be used in tests running in later CI jobs. I wrote this originally expecting that's what we would do, then I learned the shared registry isn't ready yet. Assuming that becomes available, it should just require setting parameters in the Jenkins jobs.

rhcarvalho · 2017-04-18T11:48:47Z

test/integration/run-tests.sh

+  go test ./${source_root#$PWD}/... ${gotest_options} \
+  || retval=$?
+
+ENDTIME=$(date +%s); echo "$0 took $(($ENDTIME - $STARTTIME)) seconds"


I think this is a copy-paste from scripts in Origin... go test already prints how long tests took, we could simplify the script and its output by removing this. WDYT?

Would be fine with me; FWIW this also captures the time to set up the virtualenv. shrug

rhcarvalho · 2017-04-18T11:52:31Z

test/integration/tox.ini

@@ -0,0 +1,14 @@
+[tox]


I see we had a different requirement on the Ansible version because of the issue with delegate_to, but we could probably still get away with a single tox.ini file in the repository. Earlier this year we've unified /tox.ini and /utils/tox.ini to avoid having to maintain two sets of configuration files.

Would be happy to do that. I'm not an expert on tox config :) but it looked to me like that would mean specifying ansible version individually in tox.ini on each environment. Otherwise, if I left the ansible requirement in shared requirements.txt and tried to specify a different one just for integration, it complained about the mismatch. Is there a way to do it?

It's good to keep ansible in requirements.txt, because for a python developer that's the fist place she would look into and setup runtime requirements in a virtual env (unrelated to tox).

The mismatch is most likely related to how pip works so far... I remember seeing an open issue to allow it to resolve required versions when two sources ask for different versions. The workaround is to call pip twice.

How do you get tox to call pip twice with different reqs? Or I suppose we could call it manually to update ansible after tox, though that seems odd to me...

Have pip install ansible==2.2.1.0 be the integration test "command" :)

Yes, that's the workaround. I think we've done that before. Odd, yes :)

@rhcarvalho OK, done! Testing at https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible_tox/61/ but local tests went fine so I expect it to work. Travis already passed.

rhcarvalho · 2017-04-18T11:58:33Z

Tests ran fine at https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible_tox/60/console

@sosiouxme do you know if there's any flag to make the output less verbose?

The interesting line is buried in a ton of useless minutia:

ok  	_/data/src/github.com/openshift/openshift-ansible/test/integration/openshift_health_checker/preflight	140.469s

More interesting than a GREEN test is to see how RED tests look like, and to make sure that when tests fail people can actually understand the output.

sosiouxme · 2017-04-18T13:20:42Z

On Tue, Apr 18, 2017 at 7:58 AM, Rodolfo Carvalho ***@***.***> wrote: @sosiouxme <https://github.com/sosiouxme> do you know if there's any flag to make the output less verbose?

Most of the console output is from the ansible run for setup/teardown, which is from the Jenkins job. If the tests succeeded, it's pretty hard to find the actual test output, but then, you hardly need to. If the tests fail, you can generally find a link or some red text. I wish it could be better sectioned out, perhaps indexed at the top by Jenkins stage.

More interesting than a GREEN test is to see how RED tests look like, and to make sure that when tests fail people can actually understand the output.

Here's a test failure: https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible_tox/58/ I think our output there is pretty good, but we may want to expand the number of lines of output shown on a failing test.

sosiouxme · 2017-04-18T14:00:38Z

aos-ci-test

openshift-bot · 2017-04-18T16:17:03Z

error: aos-ci-jenkins/OS_3.5_containerized for 1b94f60 (logs)

openshift-bot · 2017-04-18T16:32:33Z

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for 1b94f60 (logs)

openshift-bot · 2017-04-18T16:32:54Z

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for 1b94f60 (logs)

openshift-bot · 2017-04-19T15:00:21Z

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for bee527d (logs)

openshift-bot · 2017-04-19T15:00:37Z

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for bee527d (logs)

openshift-bot · 2017-04-19T15:02:38Z

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for bee527d (logs)

openshift-bot · 2017-04-19T15:03:26Z

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for bee527d (logs)

rhcarvalho · 2017-04-25T15:45:38Z

@sosiouxme this needs a rebase.

I haven't had time to try out using pytest-xdist as a test runner. Instead of blocking on this for longer, I'm okay with getting what we have in and following up later.

Please let's do our best to make sure our integration tests don't end up blocking the merge queue.

rhcarvalho · 2017-04-25T15:47:12Z

also, there seems to be room for a git rebase -i to combine fixup commits.

To make room for integration tests.

Make the container setup and teardown more reusable. Remove example tests. Add basic package tests.

Add some scripts that can be run from Jenkins to build/push test images and to run the tests. Updated README to expand on running tests.

sosiouxme · 2017-04-25T16:19:32Z

@rhcarvalho I rebased, and rearranged commits a bit. I think the ones left are the right level of granularity. Will make sure the tests pass, need an approved review to merge...

rhcarvalho · 2017-04-25T16:21:14Z

tox.ini

+    # So for now, install separate ansible version for integration.
+    # PR that fixes it: https://github.com/ansible/ansible/pull/23599
+    # Once that PR is available, drop this and use same ansible.
+    integration: pip install ansible==2.2.1.0


rhcarvalho · 2017-04-25T16:21:20Z

tox.ini

@@ -12,6 +13,7 @@ deps =
    -rrequirements.txt
    -rtest-requirements.txt
    py35-flake8: flake8-bugbear==17.3.0
+    integration: docker-py==1.10.6


sosiouxme · 2017-04-25T17:44:37Z

aos-ci-test

openshift-bot · 2017-04-25T20:25:59Z

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for e5f14b5 (logs)

openshift-bot · 2017-04-25T20:26:27Z

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for e5f14b5 (logs)

openshift-bot · 2017-04-25T20:27:54Z

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for e5f14b5 (logs)

openshift-bot · 2017-04-25T20:28:50Z

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for e5f14b5 (logs)

sosiouxme · 2017-04-25T21:03:53Z

[merge] at last

openshift-bot · 2017-04-25T21:07:35Z

[test]ing while waiting on the merge queue

openshift-bot · 2017-04-25T21:23:27Z

Evaluated for openshift ansible test up to e5f14b5

openshift-bot · 2017-04-25T22:55:31Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible/75/) (Base Commit: d5ec349)

sosiouxme · 2017-04-26T02:21:35Z

Flake openshift/origin#8571
re[merge]

openshift-bot · 2017-04-26T02:23:24Z

Evaluated for openshift ansible merge up to e5f14b5

openshift-bot · 2017-04-26T05:07:27Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_openshift_ansible/293/) (Base Commit: 760bdbc)

sosiouxme mentioned this pull request Mar 30, 2017

[WIP] preflight int tests: generalize and expand rhcarvalho/openshift-ansible#11

Closed

sosiouxme changed the title ~~[WIP] add preflight checks integration tests~~ add preflight checks integration tests Mar 31, 2017

sosiouxme mentioned this pull request Apr 3, 2017

Various preflight check improvements #3649

Merged

sosiouxme commented Apr 4, 2017

View reviewed changes

rhcarvalho reviewed Apr 9, 2017

View reviewed changes

sosiouxme mentioned this pull request Apr 12, 2017

openshift ansible integration tests openshift-eng/aos-cd-jobs#182

Merged

sosiouxme changed the title ~~add preflight checks integration tests~~ [WIP] add preflight checks integration tests Apr 12, 2017

sosiouxme changed the title ~~[WIP] add preflight checks integration tests~~ add preflight checks integration tests Apr 17, 2017

rhcarvalho reviewed Apr 18, 2017

View reviewed changes

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 20, 2017

rhcarvalho and others added 5 commits April 25, 2017 12:13

Move Python unit tests to subdirectory

ff8356b

To make room for integration tests.

Add stub of preflight integration tests

634a895

preflight int tests: generalize; add tests

75f0c57

Make the container setup and teardown more reusable. Remove example tests. Add basic package tests.

preflight int tests: define image builds to support tests

ce4c2f0

integration tests: add CI scripts

e5f14b5

Add some scripts that can be run from Jenkins to build/push test images and to run the tests. Updated README to expand on running tests.

rhcarvalho approved these changes Apr 25, 2017

View reviewed changes

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 25, 2017

openshift-bot merged commit c12b009 into openshift:master Apr 26, 2017

sosiouxme deleted the 20170328-integration-tests branch May 3, 2017 02:20

rhcarvalho mentioned this pull request May 5, 2017

Integration tests stub #2951

Closed

		@@ -1,4 +1,6 @@
		ansible>=2.2
		# ansible 2.2.2.0 inexplicably fails integration tests under tox;

		@@ -0,0 +1,2 @@
		FROM centos/systemd
		RUN yum install -y iproute python-dbus PyYAML yum-utils

add preflight checks integration tests #3816

add preflight checks integration tests #3816

Conversation

sosiouxme commented Mar 30, 2017 • edited Loading

sosiouxme commented Mar 31, 2017 • edited Loading

rhcarvalho commented Mar 31, 2017

sosiouxme commented Mar 31, 2017

sosiouxme commented Apr 3, 2017 • edited Loading

sosiouxme commented Apr 3, 2017 • edited Loading

sosiouxme commented Apr 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sosiouxme commented Apr 5, 2017

rhcarvalho commented Apr 5, 2017

sosiouxme commented Apr 5, 2017 • edited Loading

rhcarvalho commented Apr 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhcarvalho commented Apr 13, 2017 • edited by sosiouxme Loading

sosiouxme commented Apr 13, 2017

sosiouxme commented Apr 13, 2017

sosiouxme commented Apr 17, 2017 • edited Loading

sosiouxme commented Apr 17, 2017

Choose a reason for hiding this comment

sosiouxme Apr 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhcarvalho commented Apr 18, 2017

sosiouxme commented Apr 18, 2017 via email • edited Loading

sosiouxme commented Apr 18, 2017

openshift-bot commented Apr 18, 2017

openshift-bot commented Apr 18, 2017

openshift-bot commented Apr 18, 2017

openshift-bot commented Apr 19, 2017

openshift-bot commented Apr 19, 2017

openshift-bot commented Apr 19, 2017

openshift-bot commented Apr 19, 2017

rhcarvalho commented Apr 25, 2017

rhcarvalho commented Apr 25, 2017

sosiouxme commented Apr 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sosiouxme commented Apr 25, 2017

openshift-bot commented Apr 25, 2017

openshift-bot commented Apr 25, 2017

openshift-bot commented Apr 25, 2017

openshift-bot commented Apr 25, 2017

sosiouxme commented Apr 25, 2017

openshift-bot commented Apr 25, 2017

openshift-bot commented Apr 25, 2017

openshift-bot commented Apr 25, 2017

sosiouxme commented Apr 26, 2017

openshift-bot commented Apr 26, 2017

openshift-bot commented Apr 26, 2017 • edited Loading

sosiouxme commented Mar 30, 2017 •

edited

Loading

sosiouxme commented Mar 31, 2017 •

edited

Loading

sosiouxme commented Apr 3, 2017 •

edited

Loading

sosiouxme commented Apr 3, 2017 •

edited

Loading

sosiouxme commented Apr 5, 2017 •

edited

Loading

rhcarvalho commented Apr 13, 2017 •

edited by sosiouxme

Loading

sosiouxme commented Apr 17, 2017 •

edited

Loading

sosiouxme Apr 18, 2017 •

edited

Loading

sosiouxme commented Apr 18, 2017 via email •

edited

Loading

openshift-bot commented Apr 26, 2017 •

edited

Loading