USHIFT-7381: Add --local mode to PCP dashboard generator#238
Conversation
Enable generate-dashboard.sh to work with local scenario-info directories (via --local flag) in addition to Prow URLs (--url). This allows CI post steps to generate PCP dashboards directly from artifacts on the hypervisor without downloading from GCS. New flags: --local, --build-id, --output, --title. Also adds --title support to create-pcp-dashboard.py for custom HTML titles, and fixes symlink traversal in find/os.walk for local mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Prevent symlink cycles in extract_scenarios.py by tracking visited (st_dev, st_ino) pairs during os.walk with followlinks=True - Validate --build-id against allow-list [a-zA-Z0-9._-] to reject path traversal sequences - Fix --local help text to clarify it expects the scenario-info/ directory path itself Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…modes Address review: remove --build-id flag (hardcode "local" for local mode), make --output and --title available in both --url and --local modes for consistent interface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: suleymanakbas91 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughChangesPCP dashboard generation
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 9 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@plugins/microshift-ci/scripts/pcp-graphs/create-pcp-dashboard.py`:
- Around line 318-319: Wrap the overlong statements in build_html and the
additional lines around 392–393 so each Python line stays within the
79-character PEP 8 limit, preserving the existing function signature and
behavior.
In `@plugins/microshift-ci/scripts/pcp-graphs/extract_scenarios.py`:
- Around line 22-33: Move the `seen` set initialization inside the `for build_id
in os.listdir(artifacts_root)` loop so inode de-duplication is reset for each
logical build while preserving cycle protection within that build’s `os.walk`
traversal. Add regression coverage for multiple symlinked build IDs pointing to
the same directory and for cyclic links, verifying scenarios are retained per
build and traversal terminates.
In `@plugins/microshift-ci/scripts/pcp-graphs/generate-dashboard.sh`:
- Around line 88-91: Update process_hypervisor_dir to retain and process the
local build directory when numeric ID extraction fails, using the existing
BUILD_ID value (including “local”) as the fallback; preserve numeric extraction
behavior for other builds.
- Line 87: Update the --local argument parsing in generate-dashboard.sh to
reject paths containing parent-traversal components before LOCAL_PATH is
canonicalized with cd and pwd. Preserve valid relative and absolute paths, and
add tests covering both accepted paths and rejected ../ traversal inputs.
🪄 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: 2cc28a06-126f-4ce4-8d26-402f48d0e859
📒 Files selected for processing (5)
.claude-plugin/marketplace.jsonplugins/microshift-ci/.claude-plugin/plugin.jsonplugins/microshift-ci/scripts/pcp-graphs/create-pcp-dashboard.pyplugins/microshift-ci/scripts/pcp-graphs/extract_scenarios.pyplugins/microshift-ci/scripts/pcp-graphs/generate-dashboard.sh
6ef682b to
1405625
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The title is not parameterized — hardcode "PCP Performance Dashboard" in the HTML template directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/lgtm |
Summary
--localmode togenerate-dashboard.shso CI post steps can generate PCP dashboards from local artifact directories without downloading from GCSfind/os.walkto follow symlinks, with cycle detection via inode trackingSupersedes #233 (rebased, GitHub lost track of force-pushed commits).
Context
This is the edge-tooling side of adding a "Test PCP" toggle to MicroShift Prow job results. The companion PR in
openshift/releasewill add a CI post step that:generate-dashboard.sh --local ... --output custom-link-pcp.htmlTest plan
--localmode with real Prow job artifacts: 2 scenarios, 4/4 metrics each--urlmode unchanged (backward compatible)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores