-
Notifications
You must be signed in to change notification settings - Fork 304
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
tests: Merge installed/ and fedora-str/ directories #1513
Conversation
Note this is going to work better with https://pagure.io/standard-test-roles/pull-request/152# |
73be80b
to
c24396d
Compare
Lifting WIP. |
Looks like a timeout on async test:
|
I pulled in https://pagure.io/standard-test-roles/pull-request/152# which should help hopefully. |
bot, retest this please |
remote_user: root | ||
vars: | ||
use_git_build: True | ||
tests: "." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this can be used to select which tests to run, right? Can you document that in the README.md
?
tests/installed/run.sh
Outdated
fi | ||
|
||
# TODO: parallelize this | ||
PLAYBOOKS=${PLAYBOOKS:-destructive.yml nondestructive.yml} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, would it be better practice to run the non-destructive tests first?
tests/installed/README.md
Outdated
The high level structure is that we take a qcow2 file, inject | ||
built RPMs into it, and then use Ansible to run tests. | ||
|
||
See `papr.yml` for canonical usage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: s/papr.yml/.papr.yml/
bot, retest this please |
Hmm so locally on my desktop this task is 8 seconds:
Yet in PAPR it's four minutes:
Offhand I'm guessing this is something like us overloading container tests on a single host? |
That sounds plausible, yes. Each run spawns 8 containers, all installing deps and building ostree in parallel. The cool thing with running in OCP is that we can naturally split those out across the whole cluster. Maybe let's try marking it as destructive so it runs inline afterwards and see if it makes a difference? |
Let's be opinionated now, and our installed/ test story *is* Ansible/STR. Merge `tests/fedora-str` into `tests/installed/`. Rework the nondestructive tests into a separate playbook run, and parallelize them for more efficiency. The destructive tests are also changed to use Ansible more. Add a higher level `run.sh` entrypoint and update the `README.md` with some useful tips.
bot, retest this please |
1 similar comment
bot, retest this please |
Yeah, but this across-the-board speed hit is problematic in other ways too. And the overloading is also the source of other CI flakes like disk space. So I'd rather to try to sprint to the new Kube architecture; it's not like we're lacking resources (in general). |
bot, retest this please |
That looks better, though I see:
Agreed! I'm hoping to get there really soon. That said, just for the purposes of unblocking this right now, WDYT of just provisioning a beefy |
☀️ Test successful - status-atomicjenkins |
Somehow, this slipped through in ostreedev#1513. We weren't inheriting anymore, so `branches` defaulted back to just `master`. This means we weren't gating on most of the containerized builds anymore. Ouch! It'd make sense to teach PAPR to allow some defaults across all testsuites, even in the `inherit: false` case. Though it's tempting to also just change the hardcoded PAPR default to those branches since our use of Homu + PAPR at this point is pretty ubiquitous, and it doesn't really hurt for the ones that don't use it.
Somehow, this slipped through in #1513. We weren't inheriting anymore, so `branches` defaulted back to just `master`. This means we weren't gating on most of the containerized builds anymore. Ouch! It'd make sense to teach PAPR to allow some defaults across all testsuites, even in the `inherit: false` case. Though it's tempting to also just change the hardcoded PAPR default to those branches since our use of Homu + PAPR at this point is pretty ubiquitous, and it doesn't really hurt for the ones that don't use it. Closes: #1536 Approved by: cgwalters
Somehow, this slipped through in #1513. We weren't inheriting anymore, so `branches` defaulted back to just `master`. This means we weren't gating on most of the containerized builds anymore. Ouch! It'd make sense to teach PAPR to allow some defaults across all testsuites, even in the `inherit: false` case. Though it's tempting to also just change the hardcoded PAPR default to those branches since our use of Homu + PAPR at this point is pretty ubiquitous, and it doesn't really hurt for the ones that don't use it. Closes: #1536 Approved by: cgwalters
Somehow, this slipped through in #1513. We weren't inheriting anymore, so `branches` defaulted back to just `master`. This means we weren't gating on most of the containerized builds anymore. Ouch! It'd make sense to teach PAPR to allow some defaults across all testsuites, even in the `inherit: false` case. Though it's tempting to also just change the hardcoded PAPR default to those branches since our use of Homu + PAPR at this point is pretty ubiquitous, and it doesn't really hurt for the ones that don't use it. Closes: #1536 Approved by: cgwalters
Somehow, this slipped through in #1513. We weren't inheriting anymore, so `branches` defaulted back to just `master`. This means we weren't gating on most of the containerized builds anymore. Ouch! It'd make sense to teach PAPR to allow some defaults across all testsuites, even in the `inherit: false` case. Though it's tempting to also just change the hardcoded PAPR default to those branches since our use of Homu + PAPR at this point is pretty ubiquitous, and it doesn't really hurt for the ones that don't use it. Closes: #1536 Approved by: cgwalters
Let's be opinionated now, and our installed/ test story is
Ansible/STR. Merge
tests/fedora-str
intotests/installed/
.Rework the nondestructive tests into a separate playbook run, and parallelize
them for more efficiency.
The destructive tests are also changed to use Ansible more.
Add a higher level
run.sh
entrypoint and update theREADME.md
with some useful tips.