[ALPHA] Add volume health proposal#604
Conversation
8789044 to
456214c
Compare
|
I was thinking about this use case, but where would we report it? In `CSIDriver` object? I do not think, CSI driver object was designed for such use case, but idea is sound in theory.
I am intentionally keeping some of the health reporting as future enhancement, so as we can ship something rather than being bogged down by designing everything upfront. I am adding a section in kubernetes/enhancements#6138 to keep track of future enhancements. I will update
Thanks for the consideration. There is no need to include such a feature with this PR, but it would be nice if we can add it at a later time.
The (currently) only location where this makes sense to report, is at the StorageClass level. Depending on the parameters/secrets, a part of a storage backend could be unhealthy (like a pool of disks, HDD vs NVMe, or network connection type, or the like).
|
nixpanic
left a comment
There was a problem hiding this comment.
The current proposal looks good to me, and it should be workable with the CSI-drivers for Ceph.
| // being accessible from that node. | ||
| INACCESSIBLE = 2; | ||
|
|
||
| // Data loss is known or strongly suspected on underlying volume. |
There was a problem hiding this comment.
Permanent data loss is known or suspected.....
This is to distinguish from situations where data was lost but self-healing will eventually repair the problem.
We may also want to add text here indicating that we recommend discarding this volume and rebuilding from other (higher level) replicas if possible, because this is supposed to be one of the actionable health states and we don't want there to be any confusion about what its intended use is.
There was a problem hiding this comment.
Permanent data loss is known or suspected.....
+1
We may also want to add text here indicating that we recommend discarding this volume and rebuilding from other (higher level) replicas if possible, because this is supposed to be one of the actionable health states and we don't want there to be any confusion about what its intended use is.
So far we have avoided being too prescriptive on remediation (notice inaccessible doesn't say do not schedule workload that uses this volume on this node). I think that returned health condition is basically a suggestion to the CO and the workload (managed via some kind of operator), at the end of day - it is impossible to enforce this one way or the other from CSI perspective, so being very prescriptive IMO does not add much value.
There was a problem hiding this comment.
I agree we want to leave a little wiggle room, but too much wiggle room makes it impossible for SPs to implement the feature. An SP will worry that reporting a health condition could result in unknown COs doing unknown inappropriate things in response to that health condition, and could choose to just never report anything other than degraded for fear of some unintented outcome. If we constrain what the CO can/should do in response to any given condition, it will give the SP authors confidence.
There was a problem hiding this comment.
Okay I have updated the wording to following:
// Permanent data loss is known or strongly suspected on the
// underlying volume. The CO MAY use this signal to inform
// users to trigger recovery workflows such as restoring from
// a snapshot or replica.
Make ControllerGetVolumeHealth required if LIST_VOLUME_HEALTH is implemented
85d1755 to
72e2e56
Compare
|
@bswartz addressed the review comments PTAL |
|
/lgtm |
|
lgtm |
Implements spec as proposed in https://github.com/kubernetes/enhancements/blob/master/keps/sig-storage/1432-volume-health-monitor/README.md