Skip to content

Commit

Permalink
Make fstype configurable in external provisioner
Browse files Browse the repository at this point in the history
At present the fstype is set to `ext4` if nothing is passed in storage-class.
However a SP could prefer to have different fstype for many reasons for their
driver/volumes. This patch enables SP who is using the external-provisioner
to choose the default fstype which they want to have it.

Fix# kubernetes-csi#328

Signed-off-by: Humble Chirammal <[email protected]>
  • Loading branch information
humblec committed Mar 13, 2020
1 parent efcaee7 commit ecb715c
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 14 deletions.
3 changes: 3 additions & 0 deletions cmd/csi-provisioner/csi-provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,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", "ext4", "Specify the filesystem type of the volume. If not specified, external-provisioner will set default as `ext4`.")

featureGates map[string]bool
provisionController *controller.ProvisionController
version = "unknown"
Expand Down Expand Up @@ -224,6 +226,7 @@ func main() {
csiNodeLister,
nodeLister,
*extraCreateMetadata,
*defaultFSType,
)

provisionController = controller.NewProvisionController(
Expand Down
9 changes: 5 additions & 4 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -209,6 +207,7 @@ type csiProvisioner struct {
timeout time.Duration
identity string
volumeNamePrefix string
defaultFSType string
volumeNameUUIDLength int
config *rest.Config
driverName string
Expand Down Expand Up @@ -285,6 +284,7 @@ func NewCSIProvisioner(client kubernetes.Interface,
csiNodeLister storagelistersv1beta1.CSINodeLister,
nodeLister corelisters.NodeLister,
extraCreateMetadata bool,
defaultFSType string,
) controller.Provisioner {

csiClient := csi.NewControllerClient(grpcClient)
Expand All @@ -296,6 +296,7 @@ func NewCSIProvisioner(client kubernetes.Interface,
timeout: connectionTimeout,
identity: identity,
volumeNamePrefix: volumeNamePrefix,
defaultFSType: defaultFSType,
volumeNameUUIDLength: volumeNameUUIDLength,
driverName: driverName,
pluginCapabilities: pluginCapabilities,
Expand Down Expand Up @@ -500,8 +501,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 != "none" {
fsType = p.defaultFSType
}

capacity := options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)]
Expand Down
20 changes: 10 additions & 10 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ var (

driverNameAnnotation = map[string]string{annStorageProvisioner: driverName}
translatedKey = "translated"
defaultfsType = "ext4"
)

type csiConnection struct {
Expand Down Expand Up @@ -404,7 +405,7 @@ func TestCreateDriverReturnsInvalidCapacityDuringProvision(t *testing.T) {

pluginCaps, controllerCaps := provisionCapabilities()
csiProvisioner := NewCSIProvisioner(nil, 5*time.Second, "test-provisioner", "test",
5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, false)
5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, false, defaultfsType)

// Requested PVC with requestedBytes storage
deletePolicy := v1.PersistentVolumeReclaimDelete
Expand Down Expand Up @@ -1669,7 +1670,7 @@ func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requested

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, tc.withExtraMetadata)
nil, provisionDriverName, pluginCaps, controllerCaps, supportsMigrationFromInTreePluginName, false, csitrans.New(), nil, nil, nil, tc.withExtraMetadata, defaultfsType)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -2405,7 +2406,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, false)
client, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, false, defaultfsType)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -2579,7 +2580,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, false)
csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, csiNodeLister, nodeLister, false, defaultfsType)

pv, err := csiProvisioner.Provision(controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{},
Expand Down Expand Up @@ -2634,7 +2635,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, false)
csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, false, defaultfsType)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -2806,7 +2807,7 @@ func runDeleteTest(t *testing.T, k string, tc deleteTestcase) {
pluginCaps, controllerCaps := provisionCapabilities()
scLister, _, _, _ := listers(clientSet)
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5,
csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, nil, nil, false)
csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), scLister, nil, nil, false, defaultfsType)

err = csiProvisioner.Delete(tc.persistentVolume)
if tc.expectErr && err == nil {
Expand Down Expand Up @@ -3217,9 +3218,8 @@ func TestProvisionFromPVC(t *testing.T) {
}).Return(&csi.DeleteVolumeResponse{}, nil).Times(1)
}

// 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, false)
nil, driverName, pluginCaps, controllerCaps, "", false, csitrans.New(), nil, nil, nil, false, defaultfsType)

pv, err = csiProvisioner.Provision(tc.volOpts)
if tc.expectErr && err == nil {
Expand Down Expand Up @@ -3325,7 +3325,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, false)
inTreePluginName, false, mockTranslator, nil, nil, nil, false, defaultfsType)

// Set up return values (AnyTimes to avoid overfitting on implementation)

Expand Down Expand Up @@ -3485,7 +3485,7 @@ func TestDeleteMigration(t *testing.T) {
pluginCaps, controllerCaps := provisionCapabilities()
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner",
"test", 5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "",
false, mockTranslator, nil, nil, nil, false)
false, mockTranslator, nil, nil, nil, false, defaultfsType)

// Set mock return values (AnyTimes to avoid overfitting on implementation details)
mockTranslator.EXPECT().IsPVMigratable(gomock.Any()).Return(tc.expectTranslation).AnyTimes()
Expand Down

0 comments on commit ecb715c

Please sign in to comment.