Included resizing for CSI-based block volumes.

Perform a no-op when volume is of type raw block

Fix bug with checking volume mounts for readonly

Author: huffmanca

pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go

@@ -35,7 +35,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	clientset "k8s.io/client-go/kubernetes"
-	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
 	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/kubelet/config"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
@@ -384,16 +383,6 @@ func (dswp *desiredStateOfWorldPopulator) checkVolumeFSResize(
 		// or online resize in subsequent loop(after we confirm it has been mounted).
 		return
 	}
-	fsVolume, err := util.CheckVolumeModeFilesystem(volumeSpec)
-	if err != nil {
-		klog.Errorf("Check volume mode failed for volume %s(OuterVolumeSpecName %s): %v",
-			uniqueVolumeName, podVolume.Name, err)
-		return
-	}
-	if !fsVolume {
-		klog.V(5).Infof("Block mode volume needn't to check file system resize request")
-		return
-	}
 	if processedVolumesForFSResize.Has(string(uniqueVolumeName)) {
 		// File system resize operation is a global operation for volume,
 		// so we only need to check it once if more than one pod use it.
@@ -415,22 +404,7 @@ func mountedReadOnlyByPod(podVolume v1.Volume, pod *v1.Pod) bool {
 	if podVolume.PersistentVolumeClaim.ReadOnly {
 		return true
 	}
-
-	return podutil.VisitContainers(&pod.Spec, func(c *v1.Container) bool {
-		if !mountedReadOnlyByContainer(podVolume.Name, c) {
-			return false
-		}
-		return true
-	})
-}
-
-func mountedReadOnlyByContainer(volumeName string, container *v1.Container) bool {
-	for _, volumeMount := range container.VolumeMounts {
-		if volumeMount.Name == volumeName && !volumeMount.ReadOnly {
-			return false
-		}
-	}
-	return true
+	return false
 }
 
 func getUniqueVolumeName(

pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go

@@ -568,8 +568,6 @@ func TestCreateVolumeSpec_Invalid_Block_VolumeMounts(t *testing.T) {
 }
 
 func TestCheckVolumeFSResize(t *testing.T) {
-	mode := v1.PersistentVolumeFilesystem
-
 	setCapacity := func(pv *v1.PersistentVolume, pvc *v1.PersistentVolumeClaim, capacity int) {
 		pv.Spec.Capacity = volumeCapacity(capacity)
 		pvc.Spec.Resources.Requests = volumeCapacity(capacity)
@@ -580,6 +578,7 @@ func TestCheckVolumeFSResize(t *testing.T) {
 		verify       func(*testing.T, []v1.UniqueVolumeName, v1.UniqueVolumeName)
 		enableResize bool
 		readOnlyVol  bool
+		volumeMode   v1.PersistentVolumeMode
 	}{
 		{
 			// No resize request for volume, volumes in ASW shouldn't be marked as fsResizeRequired
@@ -591,6 +590,7 @@ func TestCheckVolumeFSResize(t *testing.T) {
 				}
 			},
 			enableResize: true,
+			volumeMode:   v1.PersistentVolumeFilesystem,
 		},
 		{
 			// Disable the feature gate, so volume shouldn't be marked as fsResizeRequired
@@ -603,6 +603,7 @@ func TestCheckVolumeFSResize(t *testing.T) {
 				}
 			},
 			enableResize: false,
+			volumeMode:   v1.PersistentVolumeFilesystem,
 		},
 		{
 			// Make volume used as ReadOnly, so volume shouldn't be marked as fsResizeRequired
@@ -616,6 +617,7 @@ func TestCheckVolumeFSResize(t *testing.T) {
 			},
 			readOnlyVol:  true,
 			enableResize: true,
+			volumeMode:   v1.PersistentVolumeFilesystem,
 		},
 		{
 			// Clear ASW, so volume shouldn't be marked as fsResizeRequired because they are not mounted
@@ -629,6 +631,7 @@ func TestCheckVolumeFSResize(t *testing.T) {
 				}
 			},
 			enableResize: true,
+			volumeMode:   v1.PersistentVolumeFilesystem,
 		},
 		{
 			// volume in ASW should be marked as fsResizeRequired
@@ -647,6 +650,26 @@ func TestCheckVolumeFSResize(t *testing.T) {
 				}
 			},
 			enableResize: true,
+			volumeMode:   v1.PersistentVolumeFilesystem,
+		},
+		{
+			// volume in ASW should be marked as fsResizeRequired
+			resize: func(_ *testing.T, pv *v1.PersistentVolume, pvc *v1.PersistentVolumeClaim, _ *desiredStateOfWorldPopulator) {
+				setCapacity(pv, pvc, 2)
+			},
+			verify: func(t *testing.T, vols []v1.UniqueVolumeName, volName v1.UniqueVolumeName) {
+				if len(vols) == 0 {
+					t.Fatalf("Request resize for volume, but volume in ASW hasn't been marked as fsResizeRequired")
+				}
+				if len(vols) != 1 {
+					t.Errorf("Some unexpected volumes are marked as fsResizeRequired: %v", vols)
+				}
+				if vols[0] != volName {
+					t.Fatalf("Mark wrong volume as fsResizeRequired: %s", vols[0])
+				}
+			},
+			enableResize: true,
+			volumeMode:   v1.PersistentVolumeBlock,
 		},
 	}
 
@@ -659,7 +682,7 @@ func TestCheckVolumeFSResize(t *testing.T) {
 				PersistentVolumeSource: v1.PersistentVolumeSource{RBD: &v1.RBDPersistentVolumeSource{}},
 				Capacity:               volumeCapacity(1),
 				ClaimRef:               &v1.ObjectReference{Namespace: "ns", Name: "file-bound"},
-				VolumeMode:             &mode,
+				VolumeMode:             &tc.volumeMode,
 			},
 		}
 		pvc := &v1.PersistentVolumeClaim{
@@ -677,20 +700,31 @@ func TestCheckVolumeFSResize(t *testing.T) {
 
 		dswp, fakePodManager, fakeDSW := createDswpWithVolume(t, pv, pvc)
 		fakeASW := dswp.actualStateOfWorld
+		containers := []v1.Container{}
 
-		// create pod
-		containers := []v1.Container{
-			{
+		if tc.volumeMode == v1.PersistentVolumeFilesystem {
+			containers = append(containers, v1.Container{
 				VolumeMounts: []v1.VolumeMount{
 					{
 						Name:      pv.Name,
 						MountPath: "/mnt",
 						ReadOnly:  tc.readOnlyVol,
 					},
 				},
-			},
+			})
+		} else {
+			containers = append(containers, v1.Container{
+				VolumeDevices: []v1.VolumeDevice{
+					{
+						Name:       pv.Name,
+						DevicePath: "/mnt/foobar",
+					},
+				},
+			})
 		}
+
 		pod := createPodWithVolume("dswp-test-pod", "dswp-test-volume-name", "file-bound", containers)
+		pod.Spec.Volumes[0].VolumeSource.PersistentVolumeClaim.ReadOnly = tc.readOnlyVol
 		uniquePodName := types.UniquePodName(pod.UID)
 		uniqueVolumeName := v1.UniqueVolumeName("fake-plugin/" + pod.Spec.Volumes[0].Name)
 

pkg/volume/awsebs/aws_ebs.go

@@ -324,7 +324,15 @@ func (plugin *awsElasticBlockStorePlugin) ExpandVolumeDevice(
 }
 
 func (plugin *awsElasticBlockStorePlugin) NodeExpand(resizeOptions volume.NodeResizeOptions) (bool, error) {
-	_, err := util.GenericResizeFS(plugin.host, plugin.GetPluginName(), resizeOptions.DevicePath, resizeOptions.DeviceMountPath)
+	fsVolume, err := util.CheckVolumeModeFilesystem(resizeOptions.VolumeSpec)
+	if err != nil {
+		return false, fmt.Errorf("error checking VolumeMode: %v", err)
+	}
+	// if volume is not a fs file system, there is nothing for us to do here.
+	if !fsVolume {
+		return true, nil
+	}
+	_, err = util.GenericResizeFS(plugin.host, plugin.GetPluginName(), resizeOptions.DevicePath, resizeOptions.DeviceMountPath)
 	if err != nil {
 		return false, err
 	}

pkg/volume/azure_dd/azure_dd.go

@@ -298,7 +298,15 @@ func (plugin *azureDataDiskPlugin) ExpandVolumeDevice(
 }
 
 func (plugin *azureDataDiskPlugin) NodeExpand(resizeOptions volume.NodeResizeOptions) (bool, error) {
-	_, err := util.GenericResizeFS(plugin.host, plugin.GetPluginName(), resizeOptions.DevicePath, resizeOptions.DeviceMountPath)
+	fsVolume, err := util.CheckVolumeModeFilesystem(resizeOptions.VolumeSpec)
+	if err != nil {
+		return false, fmt.Errorf("error checking VolumeMode: %v", err)
+	}
+	// if volume is not a fs file system, there is nothing for us to do here.
+	if !fsVolume {
+		return true, nil
+	}
+	_, err = util.GenericResizeFS(plugin.host, plugin.GetPluginName(), resizeOptions.DevicePath, resizeOptions.DeviceMountPath)
 	if err != nil {
 		return false, err
 	}

pkg/volume/cinder/cinder.go

@@ -311,7 +311,16 @@ func (plugin *cinderPlugin) ExpandVolumeDevice(spec *volume.Spec, newSize resour
 }
 
 func (plugin *cinderPlugin) NodeExpand(resizeOptions volume.NodeResizeOptions) (bool, error) {
-	_, err := util.GenericResizeFS(plugin.host, plugin.GetPluginName(), resizeOptions.DevicePath, resizeOptions.DeviceMountPath)
+	fsVolume, err := util.CheckVolumeModeFilesystem(resizeOptions.VolumeSpec)
+	if err != nil {
+		return false, fmt.Errorf("error checking VolumeMode: %v", err)
+	}
+	// if volume is not a fs file system, there is nothing for us to do here.
+	if !fsVolume {
+		return true, nil
+	}
+
+	_, err = util.GenericResizeFS(plugin.host, plugin.GetPluginName(), resizeOptions.DevicePath, resizeOptions.DeviceMountPath)
 	if err != nil {
 		return false, err
 	}

pkg/volume/flexvolume/expander-defaults.go

@@ -17,6 +17,8 @@ limitations under the License.
 package flexvolume
 
 import (
+	"fmt"
+
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/volume"
@@ -40,7 +42,16 @@ func (e *expanderDefaults) ExpandVolumeDevice(spec *volume.Spec, newSize resourc
 // generic filesystem resize
 func (e *expanderDefaults) NodeExpand(rsOpt volume.NodeResizeOptions) (bool, error) {
 	klog.Warning(logPrefix(e.plugin), "using default filesystem resize for volume ", rsOpt.VolumeSpec.Name(), ", at ", rsOpt.DevicePath)
-	_, err := util.GenericResizeFS(e.plugin.host, e.plugin.GetPluginName(), rsOpt.DevicePath, rsOpt.DeviceMountPath)
+	fsVolume, err := util.CheckVolumeModeFilesystem(rsOpt.VolumeSpec)
+	if err != nil {
+		return false, fmt.Errorf("error checking VolumeMode: %v", err)
+	}
+	// if volume is not a fs file system, there is nothing for us to do here.
+	if !fsVolume {
+		return true, nil
+	}
+
+	_, err = util.GenericResizeFS(e.plugin.host, e.plugin.GetPluginName(), rsOpt.DevicePath, rsOpt.DeviceMountPath)
 	if err != nil {
 		return false, err
 	}

pkg/volume/gcepd/gce_pd.go

@@ -280,7 +280,15 @@ func (plugin *gcePersistentDiskPlugin) ExpandVolumeDevice(
 }
 
 func (plugin *gcePersistentDiskPlugin) NodeExpand(resizeOptions volume.NodeResizeOptions) (bool, error) {
-	_, err := util.GenericResizeFS(plugin.host, plugin.GetPluginName(), resizeOptions.DevicePath, resizeOptions.DeviceMountPath)
+	fsVolume, err := util.CheckVolumeModeFilesystem(resizeOptions.VolumeSpec)
+	if err != nil {
+		return false, fmt.Errorf("error checking VolumeMode: %v", err)
+	}
+	// if volume is not a fs file system, there is nothing for us to do here.
+	if !fsVolume {
+		return true, nil
+	}
+	_, err = util.GenericResizeFS(plugin.host, plugin.GetPluginName(), resizeOptions.DevicePath, resizeOptions.DeviceMountPath)
 	if err != nil {
 		return false, err
 	}

pkg/volume/rbd/rbd.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
+	"k8s.io/kubernetes/pkg/volume/util"
 	volutil "k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
 	utilstrings "k8s.io/utils/strings"
@@ -202,7 +203,15 @@ func (plugin *rbdPlugin) ExpandVolumeDevice(spec *volume.Spec, newSize resource.
 }
 
 func (plugin *rbdPlugin) NodeExpand(resizeOptions volume.NodeResizeOptions) (bool, error) {
-	_, err := volutil.GenericResizeFS(plugin.host, plugin.GetPluginName(), resizeOptions.DevicePath, resizeOptions.DeviceMountPath)
+	fsVolume, err := util.CheckVolumeModeFilesystem(resizeOptions.VolumeSpec)
+	if err != nil {
+		return false, fmt.Errorf("error checking VolumeMode: %v", err)
+	}
+	// if volume is not a fs file system, there is nothing for us to do here.
+	if !fsVolume {
+		return true, nil
+	}
+	_, err = volutil.GenericResizeFS(plugin.host, plugin.GetPluginName(), resizeOptions.DevicePath, resizeOptions.DeviceMountPath)
 	if err != nil {
 		return false, err
 	}

pkg/volume/util/operationexecutor/BUILD

@@ -12,6 +12,7 @@ go_library(
         "fakegenerator.go",
         "operation_executor.go",
         "operation_generator.go",
+        "volume_operation.go",
     ],
     importpath = "k8s.io/kubernetes/pkg/volume/util/operationexecutor",
     deps = [