Add review comments in the APIs

pull/564/head
Xing Yang 2019-03-01 20:11:48 -08:00
parent 743d3a26e9
commit ba4ccfa8b1
7 changed files with 59 additions and 13 deletions

View File

@ -42,6 +42,10 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
obj.Spec.AttachRequired = new(bool) obj.Spec.AttachRequired = new(bool)
*(obj.Spec.AttachRequired) = true *(obj.Spec.AttachRequired) = true
} }
if obj.Spec.PodInfoOnMount == nil {
obj.Spec.PodInfoOnMount = new(bool)
*(obj.Spec.PodInfoOnMount) = false
}
}, },
} }
} }

View File

@ -278,13 +278,15 @@ type CSIDriverSpec struct {
// If set to true, podInfoOnMount indicates this CSI volume driver // If set to true, podInfoOnMount indicates this CSI volume driver
// requires additional pod information (like podName, podUID, etc.) during // requires additional pod information (like podName, podUID, etc.) during
// mount operations. // mount operations.
// If not set or set to false, pod information will not be passed on mount. // If set to false, pod information will not be passed on mount.
// Default is false.
// The CSI driver specifies podInfoOnMount as part of driver deployment. // The CSI driver specifies podInfoOnMount as part of driver deployment.
// If true, Kubelet will pass pod information as VolumeContext in the CSI // If true, Kubelet will pass pod information as VolumeContext in the CSI
// NodePublishVolume() calls. // NodePublishVolume() calls.
// The CSI driver is responsible for parsing and validating the information // The CSI driver is responsible for parsing and validating the information
// passed in as VolumeContext. // passed in as VolumeContext.
// The following VolumeConext will be passed if podInfoOnMount is set to true: // The following VolumeConext will be passed if podInfoOnMount is set to true.
// This list might grow, but the prefix will be used.
// "csi.storage.k8s.io/pod.name": pod.Name // "csi.storage.k8s.io/pod.name": pod.Name
// "csi.storage.k8s.io/pod.namespace": pod.Namespace // "csi.storage.k8s.io/pod.namespace": pod.Namespace
// "csi.storage.k8s.io/pod.uid": string(pod.UID) // "csi.storage.k8s.io/pod.uid": string(pod.UID)
@ -318,7 +320,7 @@ type CSINode struct {
// CSINodeSpec holds information about the specification of all CSI drivers installed on a node // CSINodeSpec holds information about the specification of all CSI drivers installed on a node
type CSINodeSpec struct { type CSINodeSpec struct {
// drivers is a list of information of all CSI Drivers existing on a node. // drivers is a list of information of all CSI Drivers existing on a node.
// It can be empty on initialization. // If all drivers in the list are uninstalled, this can become empty.
// +patchMergeKey=name // +patchMergeKey=name
// +patchStrategy=merge // +patchStrategy=merge
Drivers []CSINodeDriver Drivers []CSINodeDriver

View File

@ -43,4 +43,8 @@ func SetDefaults_CSIDriver(obj *storagev1beta1.CSIDriver) {
obj.Spec.AttachRequired = new(bool) obj.Spec.AttachRequired = new(bool)
*(obj.Spec.AttachRequired) = true *(obj.Spec.AttachRequired) = true
} }
if obj.Spec.PodInfoOnMount == nil {
obj.Spec.PodInfoOnMount = new(bool)
*(obj.Spec.PodInfoOnMount) = false
}
} }

View File

@ -65,12 +65,19 @@ func TestSetDefaultAttachRequired(t *testing.T) {
driver := &storagev1beta1.CSIDriver{} driver := &storagev1beta1.CSIDriver{}
// field should be defaulted // field should be defaulted
defaultMode := true defaultAttach := true
defaultPodInfo := false
output := roundTrip(t, runtime.Object(driver)).(*storagev1beta1.CSIDriver) output := roundTrip(t, runtime.Object(driver)).(*storagev1beta1.CSIDriver)
outMode := output.Spec.AttachRequired outAttach := output.Spec.AttachRequired
if outMode == nil { if outAttach == nil {
t.Errorf("Expected AttachRequired to be defaulted to: %+v, got: nil", defaultMode) t.Errorf("Expected AttachRequired to be defaulted to: %+v, got: nil", defaultAttach)
} else if *outMode != defaultMode { } else if *outAttach != defaultAttach {
t.Errorf("Expected AttachRequired to be defaulted to: %+v, got: %+v", defaultMode, outMode) t.Errorf("Expected AttachRequired to be defaulted to: %+v, got: %+v", defaultAttach, outAttach)
}
outPodInfo := output.Spec.PodInfoOnMount
if outPodInfo == nil {
t.Errorf("Expected PodInfoOnMount to be defaulted to: %+v, got: nil", defaultPodInfo)
} else if *outPodInfo != defaultPodInfo {
t.Errorf("Expected PodInfoOnMount to be defaulted to: %+v, got: %+v", defaultPodInfo, outPodInfo)
} }
} }

View File

@ -324,7 +324,7 @@ func validateCSINodeDriverNodeID(nodeID string, fldPath *field.Path) field.Error
allErrs = append(allErrs, field.Required(fldPath, nodeID)) allErrs = append(allErrs, field.Required(fldPath, nodeID))
} }
if len(nodeID) > csiNodeIDMaxLength { if len(nodeID) > csiNodeIDMaxLength {
allErrs = append(allErrs, field.Invalid(fldPath, nodeID, fmt.Sprintf("nodeID must be %d characters or less", csiNodeIDMaxLength))) allErrs = append(allErrs, field.Invalid(fldPath, nodeID, fmt.Sprintf("must be %d characters or less", csiNodeIDMaxLength)))
} }
return allErrs return allErrs
} }
@ -387,6 +387,7 @@ func validateCSIDriverSpec(
spec *storage.CSIDriverSpec, fldPath *field.Path) field.ErrorList { spec *storage.CSIDriverSpec, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
allErrs = append(allErrs, validateAttachRequired(spec.AttachRequired, fldPath.Child("attachedRequired"))...) allErrs = append(allErrs, validateAttachRequired(spec.AttachRequired, fldPath.Child("attachedRequired"))...)
allErrs = append(allErrs, validatePodInfoOnMount(spec.PodInfoOnMount, fldPath.Child("podInfoOnMount"))...)
return allErrs return allErrs
} }
@ -399,3 +400,13 @@ func validateAttachRequired(attachRequired *bool, fldPath *field.Path) field.Err
return allErrs return allErrs
} }
// validatePodInfoOnMount tests if podInfoOnMount is set for CSIDriver.
func validatePodInfoOnMount(podInfoOnMount *bool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if podInfoOnMount == nil {
allErrs = append(allErrs, field.Required(fldPath, ""))
}
return allErrs
}

View File

@ -1471,6 +1471,22 @@ func TestCSIDriverValidation(t *testing.T) {
PodInfoOnMount: &notPodInfoOnMount, PodInfoOnMount: &notPodInfoOnMount,
}, },
}, },
{
// AttachRequired not set
ObjectMeta: metav1.ObjectMeta{Name: driverName},
Spec: storage.CSIDriverSpec{
AttachRequired: nil,
PodInfoOnMount: &podInfoOnMount,
},
},
{
// AttachRequired not set
ObjectMeta: metav1.ObjectMeta{Name: driverName},
Spec: storage.CSIDriverSpec{
AttachRequired: &attachNotRequired,
PodInfoOnMount: nil,
},
},
} }
for _, csiDriver := range errorCases { for _, csiDriver := range errorCases {

View File

@ -272,13 +272,15 @@ type CSIDriverSpec struct {
// If set to true, podInfoOnMount indicates this CSI volume driver // If set to true, podInfoOnMount indicates this CSI volume driver
// requires additional pod information (like podName, podUID, etc.) during // requires additional pod information (like podName, podUID, etc.) during
// mount operations. // mount operations.
// If not set or set to false, pod information will not be passed on mount. // If set to false, pod information will not be passed on mount.
// Default is false.
// The CSI driver specifies podInfoOnMount as part of driver deployment. // The CSI driver specifies podInfoOnMount as part of driver deployment.
// If true, Kubelet will pass pod information as VolumeContext in the CSI // If true, Kubelet will pass pod information as VolumeContext in the CSI
// NodePublishVolume() calls. // NodePublishVolume() calls.
// The CSI driver is responsible for parsing and validating the information // The CSI driver is responsible for parsing and validating the information
// passed in as VolumeContext. // passed in as VolumeContext.
// The following VolumeConext will be passed if podInfoOnMount is set to true: // The following VolumeConext will be passed if podInfoOnMount is set to true.
// This list might grow, but the prefix will be used.
// "csi.storage.k8s.io/pod.name": pod.Name // "csi.storage.k8s.io/pod.name": pod.Name
// "csi.storage.k8s.io/pod.namespace": pod.Namespace // "csi.storage.k8s.io/pod.namespace": pod.Namespace
// "csi.storage.k8s.io/pod.uid": string(pod.UID) // "csi.storage.k8s.io/pod.uid": string(pod.UID)
@ -312,7 +314,7 @@ type CSINode struct {
// CSINodeSpec holds information about the specification of all CSI drivers installed on a node // CSINodeSpec holds information about the specification of all CSI drivers installed on a node
type CSINodeSpec struct { type CSINodeSpec struct {
// drivers is a list of information of all CSI Drivers existing on a node. // drivers is a list of information of all CSI Drivers existing on a node.
// It can be empty on initialization. // If all drivers in the list are uninstalled, this can become empty.
// +patchMergeKey=name // +patchMergeKey=name
// +patchStrategy=merge // +patchStrategy=merge
Drivers []CSINodeDriver `json:"drivers" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,1,rep,name=drivers"` Drivers []CSINodeDriver `json:"drivers" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,1,rep,name=drivers"`