Conversation
There was a problem hiding this comment.
Code Review
The pull request refactors the testcloud image preparation logic to include validation and retry mechanisms. It introduces new constants for image preparation retry attempts and interval, imports a retry utility, and wraps the image preparation and validation steps in a retry loop. A new validation step uses qemu-img info to verify the integrity of downloaded qcow2 images, removing corrupt ones and raising a ProvisionError if validation fails. Review comments suggest improving the descriptive comment for the new retry constants and enhancing the clarity and readability of the ProvisionError message when a corrupt image is encountered.
| f"'{self.testcloud_image_dirpath}' directory permissions." | ||
| ) from error | ||
| except KeyError as error: | ||
| raise ProvisionError(f"Failed to prepare image '{self.image_url}'.") from error |
There was a problem hiding this comment.
If any of these exceptions occur, the function will get retried by rety(). Do they have a chance of succeeding if retried? If not, only 15 seconds are wasted, so it doesn't seem like a huge deal. Just wondering if it was intended that these would also cause retries.
Pull Request Checklist
Fixes #4557
First test mentioned above sometimes fail due to the timeout being too fast and cuts off right before
/plans/multi/plan/onegets through, so I've added another expect phrase that comes after that line and increased timeout to 3 seconds.Second test imho failed due to messed up download of the qcow2 image.
The unindented "Unknown download size." error seems odd. I've checked successful runs of that test in other PRs and "Unknown download size" does not appear there. Another line says:
So I think that something did get downloaded but the file was corrupted.
I wrapped the responsible code part into a function
prepare_imageand added a validation of the qcow2. If that fails, the downloaded file will be removed andprepare_imagewill be retried.Hard to fix flaky test when they're flaky and can't be reproduced at will 😐
Co-generated by Claude Code.