improve installer tests compatibility with pit #15308
Conversation
|
trigger: test-robottelo |
rmynar
added
6.14.z
|
PRT Result |
Satellite-QE
added
the
PRT-Failed
|
trigger: test-robottelo |
|
PRT Result |
Satellite-QE
added
PRT-Passed
Satellite-QE
removed
the
PRT-Passed
rmynar
force-pushed
the
pit-installer
branch
2 times, most recently
from
49844fa to
2233a40
Compare
| for task in sync_tasks: | ||
| logger.info(f'Waiting for task {task['id']}') | ||
| sat_non_default_install.wait_for_tasks( | ||
| search_query=(f'id = {task["id"]}'), | ||
| poll_timeout=1800, | ||
| ) |
There was a problem hiding this comment.
API wise I'd expect that wait_for_tasks doesn't need to be used in a loop. Would it be better to have a more complex search query that waits for all tasks? This probably works:
| for task in sync_tasks: | |
| logger.info(f'Waiting for task {task['id']}') | |
| sat_non_default_install.wait_for_tasks( | |
| search_query=(f'id = {task["id"]}'), | |
| poll_timeout=1800, | |
| ) | |
| sat_non_default_install.wait_for_tasks( | |
| search_query=(f'id ^ {",".join(task["id"] for task in sync_tasks}'), | |
| poll_timeout=1800, | |
| ) |
But performance wise it may actually be worse. Perhaps you'd also need to pass in a status filter, but IMHO wait_for_tasks(tasks=sync_tasks) should just do that without needing to think about it.
There was a problem hiding this comment.
You're right, the function has its own loop so it can accept multiple tasks in search query.
| 2. Configure capsule repos | ||
| 3. Install and enable fapolicyd | ||
| 4. Enable capsule module | ||
| 1. Use Satellite with fapolicyd enabled (non-default) |
There was a problem hiding this comment.
I see this uses sat_non_default_install but I really disagree that we should be testing with --foreman-rails-cache-store type:file. If we want proper integration testing we should be testing a basic default Satellite instead of the edge case.
If anything, the test that actually wants to test Satellite with non-Redis should explicitly configure Satellite that way.
There was a problem hiding this comment.
I decided to create another fixture with default setup but including fapolicyd. Let's consider this as a workaround I hope it can be done better (e.g. parametrized), but it's out of scope of this PR.
| ) | ||
|
|
||
| # setup source repositories | ||
| if settings.server.version.source == "ga": |
There was a problem hiding this comment.
"ga" ?!??
Wouldn't be "cdn" better ?
There was a problem hiding this comment.
This is a remainder of historical state when there were two versions available on cdn - ga and beta. Beta is no more used. However it still seems logical to me - we have internal source or generally available source. In case of renaming I would suggest different naminginternal/public.
rmynar
force-pushed
the
pit-installer
branch
3 times, most recently
from
f28d411 to
e435c17
Compare
|
trigger: test-robottelo |
|
PRT Result |
Satellite-QE
added
the
PRT-Failed
|
trigger: test-robottelo |
|
PRT Result |
Satellite-QE
added
PRT-Passed
|
@rmynar I converted pr to draft for now, please mark it ready for review once you're done with changes. |
|
trigger: test-robottelo |
|
PRT Result |
(cherry picked from commit 3d020bc)
This change improves compatibility with PIT testing.
We can choose source repositories for RHEL and Satellite. Following combinations of sat@rhel are possible:
internal@internal - pre-release testing
ga@internal - PIT testing
internal@ga - common testing
ga@ga - invalid combination (we don't care about retesting already published products)
The same applies for Capsule. It's also possible to use different version of Capsule and Satellite (e.g. n-1 testing)
See SAT-26896