Skip to content
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.

Sign up for GitHub

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

Update API for pre provisioned group snapshots #971

Merged
merged 3 commits into from
Jan 12, 2024
Merged

Update API for pre provisioned group snapshots #971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bffa5ae
Update to API for static group volume snapshots
RaunakShah Dec 5, 2023
0bd355c
update API names in group snapshot content to be consistent with volu…
RaunakShah Jan 5, 2024
e6dc65f
Fix unit test and remove comments
RaunakShah Jan 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions client/apis/volumegroupsnapshot/v1alpha1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,16 +348,33 @@ type VolumeGroupSnapshotContentStatus struct {
// Exactly one of its members must be set.
// Members in VolumeGroupSnapshotContentSource are immutable.
type VolumeGroupSnapshotContentSource struct {
// PersistentVolumeNames is a list of names of PersistentVolumes to be snapshotted
// VolumeHandles is a list of volume handles on the backend to be snapshotted
// together. It is specified for dynamic provisioning of the VolumeGroupSnapshot.
// This field is immutable.
// +optional
PersistentVolumeNames []string `json:"persistentVolumeNames,omitempty" protobuf:"bytes,1,opt,name=persistentVolumeNames"`
VolumeHandles []string `json:"volumeHandles,omitempty" protobuf:"bytes,1,opt,name=volumeHandles"`

// GroupSnapshotHandles specifies the CSI "group_snapshot_id" of a pre-existing
// group snapshot and a list of CSI "snapshot_id" of pre-existing snapshots
// on the underlying storage system for which a Kubernetes object
// representation was (or should be) created.
// This field is immutable.
// +optional
GroupSnapshotHandles *GroupSnapshotHandles `json:"groupSnapshotHandles,omitempty" protobuf:"bytes,2,opt,name=groupSnapshotHandles"`
}

type GroupSnapshotHandles struct {
// VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id" of a pre-existing
// group snapshot on the underlying storage system for which a Kubernetes object
// representation was (or should be) created.
// This field is immutable.
// +optional
VolumeGroupSnapshotHandle *string `json:"volumeGroupSnapshotHandle,omitempty" protobuf:"bytes,2,opt,name=volumeGroupSnapshotHandle"`
// Required.
VolumeGroupSnapshotHandle string `json:"volumeGroupSnapshotHandle" protobuf:"bytes,1,opt,name=volumeGroupSnapshotHandle"`

// VolumeSnapshotHandles is a list of CSI "snapshot_id" of pre-existing
// snapshots on the underlying storage system for which Kubernetes objects
// representation were (or should be) created.
// This field is immutable.
// Required.
VolumeSnapshotHandles []string `json:"volumeSnapshotHandles" protobuf:"bytes,2,opt,name=volumeSnapshotHandles"`
}
35 changes: 28 additions & 7 deletions client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion client/apis/volumesnapshot/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 32 additions & 13 deletions client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/814"
api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/971"
controller-gen.kubebuilder.io/version: v0.12.0
creationTimestamp: null
name: volumegroupsnapshotcontents.groupsnapshot.storage.k8s.io
Expand Down Expand Up @@ -104,23 +104,42 @@ spec:
dynamically provisioned or already exists, and just requires a Kubernetes
object representation. This field is immutable after creation. Required.
properties:
persistentVolumeNames:
description: PersistentVolumeNames is a list of names of PersistentVolumes
to be snapshotted together. It is specified for dynamic provisioning
of the VolumeGroupSnapshot. This field is immutable.
groupSnapshotHandles:
description: GroupSnapshotHandles specifies the CSI "group_snapshot_id"
of a pre-existing group snapshot and a list of CSI "snapshot_id"
of pre-existing snapshots on the underlying storage system for
which a Kubernetes object representation was (or should be)
created. This field is immutable.
properties:
volumeGroupSnapshotHandle:
description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
of a pre-existing group snapshot on the underlying storage
system for which a Kubernetes object representation was
(or should be) created. This field is immutable. Required.
type: string
volumeSnapshotHandles:
description: VolumeSnapshotHandles is a list of CSI "snapshot_id"
of pre-existing snapshots on the underlying storage system
for which Kubernetes objects representation were (or should
be) created. This field is immutable. Required.
items:
type: string
type: array
required:
- volumeGroupSnapshotHandle
- volumeSnapshotHandles
type: object
volumeHandles:
description: VolumeHandles is a list of volume handles on the
backend to be snapshotted together. It is specified for dynamic
provisioning of the VolumeGroupSnapshot. This field is immutable.
items:
type: string
type: array
volumeGroupSnapshotHandle:
description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
of a pre-existing group snapshot on the underlying storage system
for which a Kubernetes object representation was (or should
be) created. This field is immutable.
type: string
type: object
oneOf:
- required: ["persistentVolumeNames"]
- required: ["volumeGroupSnapshotHandle"]
- required: ["volumeHandles"]
- required: ["groupSnapshotHandles"]
volumeGroupSnapshotClassName:
description: VolumeGroupSnapshotClassName is the name of the VolumeGroupSnapshotClass
from which this group snapshot was (or will be) created. Note that
Expand Down
45 changes: 32 additions & 13 deletions client/hack/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,30 +144,49 @@ Update the restoreSize property to use type string only:

* Add the VolumeSnapshot namespace to the `additionalPrinterColumns` section. Refer https://github.com/kubernetes-csi/external-snapshotter/pull/535 for more details.

* In `client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml `, we need to add the `oneOf` constraint to make sure only one of `persistentVolumeNames` and `volumeGroupSnapshotHandle` is specified in the `source` field of the `spec` of `VolumeGroupSnapshotContent`.
* In `client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml `, we need to add the `oneOf` constraint to make sure only one of `volumeHandles` and `groupSnapshotHandles` is specified in the `source` field of the `spec` of `VolumeGroupSnapshotContent`.

```bash
source:
description: Source specifies whether the snapshot is (or should be)
dynamically provisioned or already exists, and just requires a Kubernetes
object representation. This field is immutable after creation. Required.
properties:
persistentVolumeNames:
description: PersistentVolumeNames is a list of names of PersistentVolumes
to be snapshotted together. It is specified for dynamic provisioning
of the VolumeGroupSnapshot. This field is immutable.
groupSnapshotHandles:
description: GroupSnapshotHandles specifies the CSI "group_snapshot_id"
of a pre-existing group snapshot and a list of CSI "snapshot_id"
of pre-existing snapshots on the underlying storage system for
which a Kubernetes object representation was (or should be)
created. This field is immutable.
properties:
volumeGroupSnapshotHandle:
description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
of a pre-existing group snapshot on the underlying storage
system for which a Kubernetes object representation was
(or should be) created. This field is immutable. Required.
type: string
volumeSnapshotHandles:
description: VolumeSnapshotHandles is a list of CSI "snapshot_id"
of pre-existing snapshots on the underlying storage system
for which Kubernetes objects representation were (or should
be) created. This field is immutable. Required.
items:
type: string
type: array
required:
- volumeGroupSnapshotHandle
- volumeSnapshotHandles
type: object
volumeHandles:
description: VolumeHandles is a list of volume handles on the
backend to be snapshotted together. It is specified for dynamic
provisioning of the VolumeGroupSnapshot. This field is immutable.
items:
type: string
type: array
volumeGroupSnapshotHandle:
description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
of a pre-existing group snapshot on the underlying storage system
for which a Kubernetes object representation was (or should
be) created. This field is immutable.
type: string
type: object
oneOf:
- required: ["persistentVolumeNames"]
- required: ["volumeGroupSnapshotHandle"]
- required: ["volumeHandles"]
- required: ["groupSnapshotHandles"]
volumeGroupSnapshotClassName:
```
20 changes: 10 additions & 10 deletions pkg/common-controller/groupsnapshot_controller_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ func (ctrl *csiSnapshotCommonController) syncUnreadyGroupSnapshot(groupSnapshot

if contentObj != nil {
klog.V(5).Infof("Found VolumeGroupSnapshotContent object %s for group snapshot %s", contentObj.Name, uniqueGroupSnapshotName)
if contentObj.Spec.Source.VolumeGroupSnapshotHandle != nil {
if contentObj.Spec.Source.GroupSnapshotHandles != nil {
ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotHandleSet", fmt.Sprintf("GroupSnapshot handle should not be set in group snapshot content %s for dynamic provisioning", uniqueGroupSnapshotName))
return fmt.Errorf("VolumeGroupSnapshotHandle should not be set in the group snapshot content for dynamic provisioning for group snapshot %s", uniqueGroupSnapshotName)
}
Expand Down Expand Up @@ -483,7 +483,7 @@ func (ctrl *csiSnapshotCommonController) getPreprovisionedGroupSnapshotContentFr
return nil, nil
}
// check whether the content is a pre-provisioned VolumeGroupSnapshotContent
if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle == nil {
if groupSnapshotContent.Spec.Source.GroupSnapshotHandles == nil {
// found a group snapshot content which represents a dynamically provisioned group snapshot
// update the group snapshot and return an error
ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotContentMismatch", "VolumeGroupSnapshotContent is dynamically provisioned while expecting a pre-provisioned one")
Expand Down Expand Up @@ -682,7 +682,7 @@ func (ctrl *csiSnapshotCommonController) getDynamicallyProvisionedGroupContentFr
return nil, nil
}
// check whether the group snapshot content represents a dynamically provisioned snapshot
if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil {
if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil {
ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotContentMismatch", "VolumeGroupSnapshotContent "+contentName+" is pre-provisioned while expecting a dynamically provisioned one")
klog.V(4).Infof("sync group snapshot[%s]: group snapshot content %s is pre-provisioned while expecting a dynamically provisioned one", utils.GroupSnapshotKey(groupSnapshot), contentName)
return nil, fmt.Errorf("group snapshot %s expects a dynamically provisioned VolumeGroupSnapshotContent %s but gets a pre-provisioned one", utils.GroupSnapshotKey(groupSnapshot), contentName)
Expand Down Expand Up @@ -752,9 +752,9 @@ func (ctrl *csiSnapshotCommonController) createGroupSnapshotContent(groupSnapsho
if err != nil {
return nil, err
}
var pvNames []string
var volumeHandles []string
for _, pv := range volumes {
pvNames = append(pvNames, pv.Name)
volumeHandles = append(volumeHandles, pv.Spec.CSI.VolumeHandle)
}

groupSnapshotContent := &crdv1alpha1.VolumeGroupSnapshotContent{
Expand All @@ -764,7 +764,7 @@ func (ctrl *csiSnapshotCommonController) createGroupSnapshotContent(groupSnapsho
Spec: crdv1alpha1.VolumeGroupSnapshotContentSpec{
VolumeGroupSnapshotRef: *snapshotRef,
Source: crdv1alpha1.VolumeGroupSnapshotContentSource{
PersistentVolumeNames: pvNames,
VolumeHandles: volumeHandles,
},
VolumeGroupSnapshotClassName: &(groupSnapshotClass.Name),
DeletionPolicy: groupSnapshotClass.DeletionPolicy,
Expand Down Expand Up @@ -846,9 +846,9 @@ func (ctrl *csiSnapshotCommonController) syncGroupSnapshotContent(groupSnapshotC
klog.V(5).Infof("syncGroupSnapshotContent[%s]: check if we should add invalid label on group snapshot content", groupSnapshotContent.Name)

// Keep this check in the controller since the validation webhook may not have been deployed.
if (groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle == nil && len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) == 0) ||
(groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil && len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) > 0) {
err := fmt.Errorf("Exactly one of VolumeGroupSnapshotHandle and PersistentVolumeNames should be specified")
if (groupSnapshotContent.Spec.Source.GroupSnapshotHandles == nil && len(groupSnapshotContent.Spec.Source.VolumeHandles) == 0) ||
(groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil && len(groupSnapshotContent.Spec.Source.VolumeHandles) > 0) {
err := fmt.Errorf("Exactly one of GroupSnapshotHandles and VolumeHandles should be specified")
klog.Errorf("syncGroupSnapshotContent[%s]: validation error, %s", groupSnapshotContent.Name, err.Error())
ctrl.eventRecorder.Event(groupSnapshotContent, v1.EventTypeWarning, "GroupContentValidationError", err.Error())
return err
Expand Down Expand Up @@ -1161,7 +1161,7 @@ func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimesta
// Delete the individual snapshots part of the group snapshot
for _, snapshot := range groupSnapshot.Status.VolumeSnapshotRefList {
err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Delete(context.TODO(), snapshot.Name, metav1.DeleteOptions{})
if err != nil {
if err != nil && !apierrs.IsNotFound(err) {
msg := fmt.Sprintf("failed to delete snapshot API object %s/%s part of group snapshot %s: %v", snapshot.Namespace, snapshot.Name, utils.GroupSnapshotKey(groupSnapshot), err)
klog.Error(msg)
ctrl.eventRecorder.Event(groupSnapshot, v1.EventTypeWarning, "SnapshotDeleteError", msg)
Expand Down
Loading