diff --git a/cmd/csi-provisioner/csi-provisioner.go b/cmd/csi-provisioner/csi-provisioner.go index f1a40ca2bf..d87e24a132 100644 --- a/cmd/csi-provisioner/csi-provisioner.go +++ b/cmd/csi-provisioner/csi-provisioner.go @@ -26,28 +26,25 @@ import ( "strings" "time" - flag "github.com/spf13/pflag" - "github.com/kubernetes-csi/csi-lib-utils/deprecatedflags" "github.com/kubernetes-csi/csi-lib-utils/leaderelection" "github.com/kubernetes-csi/csi-lib-utils/metrics" ctrl "github.com/kubernetes-csi/external-provisioner/pkg/controller" snapclientset "github.com/kubernetes-csi/external-snapshotter/pkg/client/clientset/versioned" - "sigs.k8s.io/sig-storage-lib-external-provisioner/v5/controller" - + flag "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" - "k8s.io/client-go/util/workqueue" - "k8s.io/klog" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" v1 "k8s.io/client-go/listers/core/v1" storagelistersv1 "k8s.io/client-go/listers/storage/v1" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/util/workqueue" utilflag "k8s.io/component-base/cli/flag" csitrans "k8s.io/csi-translation-lib" + "k8s.io/klog" + "sigs.k8s.io/sig-storage-lib-external-provisioner/v5/controller" ) var ( @@ -74,6 +71,8 @@ var ( metricsAddress = flag.String("metrics-address", "", "The TCP network address where the prometheus metrics endpoint will listen (example: `:8080`). The default is empty string, which means metrics endpoint is disabled.") metricsPath = flag.String("metrics-path", "/metrics", "The HTTP path where prometheus metrics will be exposed. Default is `/metrics`.") + defaultFSType = flag.String("default-fstype", "", "The default filesystem type of the volume to provision when fstype is unspecified in the StorageClass. If the default is not set and fstype is unset in the StorageClass, then no fstype will be set") + featureGates map[string]bool provisionController *controller.ProvisionController version = "unknown" @@ -240,6 +239,7 @@ func main() { claimLister, vaLister, *extraCreateMetadata, + *defaultFSType, ) provisionController = controller.NewProvisionController( diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 348ce85ba7..0404c9e587 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -116,8 +116,6 @@ const ( backoffFactor = 1.2 backoffSteps = 10 - defaultFSType = "ext4" - snapshotKind = "VolumeSnapshot" snapshotAPIGroup = snapapi.GroupName // "snapshot.storage.k8s.io" pvcKind = "PersistentVolumeClaim" // Native types don't require an API group @@ -211,6 +209,7 @@ type csiProvisioner struct { timeout time.Duration identity string volumeNamePrefix string + defaultFSType string volumeNameUUIDLength int config *rest.Config driverName string @@ -292,6 +291,7 @@ func NewCSIProvisioner(client kubernetes.Interface, claimLister corelisters.PersistentVolumeClaimLister, vaLister storagelistersv1.VolumeAttachmentLister, extraCreateMetadata bool, + defaultFSType string, ) controller.Provisioner { broadcaster := record.NewBroadcaster() broadcaster.StartLogging(klog.Infof) @@ -307,6 +307,7 @@ func NewCSIProvisioner(client kubernetes.Interface, timeout: connectionTimeout, identity: identity, volumeNamePrefix: volumeNamePrefix, + defaultFSType: defaultFSType, volumeNameUUIDLength: volumeNameUUIDLength, driverName: driverName, pluginCapabilities: pluginCapabilities, @@ -518,8 +519,8 @@ func (p *csiProvisioner) ProvisionExt(options controller.ProvisionOptions) (*v1. if fsTypesFound > 1 { return nil, controller.ProvisioningFinished, fmt.Errorf("fstype specified in parameters with both \"fstype\" and \"%s\" keys", prefixedFsTypeKey) } - if len(fsType) == 0 { - fsType = defaultFSType + if fsType == "" && p.defaultFSType != "" { + fsType = p.defaultFSType } capacity := options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 38b6a590e9..6316bcf694 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -27,19 +27,11 @@ import ( "testing" "time" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - "github.com/container-storage-interface/spec/lib/go/csi" "github.com/golang/mock/gomock" - "github.com/kubernetes-csi/csi-lib-utils/connection" - "github.com/kubernetes-csi/csi-lib-utils/metrics" - "github.com/kubernetes-csi/csi-lib-utils/rpc" - "github.com/kubernetes-csi/csi-test/driver" - "github.com/kubernetes-csi/external-provisioner/pkg/features" - crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1" - "github.com/kubernetes-csi/external-snapshotter/pkg/client/clientset/versioned/fake" "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" v1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -54,6 +46,14 @@ import ( csitrans "k8s.io/csi-translation-lib" "k8s.io/klog" "sigs.k8s.io/sig-storage-lib-external-provisioner/v5/controller" + + "github.com/kubernetes-csi/csi-lib-utils/connection" + "github.com/kubernetes-csi/csi-lib-utils/metrics" + "github.com/kubernetes-csi/csi-lib-utils/rpc" + "github.com/kubernetes-csi/csi-test/driver" + "github.com/kubernetes-csi/external-provisioner/pkg/features" + crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1beta1" + "github.com/kubernetes-csi/external-snapshotter/pkg/client/clientset/versioned/fake" ) func init() { @@ -71,6 +71,7 @@ var ( driverNameAnnotation = map[string]string{annStorageProvisioner: driverName} translatedKey = "translated" + defaultfsType = "ext4" ) type csiConnection struct { @@ -412,7 +413,7 @@ func TestCreateDriverReturnsInvalidCapacityDuringProvision(t *testing.T) { pluginCaps, controllerCaps := provisionCapabilities() csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", - 5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, nil, nil, false) + 5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, nil, nil, false, defaultfsType) // Requested PVC with requestedBytes storage deletePolicy := v1.PersistentVolumeReclaimDelete @@ -814,6 +815,19 @@ type provisioningTestcase struct { skipCreateVolume bool } +type provisioningFSTypeTestcase struct { + volOpts controller.ProvisionOptions + + expectedPVSpec *pvSpec + clientSetObjects []runtime.Object + createVolumeError error + expectErr bool + expectState controller.ProvisioningState + expectCreateVolDo interface{} + + skipDefaultFSType bool +} + type pvSpec struct { Name string ReclaimPolicy v1.PersistentVolumeReclaimPolicy @@ -865,6 +879,135 @@ func getDefaultSecretObjects() []runtime.Object { } } +func TestFSTypeProvision(t *testing.T) { + var requestedBytes int64 = 100 + deletePolicy := v1.PersistentVolumeReclaimDelete + testcases := map[string]provisioningFSTypeTestcase{ + "fstype not set/'nil' in SC to provision": { + volOpts: controller.ProvisionOptions{ + StorageClass: &storagev1.StorageClass{ + ReclaimPolicy: &deletePolicy, + Parameters: map[string]string{ + // We deliberately skip fsType in sc param + // "fstype": "", + }, + }, + PVName: "test-name", + PVC: createFakePVC(requestedBytes), + }, + expectedPVSpec: &pvSpec{ + Name: "test-testi", + ReclaimPolicy: v1.PersistentVolumeReclaimDelete, + Capacity: v1.ResourceList{ + v1.ResourceName(v1.ResourceStorage): bytesToGiQuantity(requestedBytes), + }, + CSIPVS: &v1.CSIPersistentVolumeSource{ + Driver: "test-driver", + VolumeHandle: "test-volume-id", + FSType: "ext4", + VolumeAttributes: map[string]string{ + "storage.kubernetes.io/csiProvisionerIdentity": "test-provisioner", + }, + }, + }, + expectState: controller.ProvisioningFinished, + }, + "Other fstype(ex:'xfs') set in SC": { + volOpts: controller.ProvisionOptions{ + StorageClass: &storagev1.StorageClass{ + ReclaimPolicy: &deletePolicy, + Parameters: map[string]string{ + "fstype": "xfs", + }, + }, + PVName: "test-name", + PVC: createFakePVC(requestedBytes), + }, + expectedPVSpec: &pvSpec{ + Name: "test-testi", + ReclaimPolicy: v1.PersistentVolumeReclaimDelete, + Capacity: v1.ResourceList{ + v1.ResourceName(v1.ResourceStorage): bytesToGiQuantity(requestedBytes), + }, + CSIPVS: &v1.CSIPersistentVolumeSource{ + Driver: "test-driver", + VolumeHandle: "test-volume-id", + FSType: "xfs", + VolumeAttributes: map[string]string{ + "storage.kubernetes.io/csiProvisionerIdentity": "test-provisioner", + }, + }, + }, + expectState: controller.ProvisioningFinished, + }, + + "fstype not set/Nil in SC and defaultFSType arg unset for provisioner": { + volOpts: controller.ProvisionOptions{ + StorageClass: &storagev1.StorageClass{ + ReclaimPolicy: &deletePolicy, + Parameters: map[string]string{ + // We deliberately skip fsType in sc param + // "fstype": "xfs", + }, + }, + PVName: "test-name", + PVC: createFakePVC(requestedBytes), + }, + skipDefaultFSType: true, + expectedPVSpec: &pvSpec{ + Name: "test-testi", + ReclaimPolicy: v1.PersistentVolumeReclaimDelete, + Capacity: v1.ResourceList{ + v1.ResourceName(v1.ResourceStorage): bytesToGiQuantity(requestedBytes), + }, + CSIPVS: &v1.CSIPersistentVolumeSource{ + Driver: "test-driver", + VolumeHandle: "test-volume-id", + FSType: "", + VolumeAttributes: map[string]string{ + "storage.kubernetes.io/csiProvisionerIdentity": "test-provisioner", + }, + }, + }, + expectState: controller.ProvisioningFinished, + }, + + "fstype set in SC and defaultFSType arg unset for provisioner": { + volOpts: controller.ProvisionOptions{ + StorageClass: &storagev1.StorageClass{ + ReclaimPolicy: &deletePolicy, + Parameters: map[string]string{ + "fstype": "xfs", + }, + }, + PVName: "test-name", + PVC: createFakePVC(requestedBytes), + }, + skipDefaultFSType: true, + expectedPVSpec: &pvSpec{ + Name: "test-testi", + ReclaimPolicy: v1.PersistentVolumeReclaimDelete, + Capacity: v1.ResourceList{ + v1.ResourceName(v1.ResourceStorage): bytesToGiQuantity(requestedBytes), + }, + CSIPVS: &v1.CSIPersistentVolumeSource{ + Driver: "test-driver", + VolumeHandle: "test-volume-id", + FSType: "xfs", + VolumeAttributes: map[string]string{ + "storage.kubernetes.io/csiProvisionerIdentity": "test-provisioner", + }, + }, + }, + expectState: controller.ProvisioningFinished, + }, + } + + for k, tc := range testcases { + runFSTypeProvisionTest(t, k, tc, requestedBytes, driverName, "" /* no migration */) + } +} + func TestProvision(t *testing.T) { var requestedBytes int64 = 100 deletePolicy := v1.PersistentVolumeReclaimDelete @@ -1693,10 +1836,9 @@ func newSnapshot(name, className, boundToContent, snapshotUID, claimName string, return &snapshot } - -func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requestedBytes int64, provisionDriverName, supportsMigrationFromInTreePluginName string) { +func runFSTypeProvisionTest(t *testing.T, k string, tc provisioningFSTypeTestcase, requestedBytes int64, provisionDriverName, supportsMigrationFromInTreePluginName string) { t.Logf("Running test: %v", k) - + myDefaultfsType := "ext4" tmpdir := tempDir(t) defer os.RemoveAll(tmpdir) mockController, driver, _, controllerServer, csiConn, err := createMockServer(t, tmpdir) @@ -1709,9 +1851,88 @@ func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requested clientSet := fakeclientset.NewSimpleClientset(tc.clientSetObjects...) pluginCaps, controllerCaps := provisionCapabilities() + + if tc.skipDefaultFSType { + myDefaultfsType = "" + } csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, - nil, provisionDriverName, pluginCaps, controllerCaps, supportsMigrationFromInTreePluginName, false, csitrans.New(), nil, nil, nil, nil, nil, tc.withExtraMetadata) + nil, provisionDriverName, pluginCaps, controllerCaps, supportsMigrationFromInTreePluginName, false, csitrans.New(), nil, nil, nil, nil, nil, false, myDefaultfsType) + out := &csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: requestedBytes, + VolumeId: "test-volume-id", + }, + } + + // Setup regular mock call expectations. + if !tc.expectErr { + controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, tc.createVolumeError).Times(1) + } + + pv, state, err := csiProvisioner.(controller.ProvisionerExt).ProvisionExt(tc.volOpts) + if tc.expectErr && err == nil { + t.Errorf("test %q: Expected error, got none", k) + } + if !tc.expectErr && err != nil { + t.Fatalf("test %q: got error: %v", k, err) + } + + if tc.expectState == "" { + tc.expectState = controller.ProvisioningFinished + } + if tc.expectState != state { + t.Errorf("test %q: expected ProvisioningState %s, got %s", k, tc.expectState, state) + } + + if tc.expectedPVSpec != nil { + if pv.Name != tc.expectedPVSpec.Name { + t.Errorf("test %q: expected PV name: %q, got: %q", k, tc.expectedPVSpec.Name, pv.Name) + } + + if pv.Spec.PersistentVolumeReclaimPolicy != tc.expectedPVSpec.ReclaimPolicy { + t.Errorf("test %q: expected reclaim policy: %v, got: %v", k, tc.expectedPVSpec.ReclaimPolicy, pv.Spec.PersistentVolumeReclaimPolicy) + } + + if !reflect.DeepEqual(pv.Spec.AccessModes, tc.expectedPVSpec.AccessModes) { + t.Errorf("test %q: expected access modes: %v, got: %v", k, tc.expectedPVSpec.AccessModes, pv.Spec.AccessModes) + } + + if !reflect.DeepEqual(pv.Spec.VolumeMode, tc.expectedPVSpec.VolumeMode) { + t.Errorf("test %q: expected volumeMode: %v, got: %v", k, tc.expectedPVSpec.VolumeMode, pv.Spec.VolumeMode) + } + + if !reflect.DeepEqual(pv.Spec.Capacity, tc.expectedPVSpec.Capacity) { + t.Errorf("test %q: expected capacity: %v, got: %v", k, tc.expectedPVSpec.Capacity, pv.Spec.Capacity) + } + + if !reflect.DeepEqual(pv.Spec.MountOptions, tc.expectedPVSpec.MountOptions) { + t.Errorf("test %q: expected mount options: %v, got: %v", k, tc.expectedPVSpec.MountOptions, pv.Spec.MountOptions) + } + + if tc.expectedPVSpec.CSIPVS != nil { + if !reflect.DeepEqual(pv.Spec.PersistentVolumeSource.CSI, tc.expectedPVSpec.CSIPVS) { + t.Errorf("test %q: expected PV: %v, got: %v", k, tc.expectedPVSpec.CSIPVS, pv.Spec.PersistentVolumeSource.CSI) + } + } + } +} + +func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requestedBytes int64, provisionDriverName, supportsMigrationFromInTreePluginName string) { + t.Logf("Running test: %v", k) + tmpdir := tempDir(t) + defer os.RemoveAll(tmpdir) + mockController, driver, _, controllerServer, csiConn, err := createMockServer(t, tmpdir) + if err != nil { + t.Fatal(err) + } + defer mockController.Finish() + defer driver.Stop() + + clientSet := fakeclientset.NewSimpleClientset(tc.clientSetObjects...) + pluginCaps, controllerCaps := provisionCapabilities() + csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, + nil, provisionDriverName, pluginCaps, controllerCaps, supportsMigrationFromInTreePluginName, false, csitrans.New(), nil, nil, nil, nil, nil, tc.withExtraMetadata, defaultfsType) out := &csi.CreateVolumeResponse{ Volume: &csi.Volume{ CapacityBytes: requestedBytes, @@ -2446,7 +2667,7 @@ func TestProvisionFromSnapshot(t *testing.T) { pluginCaps, controllerCaps := provisionFromSnapshotCapabilities() csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, - client, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, nil, nil, false) + client, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, nil, nil, false, defaultfsType) out := &csi.CreateVolumeResponse{ Volume: &csi.Volume{ @@ -2620,7 +2841,7 @@ func TestProvisionWithTopologyEnabled(t *testing.T) { defer close(stopChan) csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, - csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, csiNodeLister, nodeLister, claimLister, vaLister, false) + csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, csiNodeLister, nodeLister, claimLister, vaLister, false, defaultfsType) pv, err := csiProvisioner.Provision(controller.ProvisionOptions{ StorageClass: &storagev1.StorageClass{}, @@ -2714,7 +2935,7 @@ func TestProvisionErrorHandling(t *testing.T) { defer close(stopChan) csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, - csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, csiNodeLister, nodeLister, claimLister, vaLister, false) + csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, csiNodeLister, nodeLister, claimLister, vaLister, false, defaultfsType) csiProvisionerExt := csiProvisioner.(controller.ProvisionerExt) options := controller.ProvisionOptions{ @@ -2788,7 +3009,7 @@ func TestProvisionWithTopologyDisabled(t *testing.T) { clientSet := fakeclientset.NewSimpleClientset() pluginCaps, controllerCaps := provisionWithTopologyCapabilities() csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, - csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, nil, nil, false) + csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, nil, nil, false, defaultfsType) out := &csi.CreateVolumeResponse{ Volume: &csi.Volume{ @@ -3125,7 +3346,7 @@ func runDeleteTest(t *testing.T, k string, tc deleteTestcase) { pluginCaps, controllerCaps := provisionCapabilities() scLister, _, _, _, vaLister, _ := listers(clientSet) csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, - csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, nil, nil, nil, vaLister, false) + csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, nil, nil, nil, vaLister, false, defaultfsType) err = csiProvisioner.Delete(tc.persistentVolume) if tc.expectErr && err == nil { @@ -3143,7 +3364,7 @@ func generatePVCForProvisionFromPVC(srcNamespace, srcName, scName string, reques provisionRequest := controller.ProvisionOptions{ StorageClass: &storagev1.StorageClass{ ReclaimPolicy: &deletePolicy, - Parameters: map[string]string{}, + Parameters: map[string]string{"fstype": "ext4"}, Provisioner: driverName, }, PVName: "new-pv-name", @@ -3546,7 +3767,7 @@ func TestProvisionFromPVC(t *testing.T) { // Phase: execute the test csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, - nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, claimLister, nil, false) + nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, claimLister, nil, false, defaultfsType) pv, err = csiProvisioner.Provision(tc.volOpts) if tc.expectErr && err == nil { @@ -3664,7 +3885,7 @@ func TestProvisionWithMigration(t *testing.T) { pluginCaps, controllerCaps := provisionCapabilities() csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, - inTreePluginName, false, mockTranslator, nil, nil, nil, nil, nil, false) + inTreePluginName, false, mockTranslator, nil, nil, nil, nil, nil, false, defaultfsType) // Set up return values (AnyTimes to avoid overfitting on implementation) @@ -3826,7 +4047,7 @@ func TestDeleteMigration(t *testing.T) { defer close(stopCh) csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", - false, mockTranslator, nil, nil, nil, nil, vaLister, false) + false, mockTranslator, nil, nil, nil, nil, vaLister, false, defaultfsType) // Set mock return values (AnyTimes to avoid overfitting on implementation details) mockTranslator.EXPECT().IsPVMigratable(gomock.Any()).Return(tc.expectTranslation).AnyTimes()