Fix infrastructure leak in template from volume creation error message - #12650
Fix infrastructure leak in template from volume creation error message#12650erikbocks wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12650 +/- ##
============================================
+ Coverage 16.26% 19.79% +3.53%
- Complexity 13428 20019 +6591
============================================
Files 5660 6371 +711
Lines 499963 575958 +75995
Branches 60708 70521 +9813
============================================
+ Hits 81330 114028 +32698
- Misses 409559 449495 +39936
- Partials 9074 12435 +3361
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates error handling around selecting an image (secondary) datastore so that API-facing exceptions no longer expose internal zone IDs, moving the detailed context into server logs instead.
Changes:
- Replace zone-ID-containing exception messages with a sanitized, user-facing
CloudRuntimeExceptionmessage. - Add error logging that retains the detailed context (zone ID) for operators.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@erikbocks could you have a look at Copilot's reviews? |
Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
clgtm, trivial change, smoke tests should do
@winterhazel , can you concur? |
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19118 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-16914)
|
|
[SF] Trillian test result (tid-16926)
|
|
@erikbocks @winterhazel , what is the status/your view on this? |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
| DataStore store = _dataStoreMgr.getImageStoreWithFreeCapacity(zoneId); | ||
| if (store == null) { | ||
| throw new CloudRuntimeException("cannot find an image store for zone " + zoneId); | ||
| throwExceptionForImageStoreObtentionFailure(zoneId, "create template"); |
| private void throwExceptionForImageStoreObtentionFailure(Long zoneId, String operation) { | ||
| logger.error("Cannot find an image store for zone [{}] while trying to {}.", zoneId, operation); | ||
| throw new CloudRuntimeException(String.format("Failed to %s. Please contact the cloud administrator.", operation)); |
| return _tmpltDao.findById(template.getId()); | ||
| } | ||
|
|
||
| private void throwExceptionForImageStoreObtentionFailure(Long zoneId, String operation) { |
Description
Currently, if an error occurs when trying to obtain a secondary storage for the creation of a template from a volume, or when uploading a volume, the message from the thrown exception exposes the zone's internal ID. Thus, the exception message was changed, and the descriptive message was moved to the logs.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
In an environment with only one secondary storage, I set it as
read-only. Then, I tried to create a template from a volume. An exception was thrown, informing that an error had occurred, but no infrastructure leak was present. I accessed the logs, and validated that the log with more information was shown, as well as the new exception message.