From 9e58d4cc61f9a9f17aa3e0c10877504dbb6583d8 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Thu, 25 Jan 2024 10:10:32 +0100 Subject: [PATCH] Selector in VolumeGroupSnapshotSource API should be optional --- client/apis/volumegroupsnapshot/v1alpha1/types.go | 4 ++-- .../v1alpha1/zz_generated.deepcopy.go | 7 ++++++- ...upsnapshot.storage.k8s.io_volumegroupsnapshots.yaml | 5 +---- .../groupsnapshot_controller_helper.go | 4 ++-- pkg/validation-webhook/groupsnapshot_test.go | 10 +++++----- .../v6/apis/volumegroupsnapshot/v1alpha1/types.go | 4 ++-- .../v1alpha1/zz_generated.deepcopy.go | 7 ++++++- 7 files changed, 24 insertions(+), 17 deletions(-) diff --git a/client/apis/volumegroupsnapshot/v1alpha1/types.go b/client/apis/volumegroupsnapshot/v1alpha1/types.go index 7e27b793..531867e6 100644 --- a/client/apis/volumegroupsnapshot/v1alpha1/types.go +++ b/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. diff --git a/client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go b/client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go index 5dc16cda..43a55b16 100644 --- a/client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go +++ b/client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go @@ -24,6 +24,7 @@ package v1alpha1 import ( v1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -337,7 +338,11 @@ func (in *VolumeGroupSnapshotList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolumeGroupSnapshotSource) DeepCopyInto(out *VolumeGroupSnapshotSource) { *out = *in - in.Selector.DeepCopyInto(&out.Selector) + if in.Selector != nil { + in, out := &in.Selector, &out.Selector + *out = new(metav1.LabelSelector) + (*in).DeepCopyInto(*out) + } if in.VolumeGroupSnapshotContentName != nil { in, out := &in.VolumeGroupSnapshotContentName, &out.VolumeGroupSnapshotContentName *out = new(string) diff --git a/client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshots.yaml b/client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshots.yaml index facfe7ff..b8bf6366 100644 --- a/client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshots.yaml +++ b/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 diff --git a/pkg/common-controller/groupsnapshot_controller_helper.go b/pkg/common-controller/groupsnapshot_controller_helper.go index 26cc44d6..1fe7f870 100644 --- a/pkg/common-controller/groupsnapshot_controller_helper.go +++ b/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()) diff --git a/pkg/validation-webhook/groupsnapshot_test.go b/pkg/validation-webhook/groupsnapshot_test.go index e1f00fe3..c555c079 100644 --- a/pkg/validation-webhook/groupsnapshot_test.go +++ b/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,7 +187,7 @@ func TestAdmitVolumeGroupSnapshotV1Alpha1(t *testing.T) { Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ VolumeGroupSnapshotContentName: &contentname, - Selector: selector, + Selector: &selector, }, }, }, @@ -195,7 +195,7 @@ func TestAdmitVolumeGroupSnapshotV1Alpha1(t *testing.T) { Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ VolumeGroupSnapshotContentName: &contentname, - Selector: selector, + Selector: &selector, }, }, }, diff --git a/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/types.go b/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/types.go index 7e27b793..531867e6 100644 --- a/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/types.go +++ b/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/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. diff --git a/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go b/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go index 5dc16cda..43a55b16 100644 --- a/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go +++ b/vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go @@ -24,6 +24,7 @@ package v1alpha1 import ( v1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -337,7 +338,11 @@ func (in *VolumeGroupSnapshotList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolumeGroupSnapshotSource) DeepCopyInto(out *VolumeGroupSnapshotSource) { *out = *in - in.Selector.DeepCopyInto(&out.Selector) + if in.Selector != nil { + in, out := &in.Selector, &out.Selector + *out = new(metav1.LabelSelector) + (*in).DeepCopyInto(*out) + } if in.VolumeGroupSnapshotContentName != nil { in, out := &in.VolumeGroupSnapshotContentName, &out.VolumeGroupSnapshotContentName *out = new(string)