Skip to content

Commit cd62ccd

Browse files
davidz627davidz627
committed
Validate that existing mount point is compatible with volume and capabilities when it already exists for Stage and Publish
1 parent 2a39852 commit cd62ccd

File tree

2 files changed

+215
-17
lines changed

2 files changed

+215
-17
lines changed

‎pkg/gce-pd-csi-driver/node.go

+72-17
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@ limitations under the License.
1515
package gceGCEDriver
1616

1717
import (
18+
"errors"
1819
"fmt"
1920
"os"
21+
"path/filepath"
2022
"strconv"
2123
"strings"
2224

@@ -59,6 +61,8 @@ var _ csi.NodeServer = &GCENodeServer{}
5961
const (
6062
volumeLimitSmall int64 = 15
6163
volumeLimitBig int64 = 127
64+
65+
defaultFSType string = "ext4"
6266
)
6367

6468
func (ns *GCENodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) {
@@ -95,13 +99,20 @@ func (ns *GCENodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePub
9599
return nil, status.Error(codes.Internal, fmt.Sprintf("cannot validate mount point: %s %v", targetPath, err))
96100
}
97101
if !notMnt {
98-
// TODO(#95): check if mount is compatible. Return OK if it is, or appropriate error.
99-
/*
100-
1) Target Path MUST be the vol referenced by vol ID
101-
2) TODO(#253): Check volume capability matches for ALREADY_EXISTS
102-
3) Readonly MUST match
102+
// Validate that the existing volume is compatible
103+
partition := ""
104+
if part, ok := req.GetVolumeContext()[common.VolumeAttributePartition]; ok {
105+
partition = part
106+
}
107+
devicePath, err := ns.getDevicePath(volumeID, partition)
108+
if err != nil {
109+
return nil, status.Error(codes.Internal, fmt.Sprintf("Error when getting device path: %v", err))
110+
}
111+
112+
if err := ns.volumeCompatible(targetPath, devicePath, volumeCapability); err != nil {
113+
return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("Mount point %s already exists but found to be incompatible: %v", stagingTargetPath, err))
114+
}
103115

104-
*/
105116
klog.V(4).Infof("NodePublishVolume succeeded on volume %v to %s, mount already exists.", volumeID, targetPath)
106117
return &csi.NodePublishVolumeResponse{}, nil
107118
}
@@ -118,8 +129,7 @@ func (ns *GCENodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePub
118129
if mnt.FsType != "" {
119130
fstype = mnt.FsType
120131
} else {
121-
// Default fstype is ext4
122-
fstype = "ext4"
132+
fstype = defaultFSType
123133
}
124134

125135
klog.V(4).Infof("NodePublishVolume with filesystem %s", fstype)
@@ -286,22 +296,18 @@ func (ns *GCENodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStage
286296
}
287297

288298
if !notMnt {
289-
// TODO(#95): Check who is mounted here. No error if its us
290-
/*
291-
1) Target Path MUST be the vol referenced by vol ID
292-
2) VolumeCapability MUST match
293-
3) Readonly MUST match
294-
295-
*/
299+
// Validate that the existing volume is compatible
300+
if err := ns.volumeCompatible(stagingTargetPath, devicePath, volumeCapability); err != nil {
301+
return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("Mount point %s already exists but found to be incompatible: %v", stagingTargetPath, err))
302+
}
296303

297304
klog.V(4).Infof("NodeStageVolume succeded on %v to %s, mount already exists.", volumeID, stagingTargetPath)
298305
return &csi.NodeStageVolumeResponse{}, nil
299306

300307
}
301308

302309
// Part 3: Mount device to stagingTargetPath
303-
// Default fstype is ext4
304-
fstype := "ext4"
310+
fstype := defaultFSType
305311
options := []string{}
306312
if mnt := volumeCapability.GetMount(); mnt != nil {
307313
if mnt.FsType != "" {
@@ -327,6 +333,55 @@ func (ns *GCENodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStage
327333
return &csi.NodeStageVolumeResponse{}, nil
328334
}
329335

336+
func (ns *GCENodeServer) volumeCompatible(mountedPath, devicePath string, volumeCapability *csi.VolumeCapability) error {
337+
// Part 1: Check that volume mounted at mountedPath is the same as one at
338+
// devicePath
339+
devicePathDev, err := filepath.EvalSymlinks(devicePath)
340+
if err != nil {
341+
return fmt.Errorf("failed to find backing disk for devicePath %s: %v", devicePath, err)
342+
}
343+
// df -P returns with the rows containing the backing disk and the location
344+
// of the mount. awk proccesses the result by getting the line with the
345+
// location of the mount we're looking for and printing out the backing
346+
// disk. the resulting output should be a path to a device such as
347+
// "/dev/sda"
348+
mountedPathDevBytes, err := ns.Mounter.Exec.Command("sh", "-c", fmt.Sprintf("df -P | awk '$6==\"%s\"{print $1}'", mountedPath)).CombinedOutput()
349+
if err != nil {
350+
return fmt.Errorf("failed to find backing disk for mountedPath %s: %s: %v", mountedPath, string(mountedPathDevBytes), err)
351+
}
352+
mountedPathDev := strings.TrimSpace(string(mountedPathDevBytes))
353+
if devicePathDev != mountedPathDev {
354+
return fmt.Errorf("devices at paths %s and %s were not the same, got %s %s respectively", devicePath, mountedPath, devicePathDev, mountedPathDev)
355+
}
356+
357+
// Part 2: Check volumeCapability format compatibility
358+
if blk := volumeCapability.GetBlock(); blk != nil {
359+
// If the volume capability request is type "Block" we don't care what
360+
// format the disk is in because user could have re-formatted to a different
361+
// type
362+
return nil
363+
}
364+
365+
format, err := ns.Mounter.GetDiskFormat(devicePath)
366+
if err != nil {
367+
return fmt.Errorf("failed to get the format of disk %s: %v", devicePath, err)
368+
}
369+
370+
mnt := volumeCapability.GetMount()
371+
if mnt == nil {
372+
return errors.New("block and mount capabilities are nil, invalid volume capability")
373+
}
374+
375+
wantFmt := mnt.FsType
376+
if wantFmt == "" {
377+
wantFmt = defaultFSType
378+
}
379+
if mnt.FsType != format {
380+
return fmt.Errorf("device at %s has format %s but volume capability requires %s", devicePath, format, mnt.FsType)
381+
}
382+
return nil
383+
}
384+
330385
func (ns *GCENodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVolumeRequest) (*csi.NodeUnstageVolumeResponse, error) {
331386
// Validate arguments
332387
volumeID := req.GetVolumeId()

‎test/e2e/tests/single_zone_e2e_test.go

+143
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ import (
3737
"google.golang.org/api/iterator"
3838
kmspb "google.golang.org/genproto/googleapis/cloud/kms/v1"
3939
fieldmask "google.golang.org/genproto/protobuf/field_mask"
40+
"google.golang.org/grpc/codes"
41+
"google.golang.org/grpc/status"
4042
)
4143

4244
const (
@@ -159,6 +161,147 @@ var _ = Describe("GCE PD CSI Driver", func() {
159161
}()
160162
})
161163

164+
It("Should fail validation if the same disk with different capabilities is staged/published to the same path", func() {
165+
testContext := getRandomTestContext()
166+
167+
p, z, _ := testContext.Instance.GetIdentity()
168+
client := testContext.Client
169+
instance := testContext.Instance
170+
171+
// Setup: Create Disk A
172+
volNameA, volIDA := createAndValidateUniqueZonalDisk(client, p, z)
173+
defer func() {
174+
// Delete Disk
175+
err := client.DeleteVolume(volIDA)
176+
Expect(err).To(BeNil(), "DeleteVolume failed")
177+
178+
// Validate Disk Deleted
179+
_, err = computeService.Disks.Get(p, z, volNameA).Do()
180+
Expect(gce.IsGCEError(err, "notFound")).To(BeTrue(), "Expected disk to not be found")
181+
}()
182+
183+
// Setup: Attach Disk A
184+
err := client.ControllerPublishVolume(volIDA, instance.GetNodeID())
185+
Expect(err).To(BeNil(), "ControllerPublishVolume failed with error for disk %v on node %v: %v", volIDA, instance.GetNodeID())
186+
defer func() {
187+
// Detach Disk
188+
err = client.ControllerUnpublishVolume(volIDA, instance.GetNodeID())
189+
if err != nil {
190+
klog.Errorf("Failed to detach disk: %v", err)
191+
}
192+
}()
193+
194+
// Setup: Create Disk B
195+
volNameB, volIDB := createAndValidateUniqueZonalDisk(client, p, z)
196+
defer func() {
197+
// Delete Disk
198+
err := client.DeleteVolume(volIDB)
199+
Expect(err).To(BeNil(), "DeleteVolume failed")
200+
201+
// Validate Disk Deleted
202+
_, err = computeService.Disks.Get(p, z, volNameB).Do()
203+
Expect(gce.IsGCEError(err, "notFound")).To(BeTrue(), "Expected disk to not be found")
204+
}()
205+
206+
// Setup: Attach Disk B
207+
err = client.ControllerPublishVolume(volIDB, instance.GetNodeID())
208+
Expect(err).To(BeNil(), "ControllerPublishVolume failed with error for disk %v on node %v: %v", volIDB, instance.GetNodeID())
209+
defer func() {
210+
// Detach Disk
211+
err = client.ControllerUnpublishVolume(volIDB, instance.GetNodeID())
212+
if err != nil {
213+
klog.Errorf("Failed to detach disk: %v", err)
214+
}
215+
}()
216+
217+
// Setup: Stage Disk A
218+
stageDirA := filepath.Join("/tmp/", volNameA, "stage")
219+
err = client.NodeStageExt4Volume(volIDA, stageDirA)
220+
Expect(err).To(BeNil(), "failed to stage volume")
221+
222+
// Assert: Stage to same location with different fstype should fail
223+
err = client.NodeStageVolume(volIDA, stageDirA, &csi.VolumeCapability{
224+
AccessType: &csi.VolumeCapability_Mount{
225+
Mount: &csi.VolumeCapability_MountVolume{
226+
FsType: "xfs",
227+
},
228+
},
229+
AccessMode: &csi.VolumeCapability_AccessMode{
230+
Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
231+
},
232+
})
233+
e, ok := status.FromError(err)
234+
Expect(ok).To(BeTrue(), "Could not get error type from err", err)
235+
Expect(e.Code()).To(Equal(codes.AlreadyExists), "Volume staged with different fs type should result in already exists error")
236+
237+
// Assert: Stage to same location with same fstype should work
238+
err = client.NodeStageVolume(volIDA, stageDirA, &csi.VolumeCapability{
239+
AccessType: &csi.VolumeCapability_Mount{
240+
Mount: &csi.VolumeCapability_MountVolume{
241+
FsType: "ext4",
242+
},
243+
},
244+
AccessMode: &csi.VolumeCapability_AccessMode{
245+
Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
246+
},
247+
})
248+
Expect(err).To(BeNil(), "Staged volume to same location with same fs type should work")
249+
250+
// Assert: Stage volume using block to location with fs type already should always work
251+
err = client.NodeStageBlockVolume(volIDA, stageDirA)
252+
Expect(err).To(BeNil(), "Staged volume of block type should always work")
253+
254+
defer func() {
255+
// Unstage Disk
256+
err = client.NodeUnstageVolume(volIDA, stageDirA)
257+
if err != nil {
258+
klog.Errorf("Failed to unstage volume: %v", err)
259+
}
260+
fp := filepath.Join("/tmp/", volNameA)
261+
err = testutils.RmAll(instance, fp)
262+
if err != nil {
263+
klog.Errorf("Failed to rm file path %s: %v", fp, err)
264+
}
265+
}()
266+
267+
// Assert: Stage Disk B to Disk A position and fail even both as EXT4
268+
err = client.NodeStageExt4Volume(volIDB, stageDirA)
269+
e, ok = status.FromError(err)
270+
Expect(ok).To(BeTrue(), "Could not get error type from err", err)
271+
Expect(e.Code()).To(Equal(codes.AlreadyExists), "Volume B staged with same fs type to Volume A staging path should result in already exists error")
272+
273+
// Setup: Stage Disk B with EXT3
274+
stageDirB := filepath.Join("/tmp/", volNameB, "stage")
275+
err = client.NodeStageVolume(volIDB, stageDirB, &csi.VolumeCapability{
276+
AccessType: &csi.VolumeCapability_Mount{
277+
Mount: &csi.VolumeCapability_MountVolume{
278+
FsType: "ext3",
279+
},
280+
},
281+
AccessMode: &csi.VolumeCapability_AccessMode{
282+
Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
283+
},
284+
})
285+
Expect(err).To(BeNil(), "failed to stage volume")
286+
287+
// Setup: Publish A to publishDirA
288+
publishDirA := filepath.Join("/tmp/", volNameA, "mount")
289+
err = client.NodePublishVolume(volIDA, stageDirA, publishDirA)
290+
Expect(err).To(BeNil(), "failed to publish volume")
291+
defer func() {
292+
err = client.NodeUnpublishVolume(volIDA, publishDirA)
293+
Expect(err).To(BeNil(), "failed to unpublish volume")
294+
}()
295+
// Assert: Publish A to publishDirA with block which already has an fs should work
296+
err = client.NodePublishBlockVolume(volIDA, stageDirA, publishDirA)
297+
Expect(err).To(BeNil(), "publish block volume to an the existing location with fstype should work")
298+
299+
// Assert: Publish B to publishDirA should fail because disks are different
300+
err = client.NodePublishBlockVolume(volIDB, stageDirB, publishDirA)
301+
Expect(ok).To(BeTrue(), "Could not get error type from err", err)
302+
Expect(e.Code()).To(Equal(codes.AlreadyExists), "Volume B staged with same fs type to Volume A staging path should result in already exists error")
303+
})
304+
162305
It("Should create disks in correct zones when topology is specified", func() {
163306
Expect(testContexts).ToNot(BeEmpty())
164307
testContext := getRandomTestContext()

0 commit comments

Comments
 (0)