Merge pull request #65032 from vladimirvivien/csi-block-path-fix
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. CSI block map file path fix **What this PR does / why we need it**: This PR is a bug fix that addresses the way CSI communicates block volume path. Instead of sending a directory to the external CSI driver, this PR fixes it to send path to a pre-existing file used for block mapping. **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes #64854 **Special notes for your reviewer**: /kind bug **Release note**: ```release-note NONE ```
This commit is contained in:
		| @@ -47,19 +47,20 @@ type csiBlockMapper struct { | ||||
|  | ||||
| var _ volume.BlockVolumeMapper = &csiBlockMapper{} | ||||
|  | ||||
| // GetGlobalMapPath returns a path (on the node) where the devicePath will be symlinked to | ||||
| // Example: plugins/kubernetes.io/csi/volumeDevices/{volumeID} | ||||
| // GetGlobalMapPath returns a path (on the node) to a device file which will be symlinked to | ||||
| // Example: plugins/kubernetes.io/csi/volumeDevices/{volumeID}/dev | ||||
| func (m *csiBlockMapper) GetGlobalMapPath(spec *volume.Spec) (string, error) { | ||||
| 	dir := getVolumeDevicePluginDir(spec.Name(), m.plugin.host) | ||||
| 	glog.V(4).Infof(log("blockMapper.GetGlobalMapPath = %s", dir)) | ||||
| 	return dir, nil | ||||
| } | ||||
|  | ||||
| // GetPodDeviceMapPath returns pod's device map path and volume name | ||||
| // path: pods/{podUid}/volumeDevices/kubernetes.io~csi/, {volumeID} | ||||
| // GetPodDeviceMapPath returns pod's device file which will be mapped to a volume | ||||
| // returns: pods/{podUid}/volumeDevices/kubernetes.io~csi/{volumeID}/dev, {volumeID} | ||||
| func (m *csiBlockMapper) GetPodDeviceMapPath() (string, string) { | ||||
| 	path, specName := m.plugin.host.GetPodVolumeDeviceDir(m.podUID, csiPluginName), m.specName | ||||
| 	glog.V(4).Infof(log("blockMapper.GetPodDeviceMapPath = %s", path)) | ||||
| 	path := filepath.Join(m.plugin.host.GetPodVolumeDeviceDir(m.podUID, csiPluginName), m.specName, "dev") | ||||
| 	specName := m.specName | ||||
| 	glog.V(4).Infof(log("blockMapper.GetPodDeviceMapPath [path=%s; name=%s]", path, specName)) | ||||
| 	return path, specName | ||||
| } | ||||
|  | ||||
| @@ -87,6 +88,9 @@ func (m *csiBlockMapper) SetUpDevice() (string, error) { | ||||
| 		return "", err | ||||
| 	} | ||||
|  | ||||
| 	globalMapPathBlockFile := filepath.Join(globalMapPath, "file") | ||||
| 	glog.V(4).Infof(log("blockMapper.SetupDevice global device map path file set [%s]", globalMapPathBlockFile)) | ||||
|  | ||||
| 	csi := m.csiClient | ||||
| 	ctx, cancel := context.WithTimeout(context.Background(), csiTimeout) | ||||
| 	defer cancel() | ||||
| @@ -128,13 +132,25 @@ func (m *csiBlockMapper) SetUpDevice() (string, error) { | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	// create globalMapPath before call to NodeStageVolume | ||||
| 	// setup path globalMapPath and block file before call to NodeStageVolume | ||||
| 	if err := os.MkdirAll(globalMapPath, 0750); err != nil { | ||||
| 		glog.Error(log("blockMapper.SetupDevice failed to create dir %s: %v", globalMapPath, err)) | ||||
| 		return "", err | ||||
| 	} | ||||
| 	glog.V(4).Info(log("blockMapper.SetupDevice created global device map path successfully [%s]", globalMapPath)) | ||||
|  | ||||
| 	// create block device file | ||||
| 	blockFile, err := os.OpenFile(globalMapPathBlockFile, os.O_CREATE|os.O_RDWR, 0750) | ||||
| 	if err != nil { | ||||
| 		glog.Error(log("blockMapper.SetupDevice failed to create dir %s: %v", globalMapPathBlockFile, err)) | ||||
| 		return "", err | ||||
| 	} | ||||
| 	if err := blockFile.Close(); err != nil { | ||||
| 		glog.Error(log("blockMapper.SetupDevice failed to close file %s: %v", globalMapPathBlockFile, err)) | ||||
| 		return "", err | ||||
| 	} | ||||
| 	glog.V(4).Info(log("blockMapper.SetupDevice created global map path block device file successfully [%s]", globalMapPathBlockFile)) | ||||
|  | ||||
| 	//TODO (vladimirvivien) implement better AccessModes mapping between k8s and CSI | ||||
| 	accessMode := v1.ReadWriteOnce | ||||
| 	if m.spec.PersistentVolume.Spec.AccessModes != nil { | ||||
| @@ -144,7 +160,7 @@ func (m *csiBlockMapper) SetUpDevice() (string, error) { | ||||
| 	err = csi.NodeStageVolume(ctx, | ||||
| 		csiSource.VolumeHandle, | ||||
| 		publishVolumeInfo, | ||||
| 		globalMapPath, | ||||
| 		globalMapPathBlockFile, | ||||
| 		fsTypeBlockName, | ||||
| 		accessMode, | ||||
| 		nodeStageSecrets, | ||||
| @@ -158,8 +174,8 @@ func (m *csiBlockMapper) SetUpDevice() (string, error) { | ||||
| 		return "", err | ||||
| 	} | ||||
|  | ||||
| 	glog.V(4).Infof(log("blockMapper.SetupDevice successfully requested NodeStageVolume [%s]", globalMapPath)) | ||||
| 	return globalMapPath, nil | ||||
| 	glog.V(4).Infof(log("blockMapper.SetupDevice successfully requested NodeStageVolume [%s]", globalMapPathBlockFile)) | ||||
| 	return globalMapPathBlockFile, nil | ||||
| } | ||||
|  | ||||
| func (m *csiBlockMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, volumeMapName string, podUID types.UID) error { | ||||
| @@ -176,16 +192,29 @@ func (m *csiBlockMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, vol | ||||
|  | ||||
| 	csiSource, err := getCSISourceFromSpec(m.spec) | ||||
| 	if err != nil { | ||||
| 		glog.Error(log("blockMapper.Map failed to get CSI persistent source: %v", err)) | ||||
| 		glog.Error(log("blockMapper.MapDevice failed to get CSI persistent source: %v", err)) | ||||
| 		return err | ||||
| 	} | ||||
|  | ||||
| 	dir := filepath.Join(volumeMapPath, volumeMapName) | ||||
| 	csi := m.csiClient | ||||
|  | ||||
| 	ctx, cancel := context.WithTimeout(context.Background(), csiTimeout) | ||||
| 	defer cancel() | ||||
|  | ||||
| 	globalMapPathBlockFile := devicePath | ||||
| 	dir, _ := m.GetPodDeviceMapPath() | ||||
| 	targetBlockFilePath := filepath.Join(dir, "file") | ||||
| 	glog.V(4).Infof(log("blockMapper.MapDevice target volume map file path %s", targetBlockFilePath)) | ||||
|  | ||||
| 	stageCapable, err := hasStageUnstageCapability(ctx, csi) | ||||
| 	if err != nil { | ||||
| 		glog.Error(log("blockMapper.MapDevice failed to check for STAGE_UNSTAGE_VOLUME capabilty: %v", err)) | ||||
| 		return err | ||||
| 	} | ||||
|  | ||||
| 	if !stageCapable { | ||||
| 		globalMapPathBlockFile = "" | ||||
| 	} | ||||
|  | ||||
| 	nodeName := string(m.plugin.host.GetNodeName()) | ||||
| 	attachID := getAttachmentName(csiSource.VolumeHandle, csiSource.Driver, nodeName) | ||||
|  | ||||
| @@ -213,10 +242,22 @@ func (m *csiBlockMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, vol | ||||
| 	} | ||||
|  | ||||
| 	if err := os.MkdirAll(dir, 0750); err != nil { | ||||
| 		glog.Error(log("blockMapper.MapDevice failed to create dir %#v:  %v", dir, err)) | ||||
| 		glog.Error(log("blockMapper.MapDevice failed to create dir %s:  %v", dir, err)) | ||||
| 		return err | ||||
| 	} | ||||
| 	glog.V(4).Info(log("blockMapper.MapDevice created NodePublish path [%s]", dir)) | ||||
| 	glog.V(4).Info(log("blockMapper.MapDevice created target volume map path successfully [%s]", dir)) | ||||
|  | ||||
| 	// create target map volume block file | ||||
| 	targetBlockFile, err := os.OpenFile(targetBlockFilePath, os.O_CREATE|os.O_RDWR, 0750) | ||||
| 	if err != nil { | ||||
| 		glog.Error(log("blockMapper.MapDevice failed to create file %s: %v", targetBlockFilePath, err)) | ||||
| 		return err | ||||
| 	} | ||||
| 	if err := targetBlockFile.Close(); err != nil { | ||||
| 		glog.Error(log("blockMapper.MapDevice failed to close file %s: %v", targetBlockFilePath, err)) | ||||
| 		return err | ||||
| 	} | ||||
| 	glog.V(4).Info(log("blockMapper.MapDevice created target volume map file successfully [%s]", targetBlockFilePath)) | ||||
|  | ||||
| 	//TODO (vladimirvivien) implement better AccessModes mapping between k8s and CSI | ||||
| 	accessMode := v1.ReadWriteOnce | ||||
| @@ -228,8 +269,8 @@ func (m *csiBlockMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, vol | ||||
| 		ctx, | ||||
| 		m.volumeID, | ||||
| 		m.readOnly, | ||||
| 		globalMapPath, | ||||
| 		dir, | ||||
| 		globalMapPathBlockFile, | ||||
| 		targetBlockFilePath, | ||||
| 		accessMode, | ||||
| 		publishVolumeInfo, | ||||
| 		csiSource.VolumeAttributes, | ||||
| @@ -240,7 +281,7 @@ func (m *csiBlockMapper) MapDevice(devicePath, globalMapPath, volumeMapPath, vol | ||||
| 	if err != nil { | ||||
| 		glog.Errorf(log("blockMapper.MapDevice failed: %v", err)) | ||||
| 		if err := os.RemoveAll(dir); err != nil { | ||||
| 			glog.Error(log("blockMapper.MapDevice failed to remove mount dir after a NodePublish() error [%s]: %v", dir, err)) | ||||
| 			glog.Error(log("blockMapper.MapDevice failed to remove mapped dir after a NodePublish() error [%s]: %v", dir, err)) | ||||
| 		} | ||||
| 		return err | ||||
| 	} | ||||
|   | ||||
| @@ -123,7 +123,7 @@ func TestBlockMapperSetupDevice(t *testing.T) { | ||||
| 		t.Fatalf("mapper failed to GetGlobalMapPath: %v", err) | ||||
| 	} | ||||
|  | ||||
| 	if devicePath != globalMapPath { | ||||
| 	if devicePath != filepath.Join(globalMapPath, "file") { | ||||
| 		t.Fatalf("mapper.SetupDevice returned unexpected path %s instead of %v", devicePath, globalMapPath) | ||||
| 	} | ||||
|  | ||||
| @@ -186,16 +186,17 @@ func TestBlockMapperMapDevice(t *testing.T) { | ||||
| 		t.Fatalf("mapper failed to GetGlobalMapPath: %v", err) | ||||
| 	} | ||||
|  | ||||
| 	if _, err := os.Stat(filepath.Join(volumeMapPath, volName)); err != nil { | ||||
| 	podVolumeBlockFilePath := filepath.Join(volumeMapPath, "file") | ||||
| 	if _, err := os.Stat(podVolumeBlockFilePath); err != nil { | ||||
| 		if os.IsNotExist(err) { | ||||
| 			t.Errorf("mapper.MapDevice failed, volume path not created: %s", volumeMapPath) | ||||
| 			t.Errorf("mapper.MapDevice failed, volume path not created: %v", err) | ||||
| 		} else { | ||||
| 			t.Errorf("mapper.MapDevice failed: %v", err) | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	pubs := csiMapper.csiClient.(*fakeCsiDriverClient).nodeClient.GetNodePublishedVolumes() | ||||
| 	if pubs[csiMapper.volumeID] != volumeMapPath { | ||||
| 	if pubs[csiMapper.volumeID] != podVolumeBlockFilePath { | ||||
| 		t.Error("csi server may not have received NodePublishVolume call") | ||||
| 	} | ||||
| } | ||||
|   | ||||
| @@ -346,6 +346,7 @@ func (p *csiPlugin) NewBlockVolumeMapper(spec *volume.Spec, podRef *api.Pod, opt | ||||
| 		driverName: pvSource.Driver, | ||||
| 		readOnly:   readOnly, | ||||
| 		spec:       spec, | ||||
| 		specName:   spec.Name(), | ||||
| 		podUID:     podRef.UID, | ||||
| 	} | ||||
|  | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Kubernetes Submit Queue
					Kubernetes Submit Queue