Merge pull request #995 from nixpanic/VolumeGroupSnapshot/optional-se…

…lector

Selector in VolumeGroupSnapshotSource API should be optional

client/apis/volumegroupsnapshot/v1alpha1/types.go

@@ -52,8 +52,8 @@ type VolumeGroupSnapshotSource struct {
 	// is created, the existing group snapshots won't be modified.
 	// Once a VolumeGroupSnapshotContent is created and the sidecar starts to process
 	// it, the volume list will not change with retries.
-	// Required.
-	Selector metav1.LabelSelector `json:"selector" protobuf:"bytes,1,opt,name=selector"`
+	// +optional
+	Selector *metav1.LabelSelector `json:"selector,omitempty" protobuf:"bytes,1,opt,name=selector"`
 
 	// VolumeGroupSnapshotContentName specifies the name of a pre-existing VolumeGroupSnapshotContent
 	// object representing an existing volume group snapshot.

client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshots.yaml

@@ -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/995"
     controller-gen.kubebuilder.io/version: v0.12.0
   creationTimestamp: null
   name: volumegroupsnapshots.groupsnapshot.storage.k8s.io
@@ -78,7 +78,6 @@ spec:
                       is created, the existing group snapshots won't be modified.
                       Once a VolumeGroupSnapshotContent is created and the sidecar
                       starts to process it, the volume list will not change with retries.
-                      Required.
                     properties:
                       matchExpressions:
                         description: matchExpressions is a list of label selector
@@ -129,8 +128,6 @@ spec:
                       if the volume group snapshot already exists and only needs a
                       representation in Kubernetes. This field is immutable.
                     type: string
-                required:
-                - selector
                 type: object
               volumeGroupSnapshotClassName:
                 description: VolumeGroupSnapshotClassName is the name of the VolumeGroupSnapshotClass

pkg/common-controller/groupsnapshot_controller_helper.go

@@ -295,8 +295,8 @@ func (ctrl *csiSnapshotCommonController) syncGroupSnapshot(groupSnapshot *crdv1a
 	}
 	// Keep this check in the controller since the validation webhook may not have been deployed.
 	klog.V(5).Infof("syncGroupSnapshot[%s]: validate group snapshot to make sure source has been correctly specified", utils.GroupSnapshotKey(groupSnapshot))
-	if (&groupSnapshot.Spec.Source.Selector == nil && groupSnapshot.Spec.Source.VolumeGroupSnapshotContentName == nil) ||
-		(&groupSnapshot.Spec.Source.Selector != nil && groupSnapshot.Spec.Source.VolumeGroupSnapshotContentName != nil) {
+	if (groupSnapshot.Spec.Source.Selector == nil && groupSnapshot.Spec.Source.VolumeGroupSnapshotContentName == nil) ||
+		(groupSnapshot.Spec.Source.Selector != nil && groupSnapshot.Spec.Source.VolumeGroupSnapshotContentName != nil) {
 		err := fmt.Errorf("Exactly one of Selector and VolumeGroupSnapshotContentName should be specified")
 		klog.Errorf("syncGroupSnapshot[%s]: validation error, %s", utils.GroupSnapshotKey(groupSnapshot), err.Error())
 		ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotValidationError", err.Error())

pkg/validation-webhook/groupsnapshot_test.go

@@ -90,7 +90,7 @@ func TestAdmitVolumeGroupSnapshotV1Alpha1(t *testing.T) {
 			volumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{
 				Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{
 					Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{
-						Selector: selector,
+						Selector: &selector,
 					},
 				},
 			},
@@ -172,13 +172,13 @@ func TestAdmitVolumeGroupSnapshotV1Alpha1(t *testing.T) {
 				Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{
 					Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{
 						VolumeGroupSnapshotContentName: &contentname,
-						Selector:                       selector,
+						Selector:                       &selector,
 					},
 				},
 			},
 			shouldAdmit: false,
 			operation:   v1.Update,
-			msg:         fmt.Sprintf("Spec.Source.Selector is immutable but was changed from %v to %v", selector, metav1.LabelSelector{}),
+			msg:         fmt.Sprintf("Spec.Source.Selector is immutable but was changed from %v to %v", &selector, "nil"),
 		},
 		{
 			// will be handled by schema validation
@@ -187,15 +187,15 @@ func TestAdmitVolumeGroupSnapshotV1Alpha1(t *testing.T) {
 				Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{
 					Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{
 						VolumeGroupSnapshotContentName: &contentname,
-						Selector:                       selector,
+						Selector:                       &selector,
 					},
 				},
 			},
 			oldVolumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{
 				Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{
 					Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{
 						VolumeGroupSnapshotContentName: &contentname,
-						Selector:                       selector,
+						Selector:                       &selector,
 					},
 				},
 			},