Repository navigation
Conversation
Extend verify_xcopy_used() and verify_xcopy_used_per_datastore() to verify the storage-offload observability annotations Forklift propagates from the volume populator to Plan DiskTransfer task annotations (MTV-6086): result, copyDurationSeconds, storageVendor, cloneMethod, storageProtocol, provisionedBytes, and allocatedBytes. - result must be "success", copyDurationSeconds and provisionedBytes must be positive numbers, allocatedBytes must not be negative - storageVendor, cloneMethod, and storageProtocol must be present (Forklift never propagates empty values) - values are reported on both XCOPY and software-fallback copies, so all existing copy-offload test classes get the coverage with no test-file changes - dedupe annotation reading via _get_task_annotation_value; add _get_task_number_annotation for numeric parsing - document the verification in docs/copy-offload-migrations.md and regenerate the docsite
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
|
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
Branch Management
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
Security Checks
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
…-plan-observability # Conflicts: # docs/llms-full.txt # docs/search-index.json
PR Summary by QodoVerify copy-offload observability annotations on Plan tasks
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
…, Any justifications (MTV-7021) - _get_task_number_annotation rejects non-finite values (inf/NaN/overflow) with a contextual ValueError before the sign checks - storage-offload observability annotation verification is now gated on MTV >= 2.12.6 (first release propagating the full MTV-6086 annotation set), via has_mtv_minimum_version(); older clusters skip those checks with a log line - added justification comments for the four task: Any helper parameters (dynamic Kubernetes objects without a typed wrapper) - docs updated for the version gate; docsite regenerated
|
Code review by qodo was updated up to the latest commit 186a8fb |
|
/qodo The following review comments were reviewed and a decision was made:
|
Acknowledged. The current review status is:
No code changes are needed based on these decisions. |
|
/qodo Please re-evaluate all remaining sticky findings against the current code. |
Re-evaluated the one remaining active finding:
For the rule scope or applicability, update or disable the rule here: https://app.qodo.ai/rules/1255924?state=active |
|
/qodo The following review comments were reviewed and a decision was made:
|
The repository guidance supports your decision: |
…-plan-observability # Conflicts: # docs/search-index.json
…-plan-observability # Conflicts: # docs/search-index.json
Summary
Automates MTV-7021 ([Automation] Xcopy Plan Status & Observability) under epic MTV-6086.
Forklift (PR kubev2v/forklift#7278 + follow-ups, in MTV 5.0.0 and recent 2.12.z) propagates the volume populator's copy details to Plan CR
DiskTransfertask annotations after copy-offload migrations. ThexcopyUsedannotation was already verified byverify_xcopy_used(); this PR extends the verification to the remaining observability annotations:resultmust besuccesscopyDurationSecondsandprovisionedBytesmust be positive numbersallocatedBytesmust not be negativestorageVendor,cloneMethod, andstorageProtocolmust be present (Forklift never propagates empty values, so presence is the contract)Implementation
_get_task_annotation_value()(generic annotation reader — dedupes_get_xcopy_used_annotation, which now delegates to it)_get_task_number_annotation()for fail-fast numeric parsing_verify_task_offload_annotations()performing the per-task checks, called from_verify_xcopy_plan_annotations()and_verify_xcopy_plan_annotations_per_datastore()test_check_xcopy_usedsteps get the coverage with zero test-file changesdocs/copy-offload-migrations.md, docsite regeneratedVerification
mainandrelease-2.12: all 8 annotations are propagated non-empty on successful copies (both XCOPY and fallback paths);copyDurationSecondsis a raw Prometheus float string, never integer-truncatedResolves: MTV-7021