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# #328

Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
  • Loading branch information
humblec committed Feb 18, 2020
1 parent 4aa560e commit 3c96181
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 13 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 @@ -208,6 +206,7 @@ type csiProvisioner struct {
timeout time.Duration
identity string
volumeNamePrefix string
defaultFSType string
volumeNameUUIDLength int
config *rest.Config
driverName string
Expand Down Expand Up @@ -284,6 +283,7 @@ func NewCSIProvisioner(client kubernetes.Interface,
csiNodeLister storagelistersv1beta1.CSINodeLister,
nodeLister corelisters.NodeLister,
extraCreateMetadata bool,
defaultFSType string,
) controller.Provisioner {

csiClient := csi.NewControllerClient(grpcClient)
Expand All @@ -295,6 +295,7 @@ func NewCSIProvisioner(client kubernetes.Interface,
timeout: connectionTimeout,
identity: identity,
volumeNamePrefix: volumeNamePrefix,
defaultFSType: defaultFSType,
volumeNameUUIDLength: volumeNameUUIDLength,
driverName: driverName,
pluginCapabilities: pluginCapabilities,
Expand Down Expand Up @@ -495,8 +496,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
19 changes: 10 additions & 9 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ var (

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

type csiConnection struct {
Expand Down Expand Up @@ -405,7 +406,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 @@ -1662,7 +1663,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 @@ -2398,7 +2399,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 @@ -2572,7 +2573,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 @@ -2627,7 +2628,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 @@ -2799,7 +2800,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 @@ -3501,7 +3502,7 @@ func TestProvisionFromPVC(t *testing.T) {
}

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 @@ -3580,7 +3581,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 @@ -3740,7 +3741,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 3c96181

Please sign in to comment.