-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KEP-3476: Add Volume Group Snapshot KEP #1551
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For creating a VolumeGroup
I see three paths
- Create an empty
VolumeGroup
and request new un-provisioned volume(s) to be provisioned and added one by one. - Create an empty
VolumeGroup
and request existing volume(s) to be added one by one. - Create an non-empty
VolumeGroup
with a fixed set of new un-provisioned volume(s) to be provisioned. - Create an non-empty
VolumeGroup
with a fixed existing set of volume(s).
Different storage vendors may support one more of these. We need to identify what is out there to figure out which of these cases to support.
To add to the complexity this may vary based on VolumeGroup
feature: snapshot vs replication vs placement vs crash consistent writes.
Similarly, for snapshot restore there may be multiple possibilities:
- Group Snapshot returns only a
GroupSnapshotID
not individual snapshot IDs, therefore restore must be via aGroupSnapshotID
. - Group Snapshot returns only individual
VolumeSnapshotID
, therefore restore must be on a per snapshot basis.
I added an item to the SIG Storage Meeting Agenda tomorrow to ask for storage vendor input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have left comments. Please feel free to contact me Thomas.Watson@dell.com if you wish.
7c7d0fd
to
f94abd3
Compare
Consider a statefulset have 3 replicas, and having 2 claims in volumeClaimTemplates.. totally, it becomes 6 PVCs This is a common usecase where application is providing high availability by storing data in such a way that even if one pod goes down, application is running. But, if the underlying storage of those 6 PVCs comes from storage of single failure domain, application cannot run. |
bb52015
to
7b1cca4
Compare
I think that should be possible. For the 2 PVCs on the 1st replica, we can add label replica-1, for the 2 PVCs on the 2nd replica, we can add label replica-2, and so on. We can create 3 VolumeGroups, one for PVCs on each replica. This is Immutable VolumeGroup with existing PVCs. To support Mutable VolumeGroup with StatefulSet and place PVCs in each replica in a different group would require enhancement of the StatefulSet controller. If we always create a different group for PVCs in each replica, we can modify the StatefulSet controller to add the pod name as a suffix to the group name, but if we sometimes need to create a different group for PVCs in each replica, sometimes need to create just 1 group for all the PVCs in all replicas, and sometimes need to create a group for 2 out of 3 replicas, it will be tricky. |
cbd977f
to
9a50a09
Compare
2a26311
to
03c1e17
Compare
03c1e17
to
bc3c940
Compare
@msau42 I addressed your comments. PTAL. Thanks. |
// +optional | ||
Message *string | ||
VolumeSnapshotRefList []core_v1.ObjectReference |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these can only be snapshot kinds, we could make our own SnapshotReference type that only includes name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we only need snapshot names, can it just be a list of strings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can follow up on this during a more detailed API review, but I believe the guidance is to define a type scoped to your supported sources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
/lgtm |
…-configuration USHIFT-2196: User-facing audit-log configuration
This PR proposes a VolumeGroupSnapshot CRD.