Add script to help setup and execute ansible_runner execution tests #23574
Conversation
|
Example run on a container: |
There was a problem hiding this comment.
I haven't seen or used the bats tests before so trying to catch up...
This feels a little like one of those "magic" curl https://example.com/install.sh | bash scripts that does everything in one go. A lot of this is stuff a developer would only need to do once (e.g. in developer_setup.md + bin/setup) and after that it is just rake test:ansible_runner_execution
| # Ensure bats and its support repos are installed | ||
| if [ ! -x "$(command -v bats)" ]; then | ||
| if [ "$APPLIANCE" == "true" ]; then | ||
| echo -e "\033[1;34mInstalling bats...\033[0m" | ||
| dnf install -y bats | ||
| else | ||
| echo -e "\033[1;31mERROR: Unable to find the bats test framework.\033[0m" | ||
| exit 1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Should this be in developer_setup.md? I know that wouldn't work help with a production appliance but we don't have anything else I don't think which auto-configures prod appliances as pseudo dev systems so some manual work isn't uncalled for there
There was a problem hiding this comment.
This is basically written to be "ensure bats is installed, and if not and we're on an appliance/container, then install it". I'm good with adding the bats installation instructions to developer-setup.md for local testing, but I'd still have to keep this for appliances anyway.
Note that the check keeps it idempotent, so it only installs once.
bin/test_ansible_runner_execution
Outdated
| if [ ! -d "$HOME/.bats/libs/bats-support" ]; then | ||
| echo -e "\033[1;34mInstalling bats-support...\033[0m" | ||
| git clone https://github.com/bats-core/bats-support $HOME/.bats/libs/bats-support | ||
| fi | ||
| if [ ! -d "$HOME/.bats/libs/bats-assert" ]; then | ||
| echo -e "\033[1;34mInstalling bats-assert...\033[0m" | ||
| git clone https://github.com/bats-core/bats-assert $HOME/.bats/libs/bats-assert | ||
| fi |
There was a problem hiding this comment.
Is this something we should do in bin/setup? And does this have to be in $HOME? Don't love it when tools clutter my top-level $HOME dotfiles when they could be in the current working directory
There was a problem hiding this comment.
There is a BATS_LIB_PATH, but it's typically not exported, even with a normal bats installation. I could check for BATS_LIB_PATH presence first then fallback to $HOME/.bats/libs. See https://bats-core.readthedocs.io/en/stable/writing-tests.html#bats-load-library-load-system-wide-libraries. I'd be open to changing where this lives, I would just have to also change the tests.
There was a problem hiding this comment.
ok switched this to use BATS_LIB_PATH, however, I think I agree that maybe we only clone on appliance/container, and then assume the person set it up correctly locally (such as via developer-setup.md). Would that work? That's kind of what I did with bats installation itself.
| if [ ! -d "./spec/lib/ansible" ]; then | ||
| echo -e "\033[1;34mCloning ansible specs...\033[0m" | ||
| rm -rf /tmp/manageiq | ||
| git clone --no-checkout --depth=1 --filter=tree:0 https://github.com/ManageIQ/manageiq /tmp/manageiq | ||
| pushd /tmp/manageiq | ||
| git sparse-checkout set --no-cone /spec/lib/ansible | ||
| git checkout | ||
| popd | ||
|
|
||
| mkdir -p ./spec/lib | ||
| cp -r /tmp/manageiq/spec/lib/ansible ./spec/lib | ||
| rm -rf /tmp/manageiq | ||
| fi |
There was a problem hiding this comment.
Seems weird that we're adding a command into the core repo but have code to handle when self/spec doesn't exist
There was a problem hiding this comment.
See below. On production builds, spec dir doesn't exist. However, locally it will. This also keeps this piece idempotent, so that it only clones once (and/or can reclone if I rm -rf the directory and re-run the script)
There was a problem hiding this comment.
Yeah longer term maybe the runner_execution_spec.bats tests don't belong in spec/lib/ansible/runner since they're not necessarily testing Ansible::Runner so that way you wouldn't need to clone spec on a production appliance for integration testing.
There was a problem hiding this comment.
The main problems are that a) I want to share the actual playbooks in spec/lib/ansible/data and b) the tests in the .bats file should be almost entirely duplicated of runner_execution_spec.rb.
Note that we do test Ansible::Runner via bats. The tests first test ansible-runner execution directly to ensure the CLI, venv, and collections are setup correctly (without Ruby), then they test the same set of tests via Ansible::Runner to ensure that the Ruby code can see the CLI and run it correctly. Then the tests verify that specific collections from the venv are valid (this helps if we pull in newer collections to ensure that things like vmware and aws can still work correctly).
|
@agrare Apologies I thought you were familiar with the background on this. So the background is that on a production appliance or container, rspec doesn't exist nor does the spec directory. In order to verify ansible-runner on one of our builds (or more accurately, verify that the manageiq-ansible-venv rpm is working correctly), I put together this bats based set of tests in #23482. The reasoning for using BATS vs rspec was a) it was really difficult to install rspec in a production appliance/container due to the way our gemset rpm is designed and b) I wanted to run a bunch of lower level tests against ansible-runner cli that didn't need Rails. However, I did want to reuse the existing set of playbooks from the spec directory as much as possible. So, the bats tests use the bats framework, however, in a production appliance I first need to install bats, the test dependencies bats-support and bats-assert, and get a copy of the specs themselves. When I was building #23482 initially, I didn't script it, and basically just mapped a volume to the container and/or scp'd the specs into the appliance, since I was actively building them. I would also run the bats tests locally to make sure they worked before transferring to those other locations to test. However, now, a few months later, I forgot how onerous the setup actually is, hence this script. My intention is that this script is part of core, because that's where the specs live, but shipped with the appliance, so a developer can just ssh into an appliance or spin up the main container, and then just run the script. This is why it's not in developer-setup.md, because it's not only meant to be run locally. However, I did design the script to also run locally as well so developers can verify their local setup, which also helps with writing new ansible-runner tests as I mentioned above. I agree this is very different than other things, but it's also the only "test-in-production" thing we have. I could move it to another directory if you think bin is not appropriate, but it can't be the spec dir, because we don't ship the spec dir. |
Fryguy
force-pushed
the
test_ansible_runner_execution
branch
from
88621ff to
8dbe2c5
Compare
|
Yeah I like the idea of resurrecting integration_tests in a new form and/or test things on remote appliances/containers. This would definitely belong there. |
|
Backported to |
Add script to help setup and execute ansible_runner execution tests (cherry picked from commit bf61e24)
@agrare Please review.
With this in place, it's much simpler to test ansible-runner low-level and Ansible::Runner on a production appliance or container, as the setup is the complicated bit that this script now takes care of. This script can also be run locally.