Automated VCR re-recording #75

cben · 2017-12-10T11:13:31Z

Something I've long wanted, a script for hands-off VCR re-recording 🎉

Can use minishift or (todo: untested) bring your own openshift setting OPENSHIFT_MASTER_HOST env var.

Steps I took for new VCR recording:

Installed minishift v1.9.0, with manageiq plugin added according to https://github.com/ManageIQ/guides/blob/master/providers/openshift.md
I run
```
minishift start --vm-driver virtualbox --openshift-version v3.6.1 --metrics --memory 5G
```
(metrics needed to use same VCR in manageiq-providers-kubernetes, at least as that spec is currently written. lots of RAM needed for metrics. --metrics for v3.7.0 doesn't work yet.)

Then waited for metrics to come up, as confirmed by:

oc get pods -n openshift-infra
curl --insecure "https://$(oc get route -n openshift-infra hawkular-metrics --template '{{.spec.host}}')"

Run the new script:

./spec/vcr_cassettes/manageiq/providers/openshift/container_manager/test_objects_record.sh

git commit, profit :-)

Issues I didn't fix

Caught difference in number of images between old and new refresh! 🐛
We already had a not-fully-explained off by 1 there.
I would like to merge this as pending test and debug later (will open issue).
TODO: Object counts not yet reproducible between runs.
However it's mostly stable
TODO: API_TOKEN gets stored in the VCR (it was in previous cassette, too).
This may be a problem if somebody passes a long-lived env.
- Found https://relishapp.com/vcr/vcr/docs/configuration/filter-sensitive-data, didn't understand how to use yet.
Sometimes project creation errors as it's still cleaning up from project deletion — despite the script waiting. I'll try to report openshift bug, but just re-running this script helps. I decided not to introduce retrying for this, would make script flow much more complicated.

cc @yaacov @enoodle @Ladas

Need this gaprindashvili/yes for improving quotas test
https://bugzilla.redhat.com/show_bug.cgi?id=1504560

yaacov

Nice, LGTM 👍

miq-bot · 2017-12-10T11:24:33Z

This pull request is not mergeable. Please rebase and repush.

enoodle

LGTM pending green tests

Ladas · 2017-12-11T10:01:05Z

@cben looking e.g at ContainerReplicator.count failure

(byebug) pp ContainerReplicator.pluck(:ems_ref, :name)
[["7b36741b-dd83-11e7-8319-169d0f7fea7d", "docker-registry-1"],
 ["7beb37af-dd83-11e7-8319-169d0f7fea7d", "router-1"],
 ["bf9b8058-dd84-11e7-8319-169d0f7fea7d", "my-replicationcontroller-2"],
 ["ea539a31-dd83-11e7-8319-169d0f7fea7d", "hawkular-cassandra-1"],
 ["e518dc66-dd83-11e7-8319-169d0f7fea7d", "hawkular-metrics"],
 ["e61f1832-dd83-11e7-8319-169d0f7fea7d", "heapster"]]

this looks correct, but spec thinks we deleted 2, so there should be 3 total. Is it possible the env. is either not deterministic, or there are more changes happening?

because

expect(ContainerReplicator.find_by(:name => "my-replicationcontroller-0")).to be_nil
expect(ContainerReplicator.find_by(:name => "my-replicationcontroller-1")).to be_nil

would pass, it's just something/somebody created a plenty more ContainerReplicator records in the meantime :-) I expect that is the reason why other counts do not match.

- compare together to get all differences printed together - express counts after deletions as subtractions

Prevents ContainerProject.count "expected: 10 got: 11" failure.

Seems to avoid error that I saw in most runs with 3.7.0 (minishift): Error from server (Forbidden): persistentvolumeclaims "my-persistentvolumeclaim-1" is forbidden: status unknown for quota: my-resource-quota-1 I guess if quota is created last, it's not really enforced, but we don't care.

Env created with minishift v1.9.0: minishift start --vm-driver virtualbox --openshift-version v3.6.1 --metrics --memory 5G Then waited for metrics to come up, as confirmed by: oc get pods -n openshift-infra curl --insecure "https://$(oc get route -n openshift-infra hawkular-metrics --template '{{.spec.host}}')"

cben · 2017-12-25T13:41:45Z

PTAL. Still getting small count difference between recordings, but it's reasonably stable and I want to move on. Added text files which will make "what changed between recordings" much easier to track with future changes...

~~I still need to look into Ladislav's comment above~~. I've since re-recorded multiple times, but I probably have that recording in many-vcrs branch.
=> Obsolete, sounds like metrics was still coming up during that recording.
~~TODO: I'm not sure I have image example I promised Ladislav.~~
=> I don't, hard to capture podwith VCR, instead added refresh_parser_spec captured by watch
Added quotas with scopes to template. May need more changes later but don't know which yet.
I again see image count difference between old and graph refresh. Marked pending, I'm happy I have it caught, debugging and fixing are out of scope for this PR.

moolitayer · 2017-12-26T08:49:11Z

spec/models/manageiq/providers/openshift/container_manager/refresher_spec.rb

+        expect(ContainerProject.count).to eq(object_counts['ContainerProject'])
+        expect(ContainerProject.active.count).to eq(object_counts['ContainerProject'] - 1)
+
+        pending("why graph refresh DELETES 1 image from DB? why old refresh archives 2?")


moolitayer · 2017-12-26T09:06:33Z

spec/models/manageiq/providers/openshift/container_manager/refresher_spec.rb

-    token = 'theToken'
+    # env vars for easier VCR recording, see test_objects_record.sh
+    hostname = ENV["API_HOST"] || "host.example.com"
+    token = ENV["API_TOKEN"] || "theToken"


We have some automation around cluster creation and there these are named:

OPENSHIFT_MASTER_HOST OPENSHIFT_MANAGEMENT_ADMIN_TOKEN

IT could prove useful to have the same names, up to you.

https://github.com/container-mgmt/cm-ops-flow/tree/a455c59f7cc7d00a6e534de570de3dbe24a53e4b#needed-environment-variables

moolitayer · 2017-12-26T09:39:25Z

spec/vcr_cassettes/manageiq/providers/openshift/container_manager/test_objects_record.sh

+#!/bin/bash
+
+set -e  # abort on error
+


Since this isn't using functions can we have section headers for readability? e.g

# # Ensure API Server #

Added an echo, I'm using these as both headings in code and headings in script output.

moolitayer · 2017-12-26T09:40:42Z

spec/vcr_cassettes/manageiq/providers/openshift/container_manager/test_objects_record.sh

+    echo 'or have minishift in $PATH and already running, e.g.'
+    echo '    minishift addons enable manageiq'
+    echo '    minishift start --vm-driver virtualbox --openshift-version v3.7.0'
+    exit 1


you could add a link to this page explaining deployment options:
http://manageiq.org/docs/guides/providers/openshift

moolitayer · 2017-12-26T10:32:28Z

spec/vcr_cassettes/manageiq/providers/openshift/container_manager/test_objects_record.sh

+  if which minishift && minishift status | grep -i 'openshift:.*running'; then
+    export API_HOST="$(minishift ip)"
+    eval $(minishift oc-env --shell bash) # Ensure oc in PATH
+    oc login -u system:admin # With minishift, we know we can just do this


where do we login to a non minishift cluster?
or do we assume we are already logged in?
I'd say running on the openshift master is a good assumption (API_HOST=localhost)

for non-minishift BYO, script expects you to already have logged in.

Running on openshift master is inconvenient — you need manageiq checked out & working (at least test DB) to run this script, and script leaves updated files you'll want to git commit.
I should document how to oc adm policy add-cluster-role-to-user cluster-admin you for remote login.

I've never actually tested the script in BYO mode. Let me try...

moolitayer · 2017-12-26T11:00:39Z

spec/vcr_cassettes/manageiq/providers/openshift/container_manager/test_objects_record.sh

+
+VCR_DIR="$(dirname "$(realpath "$0")")"
+
+cd "$(realpath "$0" | sed 's:\(manageiq-providers-openshift/\).*$:\1:')" # repo root


cd "$(git rev-parse --show-toplevel)"

moolitayer · 2017-12-26T11:11:20Z

spec/vcr_cassettes/manageiq/providers/openshift/container_manager/test_objects_record.sh

+  oc start-build my-build-config-$ind
+done
+
+while out="$(oc get build --all-namespaces)"; echo "$out"; [ "$(echo "$out" | egrep --count --word 'Complete|Failed|Error')" -ne 3 ]; do


Please upcase all the variables

moolitayer · 2017-12-26T11:13:16Z

Looks great and taking us a long way.
This is meant to be run from a developer machine right(not on an automation server)?

cben · 2017-12-26T11:56:23Z

This is meant to be run from a developer machine right(not on an automation server)?

Right, however it's very close to what we'd need for an actually frequently run integration test, if we'll want that.

miq-bot · 2017-12-26T12:29:32Z

Checked commits cben/manageiq-providers-openshift@d80e778~...466a85c with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
6 files checked, 1 offense detected

**

💣 💥 🔥 🚒 - Linter/Yaml - missing config files

cben · 2017-12-26T12:39:38Z

Oops, I was pushing to different branch, you didn't see changes I was claiming, sorry :-(
Pushed now.
AFAICT, last PR state you saw was cben/manageiq-providers-openshift@bf456cf...ab7707b — or rebased to current base 89a8936...cben:vcr-script-old-review-rebased
Can't give you a github link to see only new parts, but if you want you can locally ~~(includes some rebase noise)~~:

git remote add cben https://github.com/cben/manageiq-providers-openshift
git fetch cben
git diff vcr-script-old-review-rebased..cben/vcr-script

I believe all comments addressed (see recent commits).

@miq-bot add-label gaprindashvili/yes
will need new vcr for improving quotas test

ManageIQ/manageiq-providers-openshift#75

moolitayer · 2017-12-27T08:44:52Z

spec/vcr_cassettes/manageiq/providers/openshift/container_manager/test_objects_record.sh

+  sleep 3
+done
+
+for ind in 0 1 2; do


Upcase var please

moolitayer · 2017-12-27T08:54:19Z

spec/vcr_cassettes/manageiq/providers/openshift/container_manager/test_objects_record.sh

+
+rm -v "$VCR_DIR"/refresher_after_deletions.{yml,txt} || true
+describe_vcr > "$VCR_DIR"/refresher_after_deletions.txt
+env RECORD_VCR=after_deletions bundle exec rspec "$SPEC" || echo "^^ FAILURES ARE POSSIBLE, YOU'LL HAVE TO EDIT THE SPEC"


Are FAILURES expected due to the status issue above? something else?

Due to recordings not being 100% reproducible — it's quite stable now but still usually at least something is off by one. "An ideal world is left as an exercise to the reader."

Additionally, if you switch Openshift versions, or change template deliberately, you'll get certain differences.

In any case, this script should not abort on test fail — it should let you get both recordings, and then you can edit & run spec again and again...

Better wording welcome.

cben · 2017-12-31T15:17:04Z

ping @moolitayer anything else needed here?

moolitayer

LGTM 👍

moolitayer · 2017-12-31T17:59:54Z

Awesome 💪

Automated VCR re-recording (cherry picked from commit 6e8842e)

simaishi · 2018-01-03T17:19:35Z

Gaprindashvili backport details:

$ git log -1
commit 3731370ee732f07861b6d4598ff0befcbace3dbf
Author: Mooli Tayer <[email protected]>
Date:   Sun Dec 31 19:56:57 2017 +0200

    Merge pull request #75 from cben/vcr-script
    
    Automated VCR re-recording
    (cherry picked from commit 6e8842e4592433ea2ce7ade2fb2d8806da9b8053)

miq-bot added the wip label Dec 10, 2017

yaacov approved these changes Dec 10, 2017

View reviewed changes

miq-bot added the unmergeable label Dec 10, 2017

cben force-pushed the vcr-script branch from 8646c0c to 324753c Compare December 10, 2017 12:06

miq-bot removed the unmergeable label Dec 10, 2017

cben force-pushed the vcr-script branch from 324753c to 517c4de Compare December 10, 2017 12:18

enoodle approved these changes Dec 10, 2017

View reviewed changes

This was referenced Dec 19, 2017

Parse and save quota scopes ManageIQ/manageiq-providers-kubernetes#190

Merged

Add ContainerQuotaScope model, save them in save_inventory ManageIQ/manageiq#16655

Merged

cben added 10 commits December 25, 2017 13:01

VCR recording comment: mention need to switch projects

d80e778

Script for automated VCR re-record

360bdb9

[refresher spec] Refactor record counts assertions

869292d

- compare together to get all differences printed together - express counts after deletions as subtractions

Wait for builds to complete (with current template, always Failed)

dc6e8ad

Generate .txt files with some info and objects overview

deb0623

Wait for deleted project to disappear before recording 2nd VCR

aeacebe

Prevents ContainerProject.count "expected: 10 got: 11" failure.

more data in .txt summary files

282e14c

[VCR template] add quotas with 1 and 2 scopes

3eeac63

cben changed the title ~~[WIP] Automated VCR re-recording~~ Automated VCR re-recording Dec 25, 2017

miq-bot removed the wip label Dec 25, 2017

Rename {before,after}_openshift_deletions -> {before,after}_deletions

9e9c503

moolitayer reviewed Dec 26, 2017

View reviewed changes

cben mentioned this pull request Dec 26, 2017

refresher_spec: Refactor counts assertions, check volumes after deletions ManageIQ/manageiq-providers-kubernetes#193

Merged

moolitayer reviewed Dec 26, 2017

View reviewed changes

cben added 4 commits December 26, 2017 13:35

Rename env vars to match https://github.com/container-mgmt/cm-ops-flow

0440220

add header on top

717fb6d

Simpler command to find repo root

44734d3

upcase shell variable

6cd5391

Reference guide for how to run minishift (will add details there)

466a85c

cben force-pushed the vcr-script branch from ab7707b to 466a85c Compare December 26, 2017 12:23

miq-bot added the gaprindashvili/yes label Dec 26, 2017

cben added a commit to cben/guides that referenced this pull request Dec 26, 2017

Document VCR-recording script

1f06a6d

ManageIQ/manageiq-providers-openshift#75

cben mentioned this pull request Dec 26, 2017

Openshift VCR-recording script, touch up minishift instructions. ManageIQ/guides#278

Merged

moolitayer reviewed Dec 27, 2017

View reviewed changes

upcase var

8ccc772

moolitayer approved these changes Dec 31, 2017

View reviewed changes

moolitayer added the test label Dec 31, 2017

moolitayer merged commit 6e8842e into ManageIQ:master Dec 31, 2017

moolitayer added the inventory label Dec 31, 2017

moolitayer added this to the Sprint 76 Ending Jan 1, 2018 milestone Dec 31, 2017

simaishi pushed a commit that referenced this pull request Jan 3, 2018

Merge pull request #75 from cben/vcr-script

3731370

Automated VCR re-recording (cherry picked from commit 6e8842e)

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Jan 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automated VCR re-recording #75

Automated VCR re-recording #75

cben commented Dec 10, 2017 •

edited

Loading

yaacov left a comment

miq-bot commented Dec 10, 2017

enoodle left a comment

Ladas commented Dec 11, 2017 •

edited

Loading

cben commented Dec 25, 2017 •

edited

Loading

moolitayer Dec 26, 2017

moolitayer Dec 26, 2017

moolitayer Dec 26, 2017

moolitayer Dec 26, 2017

cben Dec 26, 2017

moolitayer Dec 26, 2017

moolitayer Dec 26, 2017

cben Dec 26, 2017

moolitayer Dec 26, 2017

moolitayer Dec 26, 2017

moolitayer commented Dec 26, 2017

cben commented Dec 26, 2017

miq-bot commented Dec 26, 2017

cben commented Dec 26, 2017 •

edited

Loading

moolitayer Dec 27, 2017

cben Dec 31, 2017

moolitayer Dec 27, 2017

cben Dec 27, 2017

cben commented Dec 31, 2017

moolitayer left a comment

moolitayer commented Dec 31, 2017

simaishi commented Jan 3, 2018


		VCR_DIR="$(dirname "$(realpath "$0")")"

		cd "$(realpath "$0" \| sed 's:\(manageiq-providers-openshift/\).*$:\1:')" # repo root

		#!/bin/bash

		set -e # abort on error

Automated VCR re-recording #75

Automated VCR re-recording #75

Conversation

cben commented Dec 10, 2017 • edited Loading

Steps I took for new VCR recording:

Issues I didn't fix

yaacov left a comment

Choose a reason for hiding this comment

miq-bot commented Dec 10, 2017

enoodle left a comment

Choose a reason for hiding this comment

Ladas commented Dec 11, 2017 • edited Loading

cben commented Dec 25, 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

moolitayer commented Dec 26, 2017

cben commented Dec 26, 2017

miq-bot commented Dec 26, 2017

cben commented Dec 26, 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

cben commented Dec 31, 2017

moolitayer left a comment

Choose a reason for hiding this comment

moolitayer commented Dec 31, 2017

simaishi commented Jan 3, 2018

cben commented Dec 10, 2017 •

edited

Loading

Ladas commented Dec 11, 2017 •

edited

Loading

cben commented Dec 25, 2017 •

edited

Loading

cben commented Dec 26, 2017 •

edited

Loading