Skip to content

Add support for raw block devices and enable block device tests on external k8s testing #283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions pkg/gce-pd-csi-driver/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ func TestCreateVolumeArguments(t *testing.T) {
},
},
{
name: "fail with block volume capability",
name: "success with block volume capability",
req: &csi.CreateVolumeRequest{
Name: name,
CapacityRange: stdCapRange,
Expand All @@ -632,7 +632,12 @@ func TestCreateVolumeArguments(t *testing.T) {
},
},
},
expErrCode: codes.InvalidArgument,
expVol: &csi.Volume{
CapacityBytes: common.GbToBytes(20),
VolumeId: testVolumeId,
VolumeContext: nil,
AccessibleTopology: stdTopology,
},
},
{
name: "fail with both mount and block volume capability",
Expand Down
94 changes: 74 additions & 20 deletions pkg/gce-pd-csi-driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,60 @@ func (ns *GCENodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePub
return &csi.NodePublishVolumeResponse{}, nil
}

if err := ns.Mounter.Interface.MakeDir(targetPath); err != nil {
glog.Errorf("mkdir failed on disk %s (%v)", targetPath, err)
return nil, err
}

// Perform a bind mount to the full path to allow duplicate mounts of the same PD.
fstype := ""
sourcePath := ""
options := []string{"bind"}
if readOnly {
options = append(options, "ro")
}

err = ns.Mounter.Interface.Mount(stagingTargetPath, targetPath, "ext4", options)
if mnt := volumeCapability.GetMount(); mnt != nil {
if mnt.FsType != "" {
fstype = mnt.FsType
} else {
// Default fstype is ext4
fstype = "ext4"
}

glog.V(4).Infof("NodePublishVolume with filesystem %s", fstype)

for _, flag := range mnt.MountFlags {
options = append(options, flag)
}

sourcePath = stagingTargetPath

if err := ns.Mounter.Interface.MakeDir(targetPath); err != nil {
glog.Errorf("mkdir failed on disk %s (%v)", targetPath, err)
return nil, err
}
} else if blk := volumeCapability.GetBlock(); blk != nil {
glog.V(4).Infof("NodePublishVolume with block volume mode")

partition := ""
if part, ok := req.GetVolumeContext()[common.VolumeAttributePartition]; ok {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to support partitions with block? I thought partitions was sort of broken already and we only wanted to keep it for backwards compatibility

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only broken part is specifying two partitions of the same disk in a pod: kubernetes/kubernetes#57915

Our fs mount supports partitions as much as in-tree supports them (and will continue to have to for migration). If the in-tree block supports partitions we will have to support them for the driver as well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if it does. We should add test cases if we do...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

partition = part
}

sourcePath, err = ns.getDevicePath(volumeID, partition)
if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("Error when getting device path: %v", err))
}

// Expose block volume as file at target path
err = ns.Mounter.MakeFile(targetPath)
if err != nil {
if removeErr := os.Remove(targetPath); removeErr != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("Error removing block file at target path %v: %v, mounti error: %v", targetPath, removeErr, err))
}
return nil, status.Error(codes.Internal, fmt.Sprintf("Failed to create block file at target path %v: %v", targetPath, err))
}
} else {
return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("NodePublishVolume volume capability must specify either mount or block mode"))
}

err = ns.Mounter.Interface.Mount(sourcePath, targetPath, fstype, options)
if err != nil {
notMnt, mntErr := ns.Mounter.Interface.IsLikelyNotMountPoint(targetPath)
if mntErr != nil {
Expand Down Expand Up @@ -197,19 +239,9 @@ func (ns *GCENodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStage
partition = part
}

deviceName, err := common.GetDeviceName(volumeKey)
devicePath, err := ns.getDevicePath(volumeID, partition)
if err != nil {
status.Error(codes.Internal, fmt.Sprintf("error getting device name: %v", err))
}

devicePaths := ns.DeviceUtils.GetDiskByIdPaths(deviceName, partition)
devicePath, err := ns.DeviceUtils.VerifyDevicePath(devicePaths)

if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("Error verifying GCE PD (%q) is attached: %v", volumeKey.Name, err))
}
if devicePath == "" {
return nil, status.Error(codes.Internal, fmt.Sprintf("Unable to find device path out of attempted paths: %v", devicePaths))
return nil, status.Error(codes.Internal, fmt.Sprintf("Error when getting device path: %v", err))
}

glog.V(4).Infof("Successfully found attached GCE PD %q at device path %s.", volumeKey.Name, devicePath)
Expand Down Expand Up @@ -251,8 +283,8 @@ func (ns *GCENodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStage
options = append(options, flag)
}
} else if blk := volumeCapability.GetBlock(); blk != nil {
// TODO(#64): Block volume support
return nil, status.Error(codes.Unimplemented, fmt.Sprintf("Block volume support is not yet implemented"))
// Noop for Block NodeStageVolume
return &csi.NodeStageVolumeResponse{}, nil
}

err = ns.Mounter.FormatAndMount(devicePath, stagingTargetPath, fstype, options)
Expand Down Expand Up @@ -333,3 +365,25 @@ func (ns *GCENodeServer) GetVolumeLimits() (int64, error) {
}
return volumeLimits, nil
}

func (ns *GCENodeServer) getDevicePath(volumeID string, partition string) (string, error) {
volumeKey, err := common.VolumeIDToKey(volumeID)
if err != nil {
return "", err
}
deviceName, err := common.GetDeviceName(volumeKey)
if err != nil {
return "", fmt.Errorf("error getting device name: %v", err)
}

devicePaths := ns.DeviceUtils.GetDiskByIdPaths(deviceName, partition)
devicePath, err := ns.DeviceUtils.VerifyDevicePath(devicePaths)

if err != nil {
return "", fmt.Errorf("error verifying GCE PD (%q) is attached: %v", volumeKey.Name, err)
}
if devicePath == "" {
return "", fmt.Errorf("unable to find device path out of attempted paths: %v", devicePaths)
}
return devicePath, nil
}
27 changes: 21 additions & 6 deletions pkg/gce-pd-csi-driver/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,28 +64,43 @@ func logGRPC(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, h
}

func validateVolumeCapabilities(vcs []*csi.VolumeCapability) error {
isMnt := false
isBlk := false

if vcs == nil {
return errors.New("volume capabilities is nil")
}

for _, vc := range vcs {
if err := validateVolumeCapability(vc); err != nil {
return err
}
if blk := vc.GetBlock(); blk != nil {
isBlk = true
}
if mnt := vc.GetMount(); mnt != nil {
isMnt = true
}
}

if isBlk && isMnt {
return errors.New("both mount and block volume capabilities specified")
}

return nil
}

func validateVolumeCapability(vc *csi.VolumeCapability) error {
if err := validateAccessMode(vc.GetAccessMode()); err != nil {
return err
}
if blk := vc.GetBlock(); blk != nil {
// TODO(#64): Block volume support
return errors.New("Block volume support is not yet implemented")
blk := vc.GetBlock()
mnt := vc.GetMount()
if mnt == nil && blk == nil {
return errors.New("must specify an access type")
}
if mnt := vc.GetMount(); mnt == nil {
// TODO(#64): Change error message after block volume support
return errors.New("Must specify an access type of Mount")
if mnt != nil && blk != nil {
return errors.New("specified both mount and block access types")
}
return nil
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/gce-pd-csi-driver/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestValidateVolumeCapabilities(t *testing.T) {
expErr: true,
},
{
name: "fail with block capabilities",
name: "success with block capabilities",
vc: []*csi.VolumeCapability{
{
AccessType: &csi.VolumeCapability_Block{
Expand All @@ -137,7 +137,6 @@ func TestValidateVolumeCapabilities(t *testing.T) {
},
},
},
expErr: true,
},
{
name: "success with reader + writer capabilities",
Expand Down
2 changes: 1 addition & 1 deletion test/k8s-integration/config/test-config-template.in
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ DriverInfo:
multipods: true
fsGroup: true
exec: true
# block: true
block: true
# dataSource: true
# RWX: true