Fix the valid_cdi_certificate fn to fetch utc time instead of localtime. #2148
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughReplaced time-based parsing and naive timestamps in a test with timezone-aware UTC datetime parsing and comparisons; removed the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
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 |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot-2
requested review from
duyanyan,
jpeimer,
kgoldbla,
kshvaika and
stesrn
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
previously approved these changes
Sep 23, 2025
![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 (3)
tests/storage/test_cdi_certificate.py (3)
85-87: Prefer datetime.timezone.utc over datetime.UTC for broader compatibilitydatetime.UTC exists only in Python 3.11+. Use datetime.timezone.utc to avoid AttributeError on 3.10 and earlier.
- start_timestamp = start_date.replace(tzinfo=datetime.UTC).timestamp() + start_timestamp = start_date.replace(tzinfo=datetime.timezone.utc).timestamp()Please confirm the CI runtime Python version; if it's <3.11 this change is required.
89-91: Same UTC constant issue hereAlign with datetime.timezone.utc for compatibility.
- end_timestamp = end_date.replace(tzinfo=datetime.UTC).timestamp() + end_timestamp = end_date.replace(tzinfo=datetime.timezone.utc).timestamp()
92-95: Simplify UTC “now” computation; avoid stringify/parse round‑tripNo need to strftime/strptime; compute the aware UTC timestamp directly. Also avoid datetime.UTC for compatibility.
- current_time = datetime.datetime.now(datetime.timezone.utc).strftime(RFC3339_FORMAT) - current_date = datetime.datetime.strptime(current_time, RFC3339_FORMAT) - current_timestamp = current_date.replace(tzinfo=datetime.UTC).timestamp() + current_timestamp = datetime.datetime.now(datetime.timezone.utc).timestamp()Optional (cleaner): compare aware datetimes instead of floats (fewer conversions):
- start_date = datetime.datetime.strptime(start, RFC3339_FORMAT) - start_timestamp = start_date.replace(tzinfo=datetime.timezone.utc).timestamp() - end_date = datetime.datetime.strptime(end, RFC3339_FORMAT) - end_timestamp = end_date.replace(tzinfo=datetime.timezone.utc).timestamp() - current_timestamp = datetime.datetime.now(datetime.timezone.utc).timestamp() - assert start_timestamp <= current_timestamp <= end_timestamp, f"Certificate of {cdi_secret} expired" + start_dt = datetime.datetime.strptime(start, RFC3339_FORMAT).replace(tzinfo=datetime.timezone.utc) + end_dt = datetime.datetime.strptime(end, RFC3339_FORMAT).replace(tzinfo=datetime.timezone.utc) + now_dt = datetime.datetime.now(datetime.timezone.utc) + assert start_dt <= now_dt <= end_dt, f"Certificate of {cdi_secret} not valid at current time"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/storage/test_cdi_certificate.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-container
- GitHub Check: tox
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
|
/approve |
rnetser
previously approved these changes
Sep 25, 2025
There was a problem hiding this comment.
nit:
see CodeRabbit's suggestion
+ start_dt = datetime.datetime.strptime(start, RFC3339_FORMAT).replace(tzinfo=datetime.timezone.utc)
+ end_dt = datetime.datetime.strptime(end, RFC3339_FORMAT).replace(tzinfo=datetime.timezone.utc)
+ now_dt = datetime.datetime.now(datetime.timezone.utc)
+ assert start_dt <= now_dt <= end_dt, f"Certificate of {cdi_secret} not valid at current time"
|
/lgtm |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
/verified |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
openshift-virtualization-qe-bot
approved these changes
Sep 26, 2025
|
/approve |
|
/approve |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Short description:
The Current implementation of valid_cdi_certificate assumes the local time on the server where we run the tests is utc. But the timezone can be different in machine we are running the tests. So I have changed the function to use utc timezone as the certificate before and after times are in utc.
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
Summary by CodeRabbit
Bug Fixes
Tests