Proposal: Separate cloud-image-val utility from test suites - #570
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 7 issues, and left some high level feedback:
- Consider explicitly calling out a deprecation/migration timeline in the ADR (e.g., when the old test_suite integration is considered unsupported) so downstream teams can plan concrete milestones against this proposal.
- The ADR currently hardcodes
/tmp/civ-*paths in multiple sections; it might be worth introducing a single configuration/contract section that defines these paths (and whether they are configurable) to avoid divergence between documentation and implementation later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider explicitly calling out a deprecation/migration timeline in the ADR (e.g., when the old test_suite integration is considered unsupported) so downstream teams can plan concrete milestones against this proposal.
- The ADR currently hardcodes `/tmp/civ-*` paths in multiple sections; it might be worth introducing a single configuration/contract section that defines these paths (and whether they are configurable) to avoid divergence between documentation and implementation later.
## Individual Comments
### Comment 1
<location path="docs/ADR-001-separation-of-concerns.md" line_range="12" />
<code_context>
+
+This ADR proposes refactoring cloud-image-val (CIV) from a monolithic Python test framework into a minimal CLI tool that provisions cloud instances, executes arbitrary test commands,
+and aggregates results into standardized reports. This approach enables downstream teams to maintain tests in their own repositories using any testing framework (pytest, shell scripts, Go tests, etc.)
+while eliminating tight coupling and reducing CIVs maintenance burden.
+
+### Key Decisions
</code_context>
<issue_to_address>
**issue (typo):** Use the possessive form "CIV's" instead of "CIVs".
Please update the phrase to "reducing CIV's maintenance burden."
```suggestion
while eliminating tight coupling and reducing CIV's maintenance burden.
```
</issue_to_address>
### Comment 2
<location path="docs/ADR-001-separation-of-concerns.md" line_range="87" />
<code_context>
+
+Refactor CIV into a minimal infrastructure-only CLI tool that provisions cloud instances, executes arbitrary test commands, and aggregates results into standardized reports.
+
+**CIVs responsibilities:**
+- Manage infrastructure lifecycle (provision → expose metadata → cleanup)
+- Execute arbitrary test commands
</code_context>
<issue_to_address>
**issue (typo):** Change "CIVs" to the possessive "CIV's".
Please update this heading to "**CIV's responsibilities:**" to use the correct possessive form.
```suggestion
**CIV's responsibilities:**
```
</issue_to_address>
### Comment 3
<location path="docs/ADR-001-separation-of-concerns.md" line_range="93" />
<code_context>
+- Aggregate and validate test results into standardized format (JUnit XML)
+- Provide consistent exit codes and reporting for CI/CD pipelines
+
+**Test teams responsibilities:**
+- Write tests using any framework/language
+- Consume standardized instance metadata
</code_context>
<issue_to_address>
**suggestion (typo):** Use a possessive form for "Test teams".
Consider "Test teams' responsibilities:" or "Test team responsibilities:" depending on whether you mean multiple teams.
```suggestion
**Test teams' responsibilities:**
```
</issue_to_address>
### Comment 4
<location path="docs/ADR-001-separation-of-concerns.md" line_range="67-68" />
<code_context>
+
+2. Improve Maintainability
+- GitLab pipelines too complex for package testing needs
+- Too much time spend fixing CIV bugs vs too little time spend developing CIV
+- PR pipeline state mostly ignored by devs
+
</code_context>
<issue_to_address>
**issue (typo):** Correct "spend" to "spent" in this sentence.
Consider rewriting this bullet as: "Too much time spent fixing CIV bugs vs too little time spent developing CIV."
```suggestion
2. Improve Maintainability
- Too much time spent fixing CIV bugs vs too little time spent developing CIV
```
</issue_to_address>
### Comment 5
<location path="docs/ADR-001-separation-of-concerns.md" line_range="414" />
<code_context>
+
+### CIV Helper Libraries
+
+To make result reporting easy for all test frameworks, CIV needs to provides helper libraries for languages that do not provide native JUnit output or auto conversion.
+Alternative: CIV includes converters for common test output formats, e.g. Golang.
+
</code_context>
<issue_to_address>
**issue (typo):** Fix the verb form in "needs to provides helper libraries".
Consider rephrasing to: "CIV needs to provide helper libraries for languages that do not provide native JUnit output or auto conversion."
```suggestion
To make result reporting easy for all test frameworks, CIV needs to provide helper libraries for languages that do not provide native JUnit output or auto conversion.
```
</issue_to_address>
### Comment 6
<location path="docs/ADR-001-separation-of-concerns.md" line_range="559" />
<code_context>
+
+1. **CIV Core Simplification**: Core codebase reduced. Structure needs to guide developers.
+2. **Language Flexibility**: At least one spike using non-Python tests (shell or Go)
+3. **Downstream Adoption**: 2+ teams using new approach in production (osbuild, packages maintainers)
+4. **Performance**: Infrastructure provision time unchanged or faster (remove pytest overhead)
+5. **Security**: No bandit high-severity findings
</code_context>
<issue_to_address>
**suggestion (typo):** Clarify "packages maintainers" phrasing.
"Packages maintainers" reads a bit awkwardly. Consider "package maintainers" or "packages team maintainers" instead.
```suggestion
3. **Downstream Adoption**: 2+ teams using new approach in production (osbuild, package maintainers)
```
</issue_to_address>
### Comment 7
<location path="docs/ADR-001-separation-of-concerns.md" line_range="613" />
<code_context>
+- `lib/aws_lib.py` (test-specific)
+- Duplicate `ci/aws.sh` (after osbuild migration)
+
+### New Test Suire Repositories
+- `tests/generic/test_generic.py` (from test_suite/generic/)
+- `tests/cloud/test_aws.py` (from test_suite/cloud/)
</code_context>
<issue_to_address>
**issue (typo):** Typo in "Suire"; should be "Suite".
Please update the heading to `### New Test Suite Repositories`.
```suggestion
### New Test Suite Repositories
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| This ADR proposes refactoring cloud-image-val (CIV) from a monolithic Python test framework into a minimal CLI tool that provisions cloud instances, executes arbitrary test commands, | ||
| and aggregates results into standardized reports. This approach enables downstream teams to maintain tests in their own repositories using any testing framework (pytest, shell scripts, Go tests, etc.) | ||
| while eliminating tight coupling and reducing CIVs maintenance burden. |
There was a problem hiding this comment.
issue (typo): Use the possessive form "CIV's" instead of "CIVs".
Please update the phrase to "reducing CIV's maintenance burden."
| while eliminating tight coupling and reducing CIVs maintenance burden. | |
| while eliminating tight coupling and reducing CIV's maintenance burden. |
There was a problem hiding this comment.
Fair point!
Since this is an ADR and may get a lot of eyeballs, I’d keep the nit-picky fix — or, if you prefer to avoid apostrophes with acronyms, we could also rephrase slightly:
while eliminating tight coupling and reducing CIV maintenance burden.Either way keeps the sentence clear and grammatically clean; feel free to pick whichever matches the docs style guide better.
|
|
||
| Refactor CIV into a minimal infrastructure-only CLI tool that provisions cloud instances, executes arbitrary test commands, and aggregates results into standardized reports. | ||
|
|
||
| **CIVs responsibilities:** |
There was a problem hiding this comment.
issue (typo): Change "CIVs" to the possessive "CIV's".
Please update this heading to "CIV's responsibilities:" to use the correct possessive form.
| **CIVs responsibilities:** | |
| **CIV's responsibilities:** |
| - Aggregate and validate test results into standardized format (JUnit XML) | ||
| - Provide consistent exit codes and reporting for CI/CD pipelines | ||
|
|
||
| **Test teams responsibilities:** |
There was a problem hiding this comment.
suggestion (typo): Use a possessive form for "Test teams".
Consider "Test teams' responsibilities:" or "Test team responsibilities:" depending on whether you mean multiple teams.
| **Test teams responsibilities:** | |
| **Test teams' responsibilities:** |
| 2. Improve Maintainability | ||
| - GitLab pipelines too complex for package testing needs |
There was a problem hiding this comment.
issue (typo): Correct "spend" to "spent" in this sentence.
Consider rewriting this bullet as: "Too much time spent fixing CIV bugs vs too little time spent developing CIV."
| 2. Improve Maintainability | |
| - GitLab pipelines too complex for package testing needs | |
| 2. Improve Maintainability | |
| - Too much time spent fixing CIV bugs vs too little time spent developing CIV |
|
|
||
| ### CIV Helper Libraries | ||
|
|
||
| To make result reporting easy for all test frameworks, CIV needs to provides helper libraries for languages that do not provide native JUnit output or auto conversion. |
There was a problem hiding this comment.
issue (typo): Fix the verb form in "needs to provides helper libraries".
Consider rephrasing to: "CIV needs to provide helper libraries for languages that do not provide native JUnit output or auto conversion."
| To make result reporting easy for all test frameworks, CIV needs to provides helper libraries for languages that do not provide native JUnit output or auto conversion. | |
| To make result reporting easy for all test frameworks, CIV needs to provide helper libraries for languages that do not provide native JUnit output or auto conversion. |
|
|
||
| 1. **CIV Core Simplification**: Core codebase reduced. Structure needs to guide developers. | ||
| 2. **Language Flexibility**: At least one spike using non-Python tests (shell or Go) | ||
| 3. **Downstream Adoption**: 2+ teams using new approach in production (osbuild, packages maintainers) |
There was a problem hiding this comment.
suggestion (typo): Clarify "packages maintainers" phrasing.
"Packages maintainers" reads a bit awkwardly. Consider "package maintainers" or "packages team maintainers" instead.
| 3. **Downstream Adoption**: 2+ teams using new approach in production (osbuild, packages maintainers) | |
| 3. **Downstream Adoption**: 2+ teams using new approach in production (osbuild, package maintainers) |
| - `lib/aws_lib.py` (test-specific) | ||
| - Duplicate `ci/aws.sh` (after osbuild migration) | ||
|
|
||
| ### New Test Suire Repositories |
There was a problem hiding this comment.
issue (typo): Typo in "Suire"; should be "Suite".
Please update the heading to ### New Test Suite Repositories.
| ### New Test Suire Repositories | |
| ### New Test Suite Repositories |
81b852e to
c2e5385
Compare
| - Manual Terraform/OpenTofu version synchronization | ||
| - `.gitlab-ci-cloud-experience.yaml` hardcodes runner names requiring manual updates | ||
|
|
||
| **4. Prevents Independent Test Maintenance** |
There was a problem hiding this comment.
Nit: I think it's fair to call out that we have both a maintenance and an ownership problem. CIV is used across several orgs within RH and the tight coupling creates bottlenecks for moving quickly.
There was a problem hiding this comment.
Very true, the ownership model that is proposed a little further along in section 4 visualizes this.
Maybe we can add something like
| **4. Prevents Independent Test Maintenance** | |
| **4. Prevents Independent Test Maintenance** | |
| - Split repo ownership across teams/orgs blurs responsibilities, slowing down project decisions and steering |
|
|
||
| **4. Prevents Independent Test Maintenance** | ||
| - Image definitions live in osbuild-composer repository | ||
| - Verification tests live in cloud-image-val repository |
There was a problem hiding this comment.
Suggestion: these two read a little oddly with the statement that they are the chicken-and-egg, but the point remains that we're coupling dependency and consumer concerns.
There was a problem hiding this comment.
Right, the chicken and egg problem occurs once we try to change one of them without at the same changing the other (e.g. synced PRs). I can add a bit more information to this.
| - No dependency separation (`requirements.txt` mixes test frameworks with runtime dependencies) | ||
| - Security vulnerabilities (`os.system()` calls without sanitization) | ||
|
|
||
| ### Business Drivers |
There was a problem hiding this comment.
Suggestion: we should consider adding speed to market as a business driver. The mandate from leaders is pretty clear on the ramp up on CVEs and it makes sense that we'll introduce more image testing over time and the coupling is both a slowdown and a risk to stability.
There was a problem hiding this comment.
Good thinking. Maybe something like thios?
| ### Business Drivers | |
| ### Business Drivers | |
| ... | |
| 6. Speed to Market | |
| - Current system coupling creates a bottleneck, directly conflicting with our interest to rapidly address and resolve CVEs. | |
| - As we introduce more image testing over time, this tight coupling will increasingly slow down our release cycles and threaten overall system stability. |
| 2. Improve Maintainability | ||
| - GitLab pipelines too complex for package testing needs | ||
| - Too much time spend fixing CIV bugs vs too little time spend developing CIV | ||
| - PR pipeline state mostly ignored by devs |
There was a problem hiding this comment.
Non-blocking Question: is the pipeline ignored by test contributors or developers in general?
There was a problem hiding this comment.
I'd say anyone opening a PR.
Schutzbot hasn't been green in a very long time even though we require it to pass to keep changes between projects (osbuild / CIV) in sync.
| %% ============================================================ | ||
| %% LAYER 2: Filesystem Contract (Shared State) | ||
| %% ============================================================ | ||
| subgraph CONTRACT["Filesystem Contract Layer<br/><i>Shared state between cloud-image-val and downstream tests</i>"] |
There was a problem hiding this comment.
Nit Question: will CIV define and expose this contract to consumers or is this something already exposed in the FS?
There was a problem hiding this comment.
I'd like to expose it.
I don't want to rely on any magically created files that are supposed to be in very specific locations on the host running the tests.
This is one of the issues I'd like to get feedback on from the teams implementing tests.
| - `[D]` = Downstream test repositories (spawned by executor.py) | ||
| - `[E]` = Optional helper utilities (junit-reporter.*) | ||
|
|
||
| **Decoupled Design:** |
There was a problem hiding this comment.
Praise: I really like the approach. It allows for team optionality.
|
|
||
| ### CIV Helper Libraries | ||
|
|
||
| To make result reporting easy for all test frameworks, CIV needs to provides helper libraries for languages that do not provide native JUnit output or auto conversion. |
There was a problem hiding this comment.
Non-blocking question: is there a generic standard for conversion or would CIV need to develop these for each language separately?
There was a problem hiding this comment.
The only standard we have in this case is that it has to be output in JUnit.
Most frameworks have existing exporters, e.g.:
- pytest --junit-xml=results.xml (native no additional changes necessary)
- gotestsum --junitfile results.xml (wrapper)
- ./my-script.sh | tap-junit > path/to/results.xml (there are packages for the test anything protocol)
So in this sense, we didn't even need to develop anything. The helper should mainly cover edgecases.
| - JIRA API integration | ||
| - HTML report generation | ||
| - Test result modification | ||
| - **Fix:** split into plugins |
There was a problem hiding this comment.
Non-blocking Question: perhaps too implementation-specific for now, but would these plugins live in the CIV repo or would they get kicked out to a utility library/
There was a problem hiding this comment.
Those would live in the CIV repo. Maybe plugin is the wrong term here. We can consider these modules of the project.
|
|
||
| 2. **Inconsistent Naming Conventions** | ||
| - Python: `AzureConfigBuilderV2` vs `aws_config_builder` | ||
| - **Fix:** Use flake8 naming consistently in CI (future cleanup) |
There was a problem hiding this comment.
Non-blocking, implementation question: would we run these as pre-commits or would they just live on CI?
There was a problem hiding this comment.
Pre-commit would be my go-to for flake8.
We should still run them on PRs anyways.
| | Risk | Impact | Solution | | ||
| |------|--------|------------| | ||
| | Breaking existing CI pipelines | HIGH | upgrade in staging environment first | | ||
| | Invalid JUnit XML output | HIGH | Mandatory XSD validation before CIV returns. Clear error messages guide test authors | |
There was a problem hiding this comment.
Non-blocking, implementation question: what would the feedback loop look like for test authors? Could they run this on their local to quickly iterate or would pipelines be involved? (Sorry for the newbie question, I'm not familiar with the dev experience.)
There was a problem hiding this comment.
Yes running this on your local machine would be no problem.
In fact, you wouldn't need CIV for local testing at all as tests and CIV wouldn't require each other. For running tests on test instances (e.g. on real cloud instances) the flow would look like this:
- Write tests using your own framework.
- Ensure that your tests consume the civ-instances.json file to connect to provisioned instances (file path can be configurable or not, depending on what we decide on).
- Ensure that your tests produce JUnit XML output
- CIV takes care of provisioning, running your tests, cleanup, aggregating results and outputting the final JUnit results.
- Once you're confident, add a CI job, ideally using the CIV container image.
| 1. **CIV Core Simplification**: Core codebase reduced. Structure needs to guide developers. | ||
| 2. **Language Flexibility**: At least one spike using non-Python tests (shell or Go) | ||
| 3. **Downstream Adoption**: 2+ teams using new approach in production (osbuild, packages maintainers) | ||
| 4. **Performance**: Infrastructure provision time unchanged or faster (remove pytest overhead) |
There was a problem hiding this comment.
Non-blocking question: do we have baseline metrics for this currently or do we need to capture them?
There was a problem hiding this comment.
I believe @sshmulev did collect some.
We should have enough opportunity to run some performance tests though.
cmchase
left a comment
cmchase
left a comment
There was a problem hiding this comment.
I think this is great overall and I really appreciate the time and thought you put into the initial design, but there are a lot of technical details I think we'll need input on from teams that consume CIV.
| 3. **Downstream Adoption**: 2+ teams using new approach in production (osbuild, packages maintainers) | ||
| 4. **Performance**: Infrastructure provision time unchanged or faster (remove pytest overhead) | ||
| 5. **Security**: No bandit high-severity findings | ||
| 6. **Type Safety**: Aiming for 80% type hint coverage in `core/` and `ssh/` |
There was a problem hiding this comment.
Non-blocking Question: are we looking to warn and not error on initial linting checks?
There was a problem hiding this comment.
I would be okay with a warning level. Error seems a bit excessive.
|
|
||
| ## 9. Open Questions for Review | ||
|
|
||
| 1. Use `/tmp/civ-*.json` or configurable path? |
There was a problem hiding this comment.
Observation: FWIW, Sourcery called this out in a previous comment.
| - lib/console_lib.py (merged into cli.py) | ||
|
|
||
|
|
||
| ### Optional Helper Package (Separate Repository) |
There was a problem hiding this comment.
My proposal for this idea is:
Each test domain folder (tests/network/, tests/storage/, etc.) contains its own fixtures.py and helpers.py for domain-specific stuff, and there's a top-level helpers/common/ for things shared across all domains.
Rather than organizing tests by cloud provider (test_aws.py, test_azure.py) or the current generic/ vs cloud/ split, I'd suggest organizing by test domain:
tests/
**common/ # shared helpers/fixtures used across domains
helpers.py
fixtures.py**
network/
test_network.py
helpers.py
fixtures.py
storage/
test_storage.py
helpers.py
fixtures.py
packages/
test_packages.py
helpers.py
fixtures.py
certificates/
test_packages.py
helpers.py
fixtures.py
security/
test_packages.py
helpers.py
fixtures.py
boot/
test_packages.py
helpers.py
fixtures.py
...
Each test uses cloud markers (@pytest.mark.run_on(['aws', 'azure'])) to declare where it runs, we already have this pattern. Domain-specific fixtures and helpers live alongside the tests that need them, and truly shared utilities
(like AVC checks) go in common/.
There was a problem hiding this comment.
There might be a misunderstanding of the intended purpose of this helper package.
It's not supposed to provide helpers for writing tests, its only purpose would be to provide some sort of export functionality to make it easier for developer teams to write tests that can keep the contract between CIV and their test input / output. E.g. something that makes it super easy for none python languages to export results in JUnit.
| To make result reporting easy for all test frameworks, CIV needs to provides helper libraries for languages that do not provide native JUnit output or auto conversion. | ||
| Alternative: CIV includes converters for common test output formats, e.g. Golang. | ||
|
|
||
| ### Simplified CIV Repository Structure |
There was a problem hiding this comment.
Rather than organizing tests by cloud provider (test_aws.py, test_azure.py) or the current generic/ vs cloud/ split, I'd suggest organizing by test domain:
tests/
common/ # shared helpers/fixtures used across domains
helpers.py
fixtures.py
network/
test_network.py
helpers.py
fixtures.py
storage/
test_storage.py
helpers.py
fixtures.py
packages/
test_packages.py
helpers.py
fixtures.py
certificates/
test_certificates.py
helpers.py
fixtures.py
security/
test_security.py
helpers.py
fixtures.py
boot/
test_boot.py
helpers.py
fixtures.py
...
Each test uses cloud markers (@pytest.mark.run_on(['aws', 'azure'])) to declare where it runs. Domain-specific fixtures and helpers live alongside the tests that need them, and truly shared utilities (like AVC checks) go in common/.
On top of this, I think we should add tier markers to classify test priority:
Tier 1 - Core functionality, must always pass. A failure here is an immediate blocker (e.g., boot, SSH access, networking basics, correct image identity).
Tier 2 - Important regression checks, but a failure doesn't block the release immediately. Needs investigation on a short timeline
Tier 3 - Nice-to-have validations, edge cases, non-critical regressions.
Usage would look like:
@pytest.mark.tier1
@pytest.mark.run_on(['all'])
def test_sshd(self, host):
...
@pytest.mark.tier2
@pytest.mark.run_on(['aws', 'azure'])
def test_yum_group_install(self, host):
...
Benefits:
Combined with domain folders, this gives you two axes of triage: "5 network failures, 3 are tier-1" is immediately actionable without digging through logs. It also lets CI pipelines gate on tier-1 only for fast feedback, while running tier-2/3 in parallel or post-merge.
Ownership by expertise: domain-specific fixtures and helpers live next to the tests that use them, and shared ones go in helpers/common/.
Current test_generic.py is already implicitly domain-grouped , there are clusters of network tests (test_eth0_network_adapter_setup, test_network_manager_cloud_setup, test_network_manager_conf_plugins), package tests
(test_required_packages_are_installed, test_yum_plugins, test_yum_package_install), security tests (test_selinux_mode, test_no_avc_denials, test_auditd), etc. This would formalize that grouping.
Cloud-specific tests (like AWS identity checks or Azure SRIOV rules) can still live in their domain folder with a cloud marker, or in a cloud_specific/ folder if they're truly provider-only
There was a problem hiding this comment.
I assume you're proposing a new structure for the existing tests?
Yeah that would work.
We need to clarify first whether any of the existing tests will even continue to be maintained by us or whether all of this ownership moves to another team. If we keep maintaining these test cases in a separate repository we can do this. I wouldn't force a specific way of organizing tests on the development teams though as long as they keep the contract between CIV and their tests.
|
I had a quick read through and it looks good on first pass. I'm gone until next week Tuesday (back on the 21st), so I didn't want to leave this unread until I'm back, but I don't have time to do a nitpicky review now. So I'm not approving the PR, but overall, this looks very sane to me. Thanks! |
knivnia
left a comment
knivnia
left a comment
There was a problem hiding this comment.
Should we discuss and add a section about migration plan/order of operations? So other teams be more prepared for construction works?
| - test_suite/ (entire directory moves to separate repos) | ||
| - lib/test_lib.py (tests don't import from CIV) | ||
| - lib/aws_lib.py (moved to cloud/aws/) | ||
| - lib/console_lib.py (merged into cli.py) |
There was a problem hiding this comment.
line 579 says it'll move to console.py
There was a problem hiding this comment.
Good catch! console.py was based on an older draft. Messed that up in copy/pasting.
So cli.py is correct. I also notice that I forgot to add cloud/aws/ into the proposed file structure..
| - `main/cloud_image_validator.py` → `core/provisioner.py` (remove test execution) | ||
| - `lib/config_lib.py` → `core/config.py` (cleanup, type hints) | ||
| - `lib/ssh_lib.py` → `ssh/client.py` | ||
| - `lib/console_lib.py` → `utils/console.py` |
There was a problem hiding this comment.
line 433 says it'll move to cli.py
There was a problem hiding this comment.
Yep this is wrong, thanks for catching this!
Should not have added these changes in multiple places
| - `lib/console_lib.py` → `utils/console.py` | |
| - `lib/console_lib.py` → `cli.py` |
|
|
||
| ### Dependency Issues | ||
| 1. **No Separation of Test vs Runtime Dependencies** | ||
| - 9/18 packages in `requirements.txt` are test-only |
There was a problem hiding this comment.
| - 9/18 packages in `requirements.txt` are test-only | |
| - 9/17 packages in `requirements.txt` are test-only |
typo 😛
|
|
||
| This tight coupling creates significant maintenance burdens and prevents downstream teams from independently managing their test cases. | ||
|
|
||
| ### Current Code Problems |
There was a problem hiding this comment.
this part intersects with Section 6, should we consolidate them? Or shorten this part as sort of a 'motivation for change' and leave details for section 6
|
|
||
| 1. Use `/tmp/civ-*.json` or configurable path? | ||
| 2. Should CIV provision instances sequentially or in parallel? (Affects cloud rate limits) | ||
| 3. fail hard on invalid JUnit XML (exit code 2) or warn and continue with raw output? |
There was a problem hiding this comment.
imo, if we have
CIV enforces standardized test input and result reporting (JUnit XML) to maintain CI/CD pipeline compatibility
then I'd say it should be hard fail, otherwise what's the point? Also, that way it will be separated from linting warnings
| To be removed: | ||
| - test_suite/ (entire directory moves to separate repos) | ||
| - lib/test_lib.py (tests don't import from CIV) | ||
| - lib/aws_lib.py (moved to cloud/aws/) |
There was a problem hiding this comment.
| - lib/aws_lib.py (moved to cloud/aws/) | |
| - lib/aws_lib.py (integrate into cloud/...) |
Sounds good to me but I'd like to get overall agreement on whether we should proceed with this idea first. |
|
Reviewed this — thanks for the write up.
|
|
Thanks for your review @src-up
Yes it makes sense to have the baseline aka the generic test cases available per default. We can provide an option to run them through a flag like "--baseline" or "--baseline rhel9". I don't want to keep them in the same place as the CIV tool as I am aiming for a complete split of tooling and test cases. Moving the existing tests to a separate repo that can be loaded in along side your more specific test cases would be possible. E.g. that would be the step mentioned in line 430:
That said, it would make sense to be able to reference multiple test sources (e.g. repositories) for one test execution.
In short: No, the way CIV operates wouldn't change much. CIV will run in any execution environment which does not need to be the host you're testing. The flow would look like this:
|
|
Overall, I think this PR is in the right direction. |
6 participants
Summary
Proposes refactoring cloud-image-val from a monolithic Python test framework into a
minimal CLI tool focused solely on infrastructure orchestration. Tests would be
maintained independently by downstream teams in their own repositories.
Problem
The current codebase tightly couples infrastructure code (multi-cloud provisioning,
test execution) with test suites (RHEL/Fedora validation tests). This creates:
Proposed Solution
Transform cloud-image-val into infrastructure-only:
This ADR documents the architectural vision, current pain points, proposed API contracts, and migration strategy.
Review Focus
Note: This is a proposal document (ADR-001) for discussion, not a finished guide.