Skip to content

Commit 01d5449

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #971 from RaunakShah/deletegroupsnapshot-part-3
Update API for pre provisioned group snapshots
2 parents 3e32435 + e6dc65f commit 01d5449

File tree

14 files changed

+219
-117
lines changed

14 files changed

+219
-117
lines changed

‎client/apis/volumegroupsnapshot/v1alpha1/types.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -348,16 +348,33 @@ type VolumeGroupSnapshotContentStatus struct {
348348
// Exactly one of its members must be set.
349349
// Members in VolumeGroupSnapshotContentSource are immutable.
350350
type VolumeGroupSnapshotContentSource struct {
351-
// PersistentVolumeNames is a list of names of PersistentVolumes to be snapshotted
351+
// VolumeHandles is a list of volume handles on the backend to be snapshotted
352352
// together. It is specified for dynamic provisioning of the VolumeGroupSnapshot.
353353
// This field is immutable.
354354
// +optional
355-
PersistentVolumeNames []string `json:"persistentVolumeNames,omitempty" protobuf:"bytes,1,opt,name=persistentVolumeNames"`
355+
VolumeHandles []string `json:"volumeHandles,omitempty" protobuf:"bytes,1,opt,name=volumeHandles"`
356356

357+
// GroupSnapshotHandles specifies the CSI "group_snapshot_id" of a pre-existing
358+
// group snapshot and a list of CSI "snapshot_id" of pre-existing snapshots
359+
// on the underlying storage system for which a Kubernetes object
360+
// representation was (or should be) created.
361+
// This field is immutable.
362+
// +optional
363+
GroupSnapshotHandles *GroupSnapshotHandles `json:"groupSnapshotHandles,omitempty" protobuf:"bytes,2,opt,name=groupSnapshotHandles"`
364+
}
365+
366+
type GroupSnapshotHandles struct {
357367
// VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id" of a pre-existing
358368
// group snapshot on the underlying storage system for which a Kubernetes object
359369
// representation was (or should be) created.
360370
// This field is immutable.
361-
// +optional
362-
VolumeGroupSnapshotHandle *string `json:"volumeGroupSnapshotHandle,omitempty" protobuf:"bytes,2,opt,name=volumeGroupSnapshotHandle"`
371+
// Required.
372+
VolumeGroupSnapshotHandle string `json:"volumeGroupSnapshotHandle" protobuf:"bytes,1,opt,name=volumeGroupSnapshotHandle"`
373+
374+
// VolumeSnapshotHandles is a list of CSI "snapshot_id" of pre-existing
375+
// snapshots on the underlying storage system for which Kubernetes objects
376+
// representation were (or should be) created.
377+
// This field is immutable.
378+
// Required.
379+
VolumeSnapshotHandles []string `json:"volumeSnapshotHandles" protobuf:"bytes,2,opt,name=volumeSnapshotHandles"`
363380
}

‎client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go

Lines changed: 28 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎client/apis/volumesnapshot/v1/zz_generated.deepcopy.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
33
kind: CustomResourceDefinition
44
metadata:
55
annotations:
6-
api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/814"
6+
api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/971"
77
controller-gen.kubebuilder.io/version: v0.12.0
88
creationTimestamp: null
99
name: volumegroupsnapshotcontents.groupsnapshot.storage.k8s.io
@@ -104,23 +104,42 @@ spec:
104104
dynamically provisioned or already exists, and just requires a Kubernetes
105105
object representation. This field is immutable after creation. Required.
106106
properties:
107-
persistentVolumeNames:
108-
description: PersistentVolumeNames is a list of names of PersistentVolumes
109-
to be snapshotted together. It is specified for dynamic provisioning
110-
of the VolumeGroupSnapshot. This field is immutable.
107+
groupSnapshotHandles:
108+
description: GroupSnapshotHandles specifies the CSI "group_snapshot_id"
109+
of a pre-existing group snapshot and a list of CSI "snapshot_id"
110+
of pre-existing snapshots on the underlying storage system for
111+
which a Kubernetes object representation was (or should be)
112+
created. This field is immutable.
113+
properties:
114+
volumeGroupSnapshotHandle:
115+
description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
116+
of a pre-existing group snapshot on the underlying storage
117+
system for which a Kubernetes object representation was
118+
(or should be) created. This field is immutable. Required.
119+
type: string
120+
volumeSnapshotHandles:
121+
description: VolumeSnapshotHandles is a list of CSI "snapshot_id"
122+
of pre-existing snapshots on the underlying storage system
123+
for which Kubernetes objects representation were (or should
124+
be) created. This field is immutable. Required.
125+
items:
126+
type: string
127+
type: array
128+
required:
129+
- volumeGroupSnapshotHandle
130+
- volumeSnapshotHandles
131+
type: object
132+
volumeHandles:
133+
description: VolumeHandles is a list of volume handles on the
134+
backend to be snapshotted together. It is specified for dynamic
135+
provisioning of the VolumeGroupSnapshot. This field is immutable.
111136
items:
112137
type: string
113138
type: array
114-
volumeGroupSnapshotHandle:
115-
description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
116-
of a pre-existing group snapshot on the underlying storage system
117-
for which a Kubernetes object representation was (or should
118-
be) created. This field is immutable.
119-
type: string
120139
type: object
121140
oneOf:
122-
- required: ["persistentVolumeNames"]
123-
- required: ["volumeGroupSnapshotHandle"]
141+
- required: ["volumeHandles"]
142+
- required: ["groupSnapshotHandles"]
124143
volumeGroupSnapshotClassName:
125144
description: VolumeGroupSnapshotClassName is the name of the VolumeGroupSnapshotClass
126145
from which this group snapshot was (or will be) created. Note that

‎client/hack/README.md

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -144,30 +144,49 @@ Update the restoreSize property to use type string only:
144144
145145
* Add the VolumeSnapshot namespace to the `additionalPrinterColumns` section. Refer https://github.com/kubernetes-csi/external-snapshotter/pull/535 for more details.
146146
147-
* 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`.
147+
* 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`.
148148
149149
```bash
150150
source:
151151
description: Source specifies whether the snapshot is (or should be)
152152
dynamically provisioned or already exists, and just requires a Kubernetes
153153
object representation. This field is immutable after creation. Required.
154154
properties:
155-
persistentVolumeNames:
156-
description: PersistentVolumeNames is a list of names of PersistentVolumes
157-
to be snapshotted together. It is specified for dynamic provisioning
158-
of the VolumeGroupSnapshot. This field is immutable.
155+
groupSnapshotHandles:
156+
description: GroupSnapshotHandles specifies the CSI "group_snapshot_id"
157+
of a pre-existing group snapshot and a list of CSI "snapshot_id"
158+
of pre-existing snapshots on the underlying storage system for
159+
which a Kubernetes object representation was (or should be)
160+
created. This field is immutable.
161+
properties:
162+
volumeGroupSnapshotHandle:
163+
description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
164+
of a pre-existing group snapshot on the underlying storage
165+
system for which a Kubernetes object representation was
166+
(or should be) created. This field is immutable. Required.
167+
type: string
168+
volumeSnapshotHandles:
169+
description: VolumeSnapshotHandles is a list of CSI "snapshot_id"
170+
of pre-existing snapshots on the underlying storage system
171+
for which Kubernetes objects representation were (or should
172+
be) created. This field is immutable. Required.
173+
items:
174+
type: string
175+
type: array
176+
required:
177+
- volumeGroupSnapshotHandle
178+
- volumeSnapshotHandles
179+
type: object
180+
volumeHandles:
181+
description: VolumeHandles is a list of volume handles on the
182+
backend to be snapshotted together. It is specified for dynamic
183+
provisioning of the VolumeGroupSnapshot. This field is immutable.
159184
items:
160185
type: string
161186
type: array
162-
volumeGroupSnapshotHandle:
163-
description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
164-
of a pre-existing group snapshot on the underlying storage system
165-
for which a Kubernetes object representation was (or should
166-
be) created. This field is immutable.
167-
type: string
168187
type: object
169188
oneOf:
170-
- required: ["persistentVolumeNames"]
171-
- required: ["volumeGroupSnapshotHandle"]
189+
- required: ["volumeHandles"]
190+
- required: ["groupSnapshotHandles"]
172191
volumeGroupSnapshotClassName:
173192
```

‎pkg/common-controller/groupsnapshot_controller_helper.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ func (ctrl *csiSnapshotCommonController) syncUnreadyGroupSnapshot(groupSnapshot
426426

427427
if contentObj != nil {
428428
klog.V(5).Infof("Found VolumeGroupSnapshotContent object %s for group snapshot %s", contentObj.Name, uniqueGroupSnapshotName)
429-
if contentObj.Spec.Source.VolumeGroupSnapshotHandle != nil {
429+
if contentObj.Spec.Source.GroupSnapshotHandles != nil {
430430
ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotHandleSet", fmt.Sprintf("GroupSnapshot handle should not be set in group snapshot content %s for dynamic provisioning", uniqueGroupSnapshotName))
431431
return fmt.Errorf("VolumeGroupSnapshotHandle should not be set in the group snapshot content for dynamic provisioning for group snapshot %s", uniqueGroupSnapshotName)
432432
}
@@ -483,7 +483,7 @@ func (ctrl *csiSnapshotCommonController) getPreprovisionedGroupSnapshotContentFr
483483
return nil, nil
484484
}
485485
// check whether the content is a pre-provisioned VolumeGroupSnapshotContent
486-
if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle == nil {
486+
if groupSnapshotContent.Spec.Source.GroupSnapshotHandles == nil {
487487
// found a group snapshot content which represents a dynamically provisioned group snapshot
488488
// update the group snapshot and return an error
489489
ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotContentMismatch", "VolumeGroupSnapshotContent is dynamically provisioned while expecting a pre-provisioned one")
@@ -682,7 +682,7 @@ func (ctrl *csiSnapshotCommonController) getDynamicallyProvisionedGroupContentFr
682682
return nil, nil
683683
}
684684
// check whether the group snapshot content represents a dynamically provisioned snapshot
685-
if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil {
685+
if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil {
686686
ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotContentMismatch", "VolumeGroupSnapshotContent "+contentName+" is pre-provisioned while expecting a dynamically provisioned one")
687687
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)
688688
return nil, fmt.Errorf("group snapshot %s expects a dynamically provisioned VolumeGroupSnapshotContent %s but gets a pre-provisioned one", utils.GroupSnapshotKey(groupSnapshot), contentName)
@@ -752,9 +752,9 @@ func (ctrl *csiSnapshotCommonController) createGroupSnapshotContent(groupSnapsho
752752
if err != nil {
753753
return nil, err
754754
}
755-
var pvNames []string
755+
var volumeHandles []string
756756
for _, pv := range volumes {
757-
pvNames = append(pvNames, pv.Name)
757+
volumeHandles = append(volumeHandles, pv.Spec.CSI.VolumeHandle)
758758
}
759759

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

848848
// Keep this check in the controller since the validation webhook may not have been deployed.
849-
if (groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle == nil && len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) == 0) ||
850-
(groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil && len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) > 0) {
851-
err := fmt.Errorf("Exactly one of VolumeGroupSnapshotHandle and PersistentVolumeNames should be specified")
849+
if (groupSnapshotContent.Spec.Source.GroupSnapshotHandles == nil && len(groupSnapshotContent.Spec.Source.VolumeHandles) == 0) ||
850+
(groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil && len(groupSnapshotContent.Spec.Source.VolumeHandles) > 0) {
851+
err := fmt.Errorf("Exactly one of GroupSnapshotHandles and VolumeHandles should be specified")
852852
klog.Errorf("syncGroupSnapshotContent[%s]: validation error, %s", groupSnapshotContent.Name, err.Error())
853853
ctrl.eventRecorder.Event(groupSnapshotContent, v1.EventTypeWarning, "GroupContentValidationError", err.Error())
854854
return err
@@ -1161,7 +1161,7 @@ func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimesta
11611161
// Delete the individual snapshots part of the group snapshot
11621162
for _, snapshot := range groupSnapshot.Status.VolumeSnapshotRefList {
11631163
err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Delete(context.TODO(), snapshot.Name, metav1.DeleteOptions{})
1164-
if err != nil {
1164+
if err != nil && !apierrs.IsNotFound(err) {
11651165
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)
11661166
klog.Error(msg)
11671167
ctrl.eventRecorder.Event(groupSnapshot, v1.EventTypeWarning, "SnapshotDeleteError", msg)

0 commit comments

Comments
 (0)