test(robot): fix recurring job test cases for v2 volumes #2196
Conversation
Signed-off-by: Yang Chiu <[email protected]>
WalkthroughThe pull request introduces a modification to the Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
e2e/libs/recurringjob/rest.py (1)
133-134: Consider handling mixed v1/v2 volume scenarios.The current implementation assumes v2 volume schema. Consider adding version detection or fallback logic to support testing in environments with mixed v1/v2 volumes.
- if 'RecurringJob' in item['spec']['labels'] and \ - item['spec']['labels']['RecurringJob'] == job_name and \ + # Support both v1 and v2 volume schemas + labels = item.get('spec', {}).get('labels') or item.get('status', {}).get('labels', {}) + if 'RecurringJob' in labels and \ + labels['RecurringJob'] == job_name and \
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
e2e/libs/recurringjob/rest.py(1 hunks)
🔇 Additional comments (1)
e2e/libs/recurringjob/rest.py (1)
133-134: LGTM! Verify alignment with v2 volume spec.
The change from status.labels to spec.labels aligns with v2 volumes schema. However, let's verify this matches the official spec.
✅ Verification successful
Change to spec.labels is correct for recurring job snapshots
The code change is verified to be correct based on the following evidence:
- The comments in the code explicitly state that "job_name is in spec.labels.RecurringJob"
- No contradicting label patterns were found in the codebase
- The test file itself serves as documentation for the correct behavior, as it's part of the e2e test suite
The change aligns with the documented behavior and maintains consistency with how recurring job labels are handled in the system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the label location in v2 volume CRD spec
kubectl get crd snapshots.longhorn.io -o yaml | grep -A 10 "spec:"
Length of output: 164
Script:
#!/bin/bash
# Let's search for snapshot and volume CRD definitions in the codebase
fd -e yaml -e yml | xargs rg -l "kind:\s*(Volume|Snapshot)"
# Then let's look for label usage patterns in the codebase
rg -A 5 "spec.*labels.*RecurringJob"
Length of output: 1140
Script:
#!/bin/bash
# Let's look for any other label usage patterns in snapshots
rg -B 2 -A 2 "labels.*RecurringJob"
# Let's also check for any test files or documentation that might show the schema
fd "snapshot|volume" | rg -l "v2"
Length of output: 2534
Which issue(s) this PR fixes:
Issue longhorn/longhorn#10001
What this PR does / why we need it:
fix recurring job test cases for v2 volumes
Special notes for your reviewer:
Additional documentation or context
Summary by CodeRabbit
Bug Fixes
Chores