[merged] make vagrant-check: Run tests with vagrant by baude · Pull Request #736 · projectatomic/atomic

baude · 2016-10-27T18:54:38Z

Run tests in a vagrant session (w/libvirt for now). This allows
the tests to occur similar to the way our jenkins tests work. Run
make vagrant-check and we created an F24 session and install
the proper deps. We then call the .redhat-ci.sh script inside
the vagrant session and the tests are executed. At this time, you
will need to vagrant halt to shutdown the session when comlete.

jlebon · 2016-10-27T20:07:12Z

Vagrantfile

+
+  # Every Vagrant development environment requires a box. You can search for
+  # boxes at https://atlas.hashicorp.com/search.
+  config.vm.box = "fedora/24-cloud-base"


Why not fedora/24-atomic-host here? Or even better, define all three (the CentOS box is called centos/atomic-host). Someone can then choose to only bring up a single one, e.g. vagrant up fedora/24/atomic.

Well, two quick reasons.

If I understood you correctly, the CentOS and Fedora Atomic host required greater preparations than simply dnf install coverage and atomic. And that is normally handled specially by your tools right which arent in play local use.

Aim small, miss small :)

In all seriousness, if you think I should pursue, I'm happy to do so. I think the Makefile would then need to call a bash script instead of trying to handle this all in make. Thoughts?

If I understood you correctly, the CentOS and Fedora Atomic host required greater preparations than simply dnf install coverage and atomic. And that is normally handled specially by your tools right which arent in play local use.

For fedora/24/atomic, we don't do any special handling. So the rest of your Vagrantfile should just work as is (minus the dnf install part). For CentOS, hmmm... yeah that's an issue. You'd have to rebase to CAHC, since the official release lags so much behind (or layer all the new reqs). I researched this a while back (we have a similar setup in rpm-ostree), and rebasing and rebooting as part of the Vagrant provisioning process was not straightforward.

Aim small, miss small :)

Agreed! This is definitely a good start. How about we start with just fedora/24-atomic-host, since I'd say this project holds more importance on AH than in the Cloud variant. Plus, I assume most developers here are not using an Atomic Host to hack on atomic, so this would be an easy way to provide them that environment. WDYT?

Agree. I'll fixer it up.

We actually have Vagrant boxes generated for CAHC (both continuous and alpha) btw, and I switched to using CAHC Alpha in coreos/rpm-ostree#502

rhatdan · 2016-10-27T21:02:40Z

SGTM

cgwalters · 2016-10-28T13:04:01Z

See also coreos/rpm-ostree#502

jlebon

👍

baude · 2016-10-31T19:18:39Z

Rebasing

miabbott · 2016-11-01T15:14:21Z

One thing of note, if you run make vagrant-check in succession, you might get an error like this:

$ make vagrant-check
vagrant up "fedora_atomic"
Bringing machine 'fedora_atomic' up with 'libvirt' provider...
vagrant ssh "fedora_atomic" -c "cd /home/vagrant/atomic && sudo sh ./.redhat-ci.sh"
+ mount --make-rshared /
+ systemctl start docker
+ '[' -f /run/ostree-booted ']'
+ ostree admin unlock
error: Deployment is already in unlocked state: development
Connection to 192.168.121.23 closed.
Makefile:97: recipe for target 'vagrant-check' failed
make: *** [vagrant-check] Error 1

I think you'll have to vagrant destroy before re-running it.

miabbott · 2016-11-01T18:23:01Z

vagrant.sh

+
+
+is_running() {
+    status=$(vagrant status | grep fedora_atomic | awk '{print $2}')


You'll have to change this to grep for ${BOX} since it is selectable

Got it, good catch @miabbott

miabbott · 2016-11-01T18:35:32Z

vagrant.sh

+        exit 1
+    fi
+}
+


Maybe add some logic before running the bulk of the script to validate that the value of ${BOX} is supported? Some usage feedback with supported values would go nicely here, too.

Added a check.

miabbott · 2016-11-01T18:39:29Z

vagrant.sh

@@ -0,0 +1,31 @@
+#!/bin/bash
+


set -euo pipefail would be a nice addition

miabbott · 2016-11-01T18:41:27Z

vagrant.sh

+tee_file="${BOX}_${timestamp}.log"
+is_running
+vagrant up ${BOX}
+vagrant ssh ${BOX} -c "cd /home/vagrant/atomic && sudo sh ./.redhat-ci.sh"


I think this needs to be piped to tee?

The piping to tee seems to break $? reflecting the actual test result.

baude · 2016-11-02T20:21:40Z

@rhatdan this is good to go now, not sure what is up with the ci tests

jlebon · 2016-11-03T11:39:05Z

vagrant.sh

+    status=$(vagrant status | grep ${BOX} | awk '{print $2}')
+    if [ ${status} == "running" ]; then
+        echo ""
+        echo "'${BOX}' is already running.  Perform a 'vagrant halt|destroy ${BOX}'"


Rather than having to destroy and bring back up each time, can't we just vagrant rsync here if we see it's already up?

How do I know if it is already running a test? I defaulted to this because I really dont know the existing state if it is running. Thoughts? @rhatdan

We can leave that to the user, no? I.e. expect them to know when the previous make vagrant-check is done before starting a new one. Or are you thinking of another use case here?

@jlebon OK I have implemented your suggestion but I hit ...

/usr/share/gems/gems/net-ssh-2.9.1/lib/net/ssh/transport/session.rb:67:in `initialize': Object#timeout is deprecated, use Timeout.timeout instead. /usr/share/gems/gems/net-ssh-2.9.1/lib/net/ssh/transport/session.rb:84:in`initialize': Object#timeout is deprecated, use Timeout.timeout instead. ==> fedora_atomic: Rsyncing folder: /home/bbaude/docker/atomic/ => /home/vagrant/atomic ==> fedora_atomic: - Exclude: [".vagrant/", ".tmp*"] - mount --make-rshared / - systemctl start docker - '[' -f /run/ostree-booted ']' - ostree admin unlock error: Deployment is already in unlocked state: development Connection to 192.168.121.217 closed. *\* Test failed. Leaving 'fedora_atomic' running for debug. *\* Be sure to halt or destroy prior to re-running the check *\* Logs are stored at fedora_atomic_2016_11_03_09_34.log Makefile:97: recipe for target 'vagrant-check' failed make: **\* [vagrant-check] Error 1

So can your rh ci script handle this?

Sure, we just need to tweak the unlock part. Maybe e.g.:

if [ -f /run/ostree-booted ]; then ostree admin unlock || : fi

Or maybe if we want to be more correct, e.g. check rpm-ostree status --json or whether a /var/tmp/ostree-unlock-ovl.* dir exists.

rhatdan · 2016-11-07T15:34:33Z

@miabbott @cgwalters Are you guys happy with this?

cgwalters · 2016-11-07T15:50:53Z

Down the line we'll definitely want to do more dedup across projects and such, but this seems sane to me to start.

cgwalters · 2016-11-07T15:51:08Z

Why does the PR have a merge from master instead of being rebased though?

jlebon · 2016-11-07T15:55:06Z

Yup, looks good on my end as well.

rhatdan · 2016-11-07T15:58:29Z

@baude REBASE please and then we can merge.

Run tests in a vagrant session (w/libvirt for now). This allows the tests to occur similar to the way our jenkins tests work. Run make vagrant-check and we created an F24 session and install the proper deps. We then call the .redhat-ci.sh script inside the vagrant session and the tests are executed. At this time, you will need to vagrant halt to shutdown the session when comlete. By default, the tests will exeute on an instance of fedora-24-atomic. You can test on Colin's CAHC centos-atomic by: make vagrant-check BOX=centos_atomic

baude · 2016-11-07T16:21:49Z

@rhatdan rebased

rhatdan · 2016-11-07T16:22:56Z

@rh-atomic-bot r+

rh-atomic-bot · 2016-11-07T16:22:57Z

📌 Commit 671ed30 has been approved by rhatdan

rh-atomic-bot · 2016-11-07T16:23:04Z

⌛ Testing commit 671ed30 with merge b982674...

Run tests in a vagrant session (w/libvirt for now). This allows the tests to occur similar to the way our jenkins tests work. Run make vagrant-check and we created an F24 session and install the proper deps. We then call the .redhat-ci.sh script inside the vagrant session and the tests are executed. At this time, you will need to vagrant halt to shutdown the session when comlete. By default, the tests will exeute on an instance of fedora-24-atomic. You can test on Colin's CAHC centos-atomic by: make vagrant-check BOX=centos_atomic Closes: #736 Approved by: rhatdan

rh-atomic-bot · 2016-11-07T16:43:32Z

💔 Test failed - status-atomicjenkins

baude · 2016-11-07T17:00:59Z

@rh-atomic-bot retry

rh-atomic-bot · 2016-11-07T17:01:09Z

⌛ Testing commit 671ed30 with merge 8ea8d1e...

rh-atomic-bot · 2016-11-07T17:11:21Z

☀️ Test successful - status-atomicjenkins
Approved by: rhatdan
Pushing 8ea8d1e to master...

jlebon reviewed Oct 27, 2016

View reviewed changes

baude force-pushed the vagrant branch from 3498170 to a473c99 Compare October 31, 2016 17:35

jlebon approved these changes Oct 31, 2016

View reviewed changes

baude force-pushed the vagrant branch from a473c99 to 3db8ec4 Compare October 31, 2016 19:18

baude force-pushed the vagrant branch 3 times, most recently from 4890fd2 to f6ea503 Compare November 1, 2016 18:08

miabbott reviewed Nov 1, 2016

View reviewed changes

vagrant.sh

@@ -0,0 +1,31 @@

#!/bin/bash

Copy link

Contributor

miabbott Nov 1, 2016

Choose a reason for hiding this comment

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

set -euo pipefail would be a nice addition

miabbott reviewed Nov 1, 2016

View reviewed changes

baude force-pushed the vagrant branch 3 times, most recently from 01c70db to 5ccc8de Compare November 2, 2016 19:14

jlebon reviewed Nov 3, 2016

View reviewed changes

baude force-pushed the vagrant branch 2 times, most recently from 3f03d87 to 0a4038e Compare November 3, 2016 18:16

baude force-pushed the vagrant branch from c1bf324 to 671ed30 Compare November 7, 2016 16:21

rh-atomic-bot changed the title ~~make vagrant-check: Run tests with vagrant~~ [merged] make vagrant-check: Run tests with vagrant Nov 7, 2016

rh-atomic-bot closed this in 8ea8d1e Nov 7, 2016



		is_running() {
		status=$(vagrant status \| grep fedora_atomic \| awk '{print $2}')

Conversation

baude commented Oct 27, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baude Oct 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhatdan commented Oct 27, 2016

Uh oh!

cgwalters commented Oct 28, 2016

Uh oh!

jlebon left a comment

Choose a reason for hiding this comment

Uh oh!

baude commented Oct 31, 2016

Uh oh!

miabbott commented Nov 1, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baude commented Nov 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhatdan commented Nov 7, 2016

Uh oh!

cgwalters commented Nov 7, 2016

Uh oh!

cgwalters commented Nov 7, 2016

Uh oh!

jlebon commented Nov 7, 2016

Uh oh!

rhatdan commented Nov 7, 2016

Uh oh!

baude commented Nov 7, 2016

Uh oh!

rhatdan commented Nov 7, 2016

Uh oh!

rh-atomic-bot commented Nov 7, 2016

Uh oh!

rh-atomic-bot commented Nov 7, 2016

Uh oh!

rh-atomic-bot commented Nov 7, 2016

Uh oh!

baude commented Nov 7, 2016

Uh oh!

rh-atomic-bot commented Nov 7, 2016

Uh oh!

rh-atomic-bot commented Nov 7, 2016

baude Oct 27, 2016 •

edited

Loading

miabbott commented Nov 1, 2016 •

edited

Loading