Merge pull request #1337 from leonardoce/dev-api-review

Deprecate VolumeGroupSnapshot v1beta1, address API review comments

Author: k8s-ci-robot

client/apis/volumegroupsnapshot/v1beta1/types.go

@@ -119,6 +119,7 @@ type VolumeGroupSnapshotStatus struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:resource:scope=Namespaced,shortName=vgs
 // +kubebuilder:subresource:status
+// +kubebuilder:deprecatedversion
 // +kubebuilder:printcolumn:name="ReadyToUse",type=boolean,JSONPath=`.status.readyToUse`,description="Indicates if all the individual snapshots in the group are ready to be used to restore a group of volumes."
 // +kubebuilder:printcolumn:name="VolumeGroupSnapshotClass",type=string,JSONPath=`.spec.volumeGroupSnapshotClassName`,description="The name of the VolumeGroupSnapshotClass requested by the VolumeGroupSnapshot."
 // +kubebuilder:printcolumn:name="VolumeGroupSnapshotContent",type=string,JSONPath=`.status.boundVolumeGroupSnapshotContentName`,description="Name of the VolumeGroupSnapshotContent object to which the VolumeGroupSnapshot object intends to bind to. Please note that verification of binding actually requires checking both VolumeGroupSnapshot and VolumeGroupSnapshotContent to ensure both are pointing at each other. Binding MUST be verified prior to usage of this object."
@@ -162,6 +163,7 @@ type VolumeGroupSnapshotList struct {
 // is used by specifying its name in a VolumeGroupSnapshot object.
 // VolumeGroupSnapshotClasses are non-namespaced.
 // +kubebuilder:object:root=true
+// +kubebuilder:deprecatedversion
 // +kubebuilder:resource:scope=Cluster,shortName=vgsclass;vgsclasses
 // +kubebuilder:printcolumn:name="Driver",type=string,JSONPath=`.driver`
 // +kubebuilder:printcolumn:name="DeletionPolicy",type=string,JSONPath=`.deletionPolicy`,description="Determines whether a VolumeGroupSnapshotContent created through the VolumeGroupSnapshotClass should be deleted when its bound VolumeGroupSnapshot is deleted."
@@ -218,6 +220,7 @@ type VolumeGroupSnapshotClassList struct {
 // in the underlying storage system
 // +kubebuilder:object:root=true
 // +kubebuilder:resource:scope=Cluster,shortName=vgsc;vgscs
+// +kubebuilder:deprecatedversion
 // +kubebuilder:subresource:status
 // +kubebuilder:printcolumn:name="ReadyToUse",type=boolean,JSONPath=`.status.readyToUse`,description="Indicates if all the individual snapshots in the group are ready to be used to restore a group of volumes."
 // +kubebuilder:printcolumn:name="DeletionPolicy",type=string,JSONPath=`.spec.deletionPolicy`,description="Determines whether this VolumeGroupSnapshotContent and its physical group snapshot on the underlying storage system should be deleted when its bound VolumeGroupSnapshot is deleted."

client/apis/volumegroupsnapshot/v1beta2/types.go

@@ -80,16 +80,14 @@ type VolumeGroupSnapshotStatus struct {
 	// VolumeGroupSnapshot and VolumeGroupSnapshotContent objects is successful
 	// (by validating that both VolumeGroupSnapshot and VolumeGroupSnapshotContent
 	// point at each other) before using this object.
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="boundVolumeGroupSnapshotContentName is immutable once set"
 	// +optional
 	BoundVolumeGroupSnapshotContentName *string `json:"boundVolumeGroupSnapshotContentName,omitempty" protobuf:"bytes,1,opt,name=boundVolumeGroupSnapshotContentName"`
 
 	// CreationTime is the timestamp when the point-in-time group snapshot is taken
 	// by the underlying storage system.
 	// If not specified, it may indicate that the creation time of the group snapshot
 	// is unknown.
-	// The format of this field is a Unix nanoseconds time encoded as an int64.
-	// On Unix, the command date +%s%N returns the current time in nanoseconds
-	// since 1970-01-01 00:00:00 UTC.
 	// This field is updated based on the CreationTime field in VolumeGroupSnapshotContentStatus
 	// +optional
 	CreationTime *metav1.Time `json:"creationTime,omitempty" protobuf:"bytes,2,opt,name=creationTime"`
@@ -177,11 +175,13 @@ type VolumeGroupSnapshotClass struct {
 
 	// Driver is the name of the storage driver expected to handle this VolumeGroupSnapshotClass.
 	// Required.
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="driver is immutable once set"
 	Driver string `json:"driver" protobuf:"bytes,2,opt,name=driver"`
 
 	// Parameters is a key-value map with storage driver specific parameters for
 	// creating group snapshots.
 	// These values are opaque to Kubernetes and are passed directly to the driver.
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="parameters are immutable once set"
 	// +optional
 	Parameters map[string]string `json:"parameters,omitempty" protobuf:"bytes,3,rep,name=parameters"`
 
@@ -194,6 +194,7 @@ type VolumeGroupSnapshotClass struct {
 	// "Delete" means that the VolumeGroupSnapshotContent and its physical group
 	// snapshot on underlying storage system are deleted.
 	// Required.
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="deletionPolicy is immutable once set"
 	DeletionPolicy snapshotv1.DeletionPolicy `json:"deletionPolicy" protobuf:"bytes,4,opt,name=deletionPolicy"`
 }
 
@@ -270,6 +271,8 @@ type VolumeGroupSnapshotContentSpec struct {
 	// This field is immutable after creation.
 	// Required.
 	// +kubebuilder:validation:XValidation:rule="has(self.name) && has(self.__namespace__)",message="both volumeGroupSnapshotRef.name and volumeGroupSnapshotRef.namespace must be set"
+	// +kubebuilder:validation:XValidation:rule="self.name == oldSelf.name && self.__namespace__ == oldSelf.__namespace__",message="volumeGroupSnapshotRef.name and volumeGroupSnapshotRef.namespace are immutable"
+	// +kubebuilder:validation:XValidation:rule="!has(oldSelf.uid) || (has(self.uid) && self.uid == oldSelf.uid)",message="volumeGroupSnapshotRef.uid is immutable once set"
 	VolumeGroupSnapshotRef core_v1.ObjectReference `json:"volumeGroupSnapshotRef" protobuf:"bytes,1,opt,name=volumeGroupSnapshotRef"`
 
 	// DeletionPolicy determines whether this VolumeGroupSnapshotContent and the
@@ -293,6 +296,7 @@ type VolumeGroupSnapshotContentSpec struct {
 	// This MUST be the same as the name returned by the CSI GetPluginName() call for
 	// that driver.
 	// Required.
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="driver is immutable once set"
 	Driver string `json:"driver" protobuf:"bytes,3,opt,name=driver"`
 
 	// VolumeGroupSnapshotClassName is the name of the VolumeGroupSnapshotClass from
@@ -303,6 +307,7 @@ type VolumeGroupSnapshotContentSpec struct {
 	// For dynamic provisioning, this field must be set.
 	// This field may be unset for pre-provisioned snapshots.
 	// +optional
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="volumeGroupSnapshotClassName is immutable once set"
 	VolumeGroupSnapshotClassName *string `json:"volumeGroupSnapshotClassName,omitempty" protobuf:"bytes,4,opt,name=volumeGroupSnapshotClassName"`
 
 	// Source specifies whether the snapshot is (or should be) dynamically provisioned
@@ -344,15 +349,13 @@ type VolumeGroupSnapshotContentStatus struct {
 	// If a storage system does not provide such an id, the
 	// CSI driver can choose to return the VolumeGroupSnapshot name.
 	// +optional
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="volumeGroupSnapshotHandle is immutable once set"
 	VolumeGroupSnapshotHandle *string `json:"volumeGroupSnapshotHandle,omitempty" protobuf:"bytes,1,opt,name=volumeGroupSnapshotHandle"`
 
 	// CreationTime is the timestamp when the point-in-time group snapshot is taken
 	// by the underlying storage system.
 	// If not specified, it indicates the creation time is unknown.
 	// If not specified, it means the readiness of a group snapshot is unknown.
-	// The format of this field is a Unix nanoseconds time encoded as an int64.
-	// On Unix, the command date +%s%N returns the current time in nanoseconds
-	// since 1970-01-01 00:00:00 UTC.
 	// This field is the source for the CreationTime field in VolumeGroupSnapshotStatus
 	// +optional
 	CreationTime *metav1.Time `json:"creationTime,omitempty" protobuf:"bytes,2,opt,name=creationTime"`

client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotclasses.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/1150"
+    api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/1337"
     controller-gen.kubebuilder.io/version: v0.15.0
   name: volumegroupsnapshotclasses.groupsnapshot.storage.k8s.io
 spec:
@@ -31,6 +31,7 @@ spec:
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date
+    deprecated: true
     name: v1beta1
     schema:
       openAPIV3Schema:
@@ -136,11 +137,17 @@ spec:
             - Delete
             - Retain
             type: string
+            x-kubernetes-validations:
+            - message: deletionPolicy is immutable once set
+              rule: self == oldSelf
           driver:
             description: |-
               Driver is the name of the storage driver expected to handle this VolumeGroupSnapshotClass.
               Required.
             type: string
+            x-kubernetes-validations:
+            - message: driver is immutable once set
+              rule: self == oldSelf
           kind:
             description: |-
               Kind is a string value representing the REST resource this object represents.
@@ -159,6 +166,9 @@ spec:
               creating group snapshots.
               These values are opaque to Kubernetes and are passed directly to the driver.
             type: object
+            x-kubernetes-validations:
+            - message: parameters are immutable once set
+              rule: self == oldSelf
         required:
         - deletionPolicy
         - driver

client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.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/1150"
+    api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/1337"
     controller-gen.kubebuilder.io/version: v0.15.0
   name: volumegroupsnapshotcontents.groupsnapshot.storage.k8s.io
 spec:
@@ -62,6 +62,7 @@ spec:
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date
+    deprecated: true
     name: v1beta1
     schema:
       openAPIV3Schema:
@@ -423,6 +424,9 @@ spec:
                   that driver.
                   Required.
                 type: string
+                x-kubernetes-validations:
+                - message: driver is immutable once set
+                  rule: self == oldSelf
               source:
                 description: |-
                   Source specifies whether the snapshot is (or should be) dynamically provisioned
@@ -494,6 +498,9 @@ spec:
                   For dynamic provisioning, this field must be set.
                   This field may be unset for pre-provisioned snapshots.
                 type: string
+                x-kubernetes-validations:
+                - message: volumeGroupSnapshotClassName is immutable once set
+                  rule: self == oldSelf
               volumeGroupSnapshotRef:
                 description: |-
                   VolumeGroupSnapshotRef specifies the VolumeGroupSnapshot object to which this
@@ -550,6 +557,11 @@ spec:
                 - message: both volumeGroupSnapshotRef.name and volumeGroupSnapshotRef.namespace
                     must be set
                   rule: has(self.name) && has(self.__namespace__)
+                - message: volumeGroupSnapshotRef.name and volumeGroupSnapshotRef.namespace
+                    are immutable
+                  rule: self.name == oldSelf.name && self.__namespace__ == oldSelf.__namespace__
+                - message: volumeGroupSnapshotRef.uid is immutable once set
+                  rule: '!has(oldSelf.uid) || (has(self.uid) && self.uid == oldSelf.uid)'
             required:
             - deletionPolicy
             - driver
@@ -565,9 +577,6 @@ spec:
                   by the underlying storage system.
                   If not specified, it indicates the creation time is unknown.
                   If not specified, it means the readiness of a group snapshot is unknown.
-                  The format of this field is a Unix nanoseconds time encoded as an int64.
-                  On Unix, the command date +%s%N returns the current time in nanoseconds
-                  since 1970-01-01 00:00:00 UTC.
                   This field is the source for the CreationTime field in VolumeGroupSnapshotStatus
                 format: date-time
                 type: string
@@ -601,6 +610,9 @@ spec:
                   If a storage system does not provide such an id, the
                   CSI driver can choose to return the VolumeGroupSnapshot name.
                 type: string
+                x-kubernetes-validations:
+                - message: volumeGroupSnapshotHandle is immutable once set
+                  rule: self == oldSelf
               volumeSnapshotInfoList:
                 description: |-
                   This field is introduced in v1beta2
@@ -623,19 +635,19 @@ spec:
                         be used to restore a volume.
                       type: boolean
                     restoreSize:
-                      description: RestoreSize represents the complete size of the
-                        snapshot in bytes.
+                      description: |-
+                        RestoreSize represents the minimum size of volume required to create a volume
+                        from this snapshot.
                       format: int64
                       type: integer
                     snapshotHandle:
-                      description: |-
-                        SnapshotHandle is a unique id returned by the CSI driver to identify a volume
-                        snapshot on the storage system
+                      description: SnapshotHandle is the CSI "snapshot_id" of this
+                        snapshot on the underlying storage system.
                       type: string
                     volumeHandle:
                       description: |-
-                        VolumeHandle is a unique id returned by the CSI driver to identify a volume
-                        on the storage system
+                        VolumeHandle specifies the CSI "volume_id" of the volume from which this snapshot
+                        was taken from.
                       type: string
                   type: object
                 type: array

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/1150"
+    api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/1337"
     controller-gen.kubebuilder.io/version: v0.15.0
   name: volumegroupsnapshots.groupsnapshot.storage.k8s.io
 spec:
@@ -43,6 +43,7 @@ spec:
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date
+    deprecated: true
     name: v1beta1
     schema:
       openAPIV3Schema:
@@ -409,15 +410,15 @@ spec:
                   (by validating that both VolumeGroupSnapshot and VolumeGroupSnapshotContent
                   point at each other) before using this object.
                 type: string
+                x-kubernetes-validations:
+                - message: boundVolumeGroupSnapshotContentName is immutable once set
+                  rule: self == oldSelf
               creationTime:
                 description: |-
                   CreationTime is the timestamp when the point-in-time group snapshot is taken
                   by the underlying storage system.
                   If not specified, it may indicate that the creation time of the group snapshot
                   is unknown.
-                  The format of this field is a Unix nanoseconds time encoded as an int64.
-                  On Unix, the command date +%s%N returns the current time in nanoseconds
-                  since 1970-01-01 00:00:00 UTC.
                   This field is updated based on the CreationTime field in VolumeGroupSnapshotContentStatus
                 format: date-time
                 type: string

client/hack/README.md

@@ -121,18 +121,9 @@ are correctly working.
 ```
   ./hack/run-cel-tests.sh
 
-  cel-tests/volumegroupsnapshotcontent/vgsc-change-ref-namespace.post.yaml: SUCCESS
-  cel-tests/volumegroupsnapshotcontent/vgsc-source-volume-to-groupsnapshot.post.yaml: SUCCESS
-  cel-tests/volumegroupsnapshotcontent/vgsc-source-empty.yaml: SUCCESS (expected failure)
-  cel-tests/volumegroupsnapshotcontent/vgsc-change-ref-namespace.pre.yaml: SUCCESS
-  cel-tests/volumegroupsnapshotcontent/vgsc-ref-only-name.yaml: SUCCESS (expected failure)
-  [...]
-  cel-tests/volumegroupsnapshotcontent/vgsc-change-ref-namespace.pre.yaml -> cel-tests/volumegroupsnapshotcontent/vgsc-change-ref-namespace.post.yaml: SUCCESS (expected failure)
-  cel-tests/volumegroupsnapshotcontent/vgsc-source-volume-immutable.pre.yaml -> cel-tests/volumegroupsnapshotcontent/vgsc-source-volume-immutable.post.yaml: SUCCESS (expected failure)
-  cel-tests/volumegroupsnapshotcontent/vgsc-source-volume-to-groupsnapshot.pre.yaml -> cel-tests/volumegroupsnapshotcontent/vgsc-source-volume-to-groupsnapshot.post.yaml: SUCCESS (expected failure)
-  cel-tests/volumegroupsnapshotcontent/vgsc-source-groupsnapshot-immutable.pre.yaml -> cel-tests/volumegroupsnapshotcontent/vgsc-source-groupsnapshot-immutable.post.yaml: SUCCESS (expected failure)
-  [...]
-
-  SUCCESS: 90
+  SUCCESS: 108
   FAILURES: 0
 ```
+
+Adding the `-v` option to `run-cel-tests.sh` will show the output of each
+test case.

client/hack/cel-tests/volumegroupsnapshotclass/vgs-class-deletionpolicy-immutable.post.yaml

@@ -0,0 +1,9 @@
+---
+apiVersion: groupsnapshot.storage.k8s.io/v1beta2
+kind: VolumeGroupSnapshotClass
+metadata:
+  name: csi-hostpath-groupsnapclass
+driver: hostpath.csi.k8s.io
+deletionPolicy: Retain
+parameters:
+  one: two

client/hack/cel-tests/volumegroupsnapshotclass/vgs-class-deletionpolicy-immutable.post.yaml.tx_err

@@ -0,0 +1 @@
+deletionPolicy: Invalid value: "string": deletionPolicy is immutable once set

client/hack/cel-tests/volumegroupsnapshotclass/vgs-class-deletionpolicy-immutable.pre.yaml

@@ -0,0 +1,9 @@
+---
+apiVersion: groupsnapshot.storage.k8s.io/v1beta2
+kind: VolumeGroupSnapshotClass
+metadata:
+  name: csi-hostpath-groupsnapclass
+driver: hostpath.csi.k8s.io
+deletionPolicy: Delete
+parameters:
+  one: two

client/hack/cel-tests/volumegroupsnapshotclass/vgs-class-parameters-immutable.post.yaml

@@ -0,0 +1,9 @@
+---
+apiVersion: groupsnapshot.storage.k8s.io/v1beta2
+kind: VolumeGroupSnapshotClass
+metadata:
+  name: csi-hostpath-groupsnapclass
+driver: hostpath.csi.k8s.io
+deletionPolicy: Delete
+parameters:
+  one: three