Copilot commented on code in PR #13059:
URL: https://github.com/apache/cloudstack/pull/13059#discussion_r3130530721
##########
plugins/storage/volume/adaptive/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/AdaptiveDataStoreLifeCycleImpl.java:
##########
@@ -217,13 +217,14 @@ public DataStore initialize(Map<String, Object> dsInfos) {
// validate the provided details are correct/valid for the provider
api.validate();
- // if we have user-provided capacity bytes, validate they do not
exceed the manaaged storage capacity bytes
+ // User-provided capacityBytes always wins; validate against
storage stats only when
+ // the provider could actually report them. If the provider cannot
(empty pod with no
+ // footprint, no quota set, transient probe failure), fall through
and use what the
+ // user supplied rather than failing the whole registration.
ProviderVolumeStorageStats stats = api.getManagedStorageStats();
- if (capacityBytes != null && capacityBytes != 0 && stats != null) {
- if (stats.getCapacityInBytes() > 0) {
- if (stats.getCapacityInBytes() < capacityBytes) {
- throw new InvalidParameterValueException("Capacity
bytes provided exceeds the capacity of the storage endpoint: provided by user:
" + capacityBytes + ", storage capacity from storage provider: " +
stats.getCapacityInBytes());
- }
+ if (capacityBytes != null && capacityBytes != 0) {
+ if (stats != null && stats.getCapacityInBytes() > 0 &&
stats.getCapacityInBytes() < capacityBytes) {
+ throw new InvalidParameterValueException("Capacity bytes
provided exceeds the capacity of the storage endpoint: provided by user: " +
capacityBytes + ", storage capacity from storage provider: " +
stats.getCapacityInBytes());
}
parameters.setCapacityBytes(capacityBytes);
Review Comment:
The condition `capacityBytes != null && capacityBytes != 0` treats negative
values as valid user-provided capacity and will persist them (especially now
when provider stats are null). Negative capacity doesn't make sense and can
lead to broken capacity calculations elsewhere; consider validating
`capacityBytes > 0` (and/or throwing InvalidParameterValueException when
capacityBytes <= 0).
##########
plugins/storage/volume/adaptive/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/AdaptiveDataStoreLifeCycleImpl.java:
##########
@@ -217,13 +217,14 @@ public DataStore initialize(Map<String, Object> dsInfos) {
// validate the provided details are correct/valid for the provider
api.validate();
- // if we have user-provided capacity bytes, validate they do not
exceed the manaaged storage capacity bytes
+ // User-provided capacityBytes always wins; validate against
storage stats only when
+ // the provider could actually report them. If the provider cannot
(empty pod with no
+ // footprint, no quota set, transient probe failure), fall through
and use what the
+ // user supplied rather than failing the whole registration.
ProviderVolumeStorageStats stats = api.getManagedStorageStats();
- if (capacityBytes != null && capacityBytes != 0 && stats != null) {
- if (stats.getCapacityInBytes() > 0) {
- if (stats.getCapacityInBytes() < capacityBytes) {
- throw new InvalidParameterValueException("Capacity
bytes provided exceeds the capacity of the storage endpoint: provided by user:
" + capacityBytes + ", storage capacity from storage provider: " +
stats.getCapacityInBytes());
- }
+ if (capacityBytes != null && capacityBytes != 0) {
+ if (stats != null && stats.getCapacityInBytes() > 0 &&
stats.getCapacityInBytes() < capacityBytes) {
+ throw new InvalidParameterValueException("Capacity bytes
provided exceeds the capacity of the storage endpoint: provided by user: " +
capacityBytes + ", storage capacity from storage provider: " +
stats.getCapacityInBytes());
Review Comment:
The exception message has a grammar issue: "Capacity bytes provided
exceeds..." should be reworded (e.g., "Provided capacity bytes exceed...") to
be clearer and grammatically correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
