Add firewatch-granular-analysis step registry entry#80873
Conversation
Adds a step that parses JUnit XML test artifacts and extracts granular
metadata (operator names, component names, file locations) as Jira labels.
Writes labels to ${SHARED_DIR}/firewatch-additional-labels for consumption
by firewatch-report-issues. Runs with best_effort: true.
Supersedes openshift#80862 with a simpler shell-script approach that eliminates the
need for a separate repository and container image.
Relates: INTEROP-9185
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a new CI step Changesfirewatch-granular-analysis CI step
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
the
approved
Adds the firewatch-granular-analysis step before firewatch-report-issues in the interop-opp-aws test post chain. The step extracts structured labels from JUnit XML artifacts so firewatch-report-issues can apply them to Jira tickets. Relates: INTEROP-9185
|
@amp-rh, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: amp-rh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
removed
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/firewatch/granular-analysis/firewatch-granular-analysis-commands.sh`:
- Around line 47-57: The XML file discovery logic is too narrow and does not
match the Bash pre-check semantics. Currently, the code only checks the direct
`junit` subdirectory under artifact_dir and top-level XMLs, but it misses XML
files in nested junit directories at any depth (such as
`artifact_dir/some/path/junit/file.xml`). Refactor the xml_paths population to
recursively walk through all directories in artifact_dir using os.walk() to find
XML files under any nested `junit` directory path, ensuring that all XML files
matching the `*/junit/*.xml` pattern are discovered regardless of nesting depth.
- Around line 3-5: The firewatch-granular-analysis-commands.sh script is missing
the `errexit` option in its set command, which means command failures will not
cause the script to exit immediately. Add the `-e` flag to the existing `set -o
nounset` and `set -o pipefail` statement to make it `set -euo pipefail`,
following the standard step-registry baseline flags. This ensures the script
fails fast on any command error rather than continuing silently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8a677b43-f1ec-4dc8-8af3-176bcf42e8b2
📒 Files selected for processing (3)
ci-operator/step-registry/firewatch/granular-analysis/OWNERSci-operator/step-registry/firewatch/granular-analysis/firewatch-granular-analysis-commands.shci-operator/step-registry/firewatch/granular-analysis/firewatch-granular-analysis-ref.yaml
| set -o nounset | ||
| set -o pipefail | ||
|
|
There was a problem hiding this comment.
Enable errexit to avoid silent continuation on command failures.
This step omits -e, so unexpected command errors can be ignored before the explicit Python exit handling. Use the standard step-registry baseline flags.
As per coding guidelines, step-registry command scripts should use set -euo pipefail by default.
Suggested patch
-set -o nounset
-set -o pipefail
+set -euo pipefail🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ci-operator/step-registry/firewatch/granular-analysis/firewatch-granular-analysis-commands.sh`
around lines 3 - 5, The firewatch-granular-analysis-commands.sh script is
missing the `errexit` option in its set command, which means command failures
will not cause the script to exit immediately. Add the `-e` flag to the existing
`set -o nounset` and `set -o pipefail` statement to make it `set -euo pipefail`,
following the standard step-registry baseline flags. This ensures the script
fails fast on any command error rather than continuing silently.
Source: Coding guidelines
| xml_paths = [] | ||
| junit_dir = os.path.join(artifact_dir, "junit") | ||
| if os.path.isdir(junit_dir): | ||
| for name in os.listdir(junit_dir): | ||
| if name.endswith(".xml"): | ||
| xml_paths.append(os.path.join(junit_dir, name)) | ||
| for name in os.listdir(artifact_dir): | ||
| full = os.path.join(artifact_dir, name) | ||
| if name.endswith(".xml") and os.path.isfile(full) and full not in xml_paths: | ||
| xml_paths.append(full) | ||
|
|
There was a problem hiding this comment.
XML discovery in Python is narrower than the Bash pre-check, causing missed inputs.
Bash accepts XMLs under any */junit/*, but Python only reads ${artifact_dir}/junit and top-level XMLs. If files exist only in nested junit paths, the step reports zero extracted labels despite passing the initial Bash gate.
Suggested patch (align Python discovery with Bash semantics)
-xml_paths = []
-junit_dir = os.path.join(artifact_dir, "junit")
-if os.path.isdir(junit_dir):
- for name in os.listdir(junit_dir):
- if name.endswith(".xml"):
- xml_paths.append(os.path.join(junit_dir, name))
-for name in os.listdir(artifact_dir):
- full = os.path.join(artifact_dir, name)
- if name.endswith(".xml") and os.path.isfile(full) and full not in xml_paths:
- xml_paths.append(full)
+xml_paths = []
+seen = set()
+for root, _, files in os.walk(artifact_dir):
+ for name in files:
+ if not name.endswith(".xml"):
+ continue
+ full = os.path.join(root, name)
+ rel = os.path.relpath(full, artifact_dir)
+ is_top_level_xml = os.sep not in rel
+ is_under_junit = f"{os.sep}junit{os.sep}" in f"{os.sep}{rel}"
+ if (is_top_level_xml or is_under_junit) and full not in seen:
+ seen.add(full)
+ xml_paths.append(full)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ci-operator/step-registry/firewatch/granular-analysis/firewatch-granular-analysis-commands.sh`
around lines 47 - 57, The XML file discovery logic is too narrow and does not
match the Bash pre-check semantics. Currently, the code only checks the direct
`junit` subdirectory under artifact_dir and top-level XMLs, but it misses XML
files in nested junit directories at any depth (such as
`artifact_dir/some/path/junit/file.xml`). Refactor the xml_paths population to
recursively walk through all directories in artifact_dir using os.walk() to find
XML files under any nested `junit` directory path, ensuring that all XML files
matching the `*/junit/*.xml` pattern are discovered regardless of nesting depth.
|
@amp-rh, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@amp-rh: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
Summary
Adds a
firewatch-granular-analysisstep to the CI step registry that parses JUnit XML test artifacts and extracts granular metadata as Jira labels for consumption byfirewatch-report-issues.operator:,component:, andlocation:labels from failing test casesfirewatch-additional-labelsto${SHARED_DIR}(read byfirewatch-report-issues)firewatch-granular-data.jsonstructured report as a CI artifactbest_effort: trueso it cannot block existing jobsApproach
This is a shell script with an inline Python snippet that uses
xml.etree.ElementTree(stdlib, no pip deps). It replaces the approach in #80862 which required a new Go repository, container image, and CI pipeline.Files
ci-operator/step-registry/firewatch/granular-analysis/firewatch-granular-analysis-commands.shci-operator/step-registry/firewatch/granular-analysis/firewatch-granular-analysis-ref.yamlci-operator/step-registry/firewatch/granular-analysis/OWNERSfirewatch-owners(matches parent)Integration
Place in the post chain before
firewatch-report-issues:The
firewatch-report-issuesstep already checks for and consumes${SHARED_DIR}/firewatch-additional-labelsvia the--additional-labels-fileflag.Supersedes
Supersedes #80862 (same functionality, simpler implementation).
Relates: INTEROP-9185
Summary by CodeRabbit
This PR adds a new
firewatch-granular-analysisstep to the OpenShift CI step registry. This step enhances the existing Firewatch integration by extracting granular metadata from failing test cases in JUnit XML artifacts.What the step does:
The new step parses JUnit XML test result files and extracts three types of labels from failure information:
These extracted labels are written to
${SHARED_DIR}/firewatch-additional-labelsfor consumption by the existingfirewatch-report-issuesstep, and also saved asfirewatch-granular-data.jsonas a CI artifact for reporting purposes.Implementation details:
best_effort: true, ensuring failures won't block downstream jobsfirewatch-report-issuesto provide enhanced issue metadataFiles added:
firewatch-granular-analysis-commands.sh: Contains the JUnit parser and label extraction logicfirewatch-granular-analysis-ref.yaml: Step registry definition with resource limits and configurationOWNERS: References the firewatch-owners team for code ownershipThis change enables more detailed Jira issue reporting by automatically categorizing test failures, improving the ability to route and track infrastructure issues across multiple components.