Merge pull request #68159 from saad-ali/csiClusterRegFix

Automatic merge from submit-queue (batch tested with PRs 66840, 68159). If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

CSI Cluster Registry and Node Info CRDs Improvements

**What this PR does / why we need it**:
https://github.com/kubernetes/kubernetes/pull/67803 merged before I could address @lavalamp's feedback. This PR addresses his feedback

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Follow up on PR https://github.com/kubernetes/kubernetes/pull/67803

**Special notes for your reviewer**:

**Release note**:

```release-note

```

/assign @lavalamp 
/assign @thockin 

CC @jsafrane @vladimirvivien @verult @gnufied @childsb

pkg/controller/volume/attachdetach/attach_detach_controller.go

@@ -148,7 +148,7 @@ func NewAttachDetachController(
 	}
 
 	// Install required CSI CRDs on API server
-	if utilfeature.DefaultFeatureGate.Enabled(features.CSICrdAutoInstall) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.CSICRDAutoInstall) {
 		adc.installCRDs()
 	}
 
@@ -685,15 +685,16 @@ func (adc *attachDetachController) installCRDs() error {
 	}
 	res, err := adc.crdClient.ApiextensionsV1beta1().CustomResourceDefinitions().Create(crd)
 
-	if err != nil && !apierrors.IsAlreadyExists(err) {
+	if err == nil {
-		glog.Errorf("failed to create CSIDrivers CRD: %#v, err: %#v",
+		glog.Infof("CSIDrivers CRD created successfully: %#v",
-			res, err)
+			res)
 	} else if apierrors.IsAlreadyExists(err) {
 		glog.Warningf("CSIDrivers CRD already exists: %#v, err: %#v",
 			res, err)
 	} else {
-		glog.Infof("CSIDrivers CRD created successfully: %#v",
+		glog.Errorf("failed to create CSIDrivers CRD: %#v, err: %#v",
-			res)
+			res, err)
+		return err
 	}
 
 	crd = &apiextensionsv1beta1.CustomResourceDefinition{
@@ -712,15 +713,16 @@ func (adc *attachDetachController) installCRDs() error {
 	}
 	res, err = adc.crdClient.ApiextensionsV1beta1().CustomResourceDefinitions().Create(crd)
 
-	if err != nil && !apierrors.IsAlreadyExists(err) {
+	if err == nil {
-		glog.Errorf("failed to create CSINodeInfo CRD: %#v, err: %#v",
+		glog.Infof("CSINodeInfo CRD created successfully: %#v",
-			res, err)
+			res)
 	} else if apierrors.IsAlreadyExists(err) {
 		glog.Warningf("CSINodeInfo CRD already exists: %#v, err: %#v",
 			res, err)
 	} else {
-		glog.Infof("CSINodeInfo CRD created successfully: %#v",
+		glog.Errorf("failed to create CSINodeInfo CRD: %#v, err: %#v",
-			res)
+			res, err)
+		return err
 	}
 
 	return nil

pkg/features/kube_features.go

@@ -207,9 +207,8 @@ const (
 
 	// owner: @saad-ali
 	// alpha: v1.12
-	//
 	// Enable automatic installation of CRD for csi.storage.k8s.io API objects.
-	CSICrdAutoInstall utilfeature.Feature = "CSICrdAutoInstall"
+	CSICRDAutoInstall utilfeature.Feature = "CSICRDAutoInstall"
 
 	// owner @MrHohn
 	// beta: v1.10
@@ -425,7 +424,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
 	MountContainers:                             {Default: false, PreRelease: utilfeature.Alpha},
 	VolumeScheduling:                            {Default: true, PreRelease: utilfeature.Beta},
 	CSIPersistentVolume:                         {Default: true, PreRelease: utilfeature.Beta},
-	CSICrdAutoInstall:                           {Default: false, PreRelease: utilfeature.Alpha},
+	CSICRDAutoInstall:                           {Default: false, PreRelease: utilfeature.Alpha},
 	CustomPodDNS:                                {Default: true, PreRelease: utilfeature.Beta},
 	BlockVolume:                                 {Default: false, PreRelease: utilfeature.Alpha},
 	StorageObjectInUseProtection:                {Default: true, PreRelease: utilfeature.GA},

staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/doc.go

@@ -14,8 +14,9 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// Package v1alpha1 provides alpha API for CSI API objects.
 // +k8s:deepcopy-gen=package,register
 // +groupName=csi.storage.k8s.io
 // +k8s:openapi-gen=true
+
+// Package v1alpha1 provides alpha API for CSI API objects.
 package v1alpha1 // import "k8s.io/csi-api/pkg/apis/csi/v1alpha1"

staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go

@@ -50,29 +50,34 @@ type CSIDriverList struct {
 	// +optional
 	metav1.ListMeta `json:"metadata,omitempty"`
 
-	// Items is the list of CSIDriver
+	// items is the list of CSIDriver
 	Items []CSIDriver `json:"items"`
 }
 
 // CSIDriverSpec is the specification of a CSIDriver.
 type CSIDriverSpec struct {
-	// Indicates this CSI volume driver requires an attach operation (because it
+	// attachRequired indicates this CSI volume driver requires an attach
-	// implements the CSI ControllerPublishVolume() method), and that Kubernetes
+	// operation (because it implements the CSI ControllerPublishVolume()
-	// should call attach and wait for any attach operation to complete before
+	// method), and that Kubernetes should call attach and wait for any attach
-	// proceeding to mounting.
+	// operation to complete before proceeding to mounting.
 	// If value is not specified, default is false -- meaning attach will not be
 	// called.
 	// +optional
 	AttachRequired *bool `json:"attachRequired"`
 
-	// Indicates this CSI volume driver requires additional pod information
+	// If specified, podInfoRequiredOnMount indicates this CSI volume driver
-	// (like podName, podUID, etc.) during mount operations.
+	// requires additional pod information (like podName, podUID, etc.) during
-	// If this is set to true, Kubelet will pass pod information as
+	// mount operations.
+	// If value is not specified, pod information will not be passed on mount.
+	// If value is set to a valid version, Kubelet will pass pod information as
 	// VolumeAttributes in the CSI NodePublishVolume() calls.
-	// If value is not specified, default is false -- meaning pod information
+	// Supported versions:
-	// will not be passed on mount.
+	// Version "v1" will pass the following ValueAttributes
+	// "csi.storage.k8s.io/pod.name": pod.Name
+	// "csi.storage.k8s.io/pod.namespace": pod.Namespace
+	// "csi.storage.k8s.io/pod.uid": string(pod.UID)
 	// +optional
-	PodInfoRequiredOnMount *bool `json:"podInfoRequiredOnMount"`
+	PodInfoOnMountVersion *string `json:"podInfoOnMountVersion"`
 }
 
 // +genclient
@@ -82,24 +87,42 @@ type CSIDriverSpec struct {
 // CSINodeInfo holds information about all CSI drivers installed on a node.
 type CSINodeInfo struct {
 	metav1.TypeMeta `json:",inline"`
-	// ObjectMeta.Name must be node name.
+
+	// metadata.name must be the Kubernetes node name.
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
 	// List of CSI drivers running on the node and their properties.
-	CSIDrivers []CSIDriverInfo `json:"csiDrivers"`
+	// +patchMergeKey=driver
+	// +patchStrategy=merge
+	CSIDrivers []CSIDriverInfo `json:"csiDrivers" patchStrategy:"merge" patchMergeKey:"driver"`
 }
 
 // CSIDriverInfo contains information about one CSI driver installed on a node.
 type CSIDriverInfo struct {
-	// Driver is the name of the CSI driver that this object refers to.
+	// driver is the name of the CSI driver that this object refers to.
 	// This MUST be the same name returned by the CSI GetPluginName() call for
 	// that driver.
 	Driver string `json:"driver"`
 
-	// ID of the node from the driver point of view.
+	// nodeID of the node from the driver point of view.
+	// This field enables Kubernetes to communicate with storage systems that do
+	// not share the same nomenclature for nodes. For example, Kubernetes may
+	// refer to a given node as "node1", but the storage system may refer to
+	// the same node as "nodeA". When Kubernetes issues a command to the storage
+	// system to attach a volume to a specific node, it can use this field to
+	// refer to the node name using the ID that the storage system will
+	// understand, e.g. "nodeA" instead of "node1".
 	NodeID string `json:"nodeID"`
 
-	// Topology keys reported by the driver on the node.
+	// topologyKeys is the list of keys supported by the driver.
+	// When a driver is initialized on a cluster, it provides a set of topology
+	// keys that it understands (e.g. "company.com/zone", "company.com/region").
+	// When a driver is initialized on a node it provides the same topology keys
+	// along with values that kubelet applies to the coresponding node API
+	// object as labels.
+	// When Kubernetes does topology aware provisioning, it can use this list to
+	// determine which labels it should retrieve from the node object and pass
+	// back to the driver.
 	TopologyKeys []string `json:"topologyKeys"`
 }
 
@@ -108,10 +131,12 @@ type CSIDriverInfo struct {
 // CSINodeInfoList is a collection of CSINodeInfo objects.
 type CSINodeInfoList struct {
 	metav1.TypeMeta `json:",inline"`
+
 	// Standard list metadata
 	// More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata
 	// +optional
 	metav1.ListMeta `json:"metadata,omitempty"`
-	// Items is the list of CSINodeInfo
+
+	// items is the list of CSINodeInfo
 	Items []CSINodeInfo `json:"items"`
 }

staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/zz_generated.deepcopy.go

@@ -113,9 +113,9 @@ func (in *CSIDriverSpec) DeepCopyInto(out *CSIDriverSpec) {
 		*out = new(bool)
 		**out = **in
 	}
-	if in.PodInfoRequiredOnMount != nil {
+	if in.PodInfoOnMountVersion != nil {
-		in, out := &in.PodInfoRequiredOnMount, &out.PodInfoRequiredOnMount
+		in, out := &in.PodInfoOnMountVersion, &out.PodInfoOnMountVersion
-		*out = new(bool)
+		*out = new(string)
 		**out = **in
 	}
 	return