controllers: set storage utilization ratio in heartbeat #254
Conversation
rchikatw
commented
Oct 9, 2024
•
rchikatw
commented
- set storage utilization ratio in a heartbeat
- This will be calculated from a CRQ resource
- This field will be used to show the storage utilization of the client in the provider mode dashboard
- go.mod file changes was added by taking this PR into account: Adding StorageQuotaUtilizationRatio field for storage consumer api ocs-operator#2697
- Initial PR, Need to test
rchikatw
force-pushed
the
uiclustername
branch
5 times, most recently
from
79fe559
to
f9a97fd
Compare
rchikatw
force-pushed
the
uiclustername
branch
from
f9a97fd
to
6758d81
Compare
...or/github.com/red-hat-storage/ocs-operator/services/provider/api/v4/interfaces/interfaces.go
Show resolved
Hide resolved
rchikatw
force-pushed
the
uiclustername
branch
4 times, most recently
from
98a75ad
to
e159cf2
Compare
|
LGTM. will wait for other review. |
service/status-report/main.go
Outdated
| if ratio > 1 { | ||
| status.SetStorageQuotaUtilizationRatio(1) | ||
| } else { | ||
| status.SetStorageQuotaUtilizationRatio(float32(ratio)) |
There was a problem hiding this comment.
are we going to show only ratio? and not capacity utilized
There was a problem hiding this comment.
@rohan47 Because this is not about ODF capacity utilization, the quota reports the number of bytes out of the quota for total cluster storage utilization. I made the call to use ratio because we are talking about a number that describes how close we are to the quota, I agree that the world utilization here is problematic, but we couldn't find better terminology. If you have a suggestion for a more fitting terminology, please reply
| klog.Warningf("Failed to get clusterResourceQuota %q: %v", clusterResourceQuota.Name, err) | ||
| } | ||
|
|
||
| if clusterResourceQuota.Status.Total.Hard != nil { |
There was a problem hiding this comment.
Are we sure none of parts of .Status.Total.Hard is a pointer? If any is you will need a nil pointer check
There was a problem hiding this comment.
Yes, I have verified with multiple scenario.
service/status-report/main.go
Outdated
| if ratio > 1 { | ||
| status.SetStorageQuotaUtilizationRatio(1) | ||
| } else { | ||
| status.SetStorageQuotaUtilizationRatio(float32(ratio)) | ||
| } |
There was a problem hiding this comment.
- What if total is 0, you cannot divide by 0
- Why an
if ... else ...? it makes more sense usingmath.Minto get the min between the ratio and 1
There was a problem hiding this comment.
- Total will never be zero here because we are creating a resource, and we will always have a value greater than 0. We can't generate a token with a value of 0 because that validation is added as part of the token generation API. I will include that in my upcoming commit once you let me know about my point 2. the validation for that edge case(If the client admin created a CRQ resource with a quota on a different resource.).
- math.Min is not available for float32 I have to do lot of to and from conversion like this
status.SetStorageQuotaUtilizationRatio(float32(math.Min(float64(ratio), float64(1.0))))
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
rchikatw
force-pushed
the
uiclustername
branch
from
e159cf2
to
038aac1
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rchikatw The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: rchikatw <[email protected]>
rchikatw
force-pushed
the
uiclustername
branch
from
038aac1
to
d7a9a09
Compare
