[pve-devel] [PATCH manager] ui: dc summary: fix calculation of shared storage size

[Fiona Ebner]

Am 30.07.24 um 15:44 schrieb Igor Thaller via pve-devel:
> The issue is related to the 'Summary' tab under 'Datacenter' inside a
> cluster. To get a steady reading of the storage size data, the frontend
> requests the '/api2/json/cluster/resources' every three seconds to
> retrieve the necessary data to calculate the used and total storage
> size.
> 
> The problem occurs when a shared storage is defined and a node goes
> offline. As the node is not online, it cannot report the shared storage
> size (both used and total) back to the other nodes. The order of the
> JSON response is not always the same, so it is possible that the offline
> node will appear first. Consequently, the front-end will display the
> wrong total and used storage. This is because the shared storage data
> has both the maximum disk size and the used disk set to zero when the
> node is offline. This causes the total and used space data to be
> calculated and displayed incorrectly, leading to fluctuations in the
> displayed percentage of used disk space.
> 
> To fix this, a conditional check to skip the storage report if its
> status is 'unknown' and it is shared was added. This prevents the
> unreliable data from being processed.
> 

Nit: "and it is shared was added" could be improved language-wise

While the issue is seen with shared storages, would there be a downside
of skipping non-shared storages with status "unknown" too? From a quick
look, it seems that disk/maxdisk not being set does not depend on the
storage being shared, so I'd rather skip the addition with unset values too.

> To test these changes, adjust the 'max_requests' variable in the Perl
> script located at '/usr/share/perl5/PVE/Service/pveproxy.pm' to increase
> the likelihood of the error to occur. This makes the storage size
> fluctuations more frequent. Then compare the storage results (both used
> and total sizes) before and after implementing the fix.
> 
> Note: Be aware that it takes around one minute for the spike to happen.
> 

IMHO this part should not go into the commit message, but be mentioned
below the "---"

> Reported-by: Friedrich Weber <f.weber at proxmox.com>
> Signed-off-by: Igor Thaller <igor.thaller at aon.at>
> ---
>  www/manager6/dc/Summary.js | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/www/manager6/dc/Summary.js b/www/manager6/dc/Summary.js
> index efb44dae..9b8f3280 100644
> --- a/www/manager6/dc/Summary.js
> +++ b/www/manager6/dc/Summary.js
> @@ -170,6 +170,11 @@ Ext.define('PVE.dc.Summary', {
>  			} else if (countedStorage[sid]) {
>  			    break;
>  			}
> +
> +			if (data.status === "unknown" && data.shared) {
> +			    break;
> +			}
> +
>  			used += data.disk;
>  			total += data.maxdisk;
>  			countedStorage[sid] = true;
> -- 
> 2.39.2
> 
>