From 165b82a44cc5a69870e20654ee758279efd8e174 Mon Sep 17 00:00:00 2001 From: Masaki Kimura Date: Thu, 1 Nov 2018 01:03:03 +0000 Subject: [PATCH 1/6] Add block supports to rbd driver --- .../rbd/kubernetes/csi-provisioner-rbac.yaml | 3 + deploy/rbd/kubernetes/csi-rbdplugin.yaml | 4 + pkg/rbd/nodeserver.go | 93 ++++++++++++------- 3 files changed, 65 insertions(+), 35 deletions(-) diff --git a/deploy/rbd/kubernetes/csi-provisioner-rbac.yaml b/deploy/rbd/kubernetes/csi-provisioner-rbac.yaml index 0c496b422..4eb2510cf 100644 --- a/deploy/rbd/kubernetes/csi-provisioner-rbac.yaml +++ b/deploy/rbd/kubernetes/csi-provisioner-rbac.yaml @@ -24,6 +24,9 @@ rules: - apiGroups: [""] resources: ["events"] verbs: ["list", "watch", "create", "update", "patch"] + - apiGroups: [""] + resources: ["endpoints"] + verbs: ["get", "create", "update"] --- kind: ClusterRoleBinding diff --git a/deploy/rbd/kubernetes/csi-rbdplugin.yaml b/deploy/rbd/kubernetes/csi-rbdplugin.yaml index f15a0c756..264104ac7 100644 --- a/deploy/rbd/kubernetes/csi-rbdplugin.yaml +++ b/deploy/rbd/kubernetes/csi-rbdplugin.yaml @@ -86,6 +86,10 @@ spec: hostPath: path: /var/lib/kubelet/plugins_registry/csi-rbdplugin type: DirectoryOrCreate + - name: plugin-mount-dir + hostPath: + path: /var/lib/kubelet/plugins/kubernetes.io/csi/volumeDevices/ + type: DirectoryOrCreate - name: registration-dir hostPath: path: /var/lib/kubelet/plugins_registry/ diff --git a/pkg/rbd/nodeserver.go b/pkg/rbd/nodeserver.go index 09f959ca1..092754aba 100644 --- a/pkg/rbd/nodeserver.go +++ b/pkg/rbd/nodeserver.go @@ -40,30 +40,53 @@ type nodeServer struct { func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { targetPath := req.GetTargetPath() - - if !strings.HasSuffix(targetPath, "/mount") { - return nil, fmt.Errorf("rnd: malformed the value of target path: %s", targetPath) - } - s := strings.Split(strings.TrimSuffix(targetPath, "/mount"), "/") - volName := s[len(s)-1] - targetPathMutex.LockKey(targetPath) defer targetPathMutex.UnlockKey(targetPath) - notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath) - if err != nil { - if os.IsNotExist(err) { - if err = os.MkdirAll(targetPath, 0750); err != nil { - return nil, status.Error(codes.Internal, err.Error()) + var volName string + isBlock := req.GetVolumeCapability().GetBlock() != nil + + if isBlock { + // Get volName from targetPath + s := strings.Split(targetPath, "/") + volName = s[len(s)-1] + + // Check if that target path exists properly + // targetPath should exists and should be a file + st, err := os.Stat(targetPath) + if err != nil { + if os.IsNotExist(err) { + return nil, status.Error(codes.NotFound, "targetPath not mounted") } - notMnt = true - } else { return nil, status.Error(codes.Internal, err.Error()) } - } + if !st.Mode().IsRegular() { + return nil, status.Error(codes.Internal, "targetPath is not regular file") + } + } else { + // Get volName from targetPath + if !strings.HasSuffix(targetPath, "/mount") { + return nil, fmt.Errorf("rnd: malformed the value of target path: %s", targetPath) + } + s := strings.Split(strings.TrimSuffix(targetPath, "/mount"), "/") + volName = s[len(s)-1] - if !notMnt { - return &csi.NodePublishVolumeResponse{}, nil + // Check if that target path exists properly + notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath) + if err != nil { + if os.IsNotExist(err) { + if err = os.MkdirAll(targetPath, 0750); err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + notMnt = true + } else { + return nil, status.Error(codes.Internal, err.Error()) + } + } + + if !notMnt { + return &csi.NodePublishVolumeResponse{}, nil + } } volOptions, err := getRBDVolumeOptions(req.GetVolumeContext()) if err != nil { @@ -76,23 +99,31 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis return nil, err } glog.V(4).Infof("rbd image: %s/%s was successfully mapped at %s\n", req.GetVolumeId(), volOptions.Pool, devicePath) - fsType := req.GetVolumeCapability().GetMount().GetFsType() + // Publish Path + fsType := req.GetVolumeCapability().GetMount().GetFsType() readOnly := req.GetReadonly() attrib := req.GetVolumeContext() mountFlags := req.GetVolumeCapability().GetMount().GetMountFlags() - glog.V(4).Infof("target %v\nfstype %v\ndevice %v\nreadonly %v\nattributes %v\n mountflags %v\n", - targetPath, fsType, devicePath, readOnly, attrib, mountFlags) - - options := []string{} - if readOnly { - options = append(options, "ro") - } + glog.V(4).Infof("target %v\nisBlock %v\nfstype %v\ndevice %v\nreadonly %v\nattributes %v\n mountflags %v\n", + targetPath, isBlock, fsType, devicePath, readOnly, attrib, mountFlags) diskMounter := &mount.SafeFormatAndMount{Interface: ns.mounter, Exec: mount.NewOsExec()} - if err := diskMounter.FormatAndMount(devicePath, targetPath, fsType, options); err != nil { - return nil, err + if isBlock { + options := []string{"bind"} + if err := diskMounter.Mount(devicePath, targetPath, fsType, options); err != nil { + return nil, err + } + } else { + options := []string{} + if readOnly { + options = append(options, "ro") + } + + if err := diskMounter.FormatAndMount(devicePath, targetPath, fsType, options); err != nil { + return nil, err + } } return &csi.NodePublishVolumeResponse{}, nil @@ -103,14 +134,6 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu targetPathMutex.LockKey(targetPath) defer targetPathMutex.UnlockKey(targetPath) - notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath) - if err != nil { - return nil, status.Error(codes.Internal, err.Error()) - } - if notMnt { - return nil, status.Error(codes.NotFound, "Volume not mounted") - } - devicePath, cnt, err := mount.GetDeviceNameFromMount(ns.mounter, targetPath) if err != nil { return nil, status.Error(codes.Internal, err.Error()) From 5867d495fd40bf67f0157a7793353efa243cc988 Mon Sep 17 00:00:00 2001 From: Masaki Kimura Date: Thu, 15 Nov 2018 02:06:42 +0000 Subject: [PATCH 2/6] Change csi rbd to create/delete targetPath for publish/unpublish --- pkg/rbd/nodeserver.go | 69 ++++++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/pkg/rbd/nodeserver.go b/pkg/rbd/nodeserver.go index 092754aba..069874e38 100644 --- a/pkg/rbd/nodeserver.go +++ b/pkg/rbd/nodeserver.go @@ -50,19 +50,6 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis // Get volName from targetPath s := strings.Split(targetPath, "/") volName = s[len(s)-1] - - // Check if that target path exists properly - // targetPath should exists and should be a file - st, err := os.Stat(targetPath) - if err != nil { - if os.IsNotExist(err) { - return nil, status.Error(codes.NotFound, "targetPath not mounted") - } - return nil, status.Error(codes.Internal, err.Error()) - } - if !st.Mode().IsRegular() { - return nil, status.Error(codes.Internal, "targetPath is not regular file") - } } else { // Get volName from targetPath if !strings.HasSuffix(targetPath, "/mount") { @@ -70,23 +57,39 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis } s := strings.Split(strings.TrimSuffix(targetPath, "/mount"), "/") volName = s[len(s)-1] + } - // Check if that target path exists properly - notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath) - if err != nil { - if os.IsNotExist(err) { + // Check if that target path exists properly + // IsLikelyNotMountPoint doesn't return right result to bind mount of device file + // TODO: Need to fix this to a proper check + notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath) + if err != nil { + if os.IsNotExist(err) { + if isBlock { + // create an empty file + targetPathFile, err := os.OpenFile(targetPath, os.O_CREATE|os.O_RDWR, 0750) + if err != nil { + glog.V(4).Infof("Failed to create targetPath:%s with error: %v", targetPath, err) + return nil, status.Error(codes.Internal, err.Error()) + } + if err := targetPathFile.Close(); err != nil { + glog.V(4).Infof("Failed to close targetPath:%s with error: %v", targetPath, err) + return nil, status.Error(codes.Internal, err.Error()) + } + } else { + // Create a directory if err = os.MkdirAll(targetPath, 0750); err != nil { return nil, status.Error(codes.Internal, err.Error()) } - notMnt = true - } else { - return nil, status.Error(codes.Internal, err.Error()) } + notMnt = true + } else { + return nil, status.Error(codes.Internal, err.Error()) } + } - if !notMnt { - return &csi.NodePublishVolumeResponse{}, nil - } + if !notMnt { + return &csi.NodePublishVolumeResponse{}, nil } volOptions, err := getRBDVolumeOptions(req.GetVolumeContext()) if err != nil { @@ -134,6 +137,19 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu targetPathMutex.LockKey(targetPath) defer targetPathMutex.UnlockKey(targetPath) + // IsLikelyNotMountPoint doesn't return right result to bind mount of device file + // So, just use it to check if file exists, not a mount point + // TODO: Need to fix this to a proper check + _, err := ns.mounter.IsLikelyNotMountPoint(targetPath) + if err != nil { + if os.IsNotExist(err) { + // targetPath has already been deleted + glog.V(4).Infof("targetPath: %s has already been deleted", targetPath) + return &csi.NodeUnpublishVolumeResponse{}, nil + } + return nil, status.Error(codes.NotFound, err.Error()) + } + devicePath, cnt, err := mount.GetDeviceNameFromMount(ns.mounter, targetPath) if err != nil { return nil, status.Error(codes.Internal, err.Error()) @@ -142,6 +158,7 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu // Unmounting the image err = ns.mounter.Unmount(targetPath) if err != nil { + glog.V(3).Infof("failed to unmount targetPath: %s with error: %v", targetPath, err) return nil, status.Error(codes.Internal, err.Error()) } @@ -156,6 +173,12 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu return nil, err } + // Remove targetPath + if err := os.RemoveAll(targetPath); err != nil { + glog.V(3).Infof("failed to remove targetPath: %s with error: %v", targetPath, err) + return nil, err + } + return &csi.NodeUnpublishVolumeResponse{}, nil } From ea75a9d1629b29dee0d0bdf62f00310980369131 Mon Sep 17 00:00:00 2001 From: Masaki Kimura Date: Thu, 15 Nov 2018 20:40:19 +0000 Subject: [PATCH 3/6] Fix pv deletion issue caused by not dettaching due to wrong mount count --- pkg/rbd/nodeserver.go | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/pkg/rbd/nodeserver.go b/pkg/rbd/nodeserver.go index 069874e38..a2b7785f1 100644 --- a/pkg/rbd/nodeserver.go +++ b/pkg/rbd/nodeserver.go @@ -60,9 +60,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis } // Check if that target path exists properly - // IsLikelyNotMountPoint doesn't return right result to bind mount of device file - // TODO: Need to fix this to a proper check - notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath) + notMnt, err := ns.mounter.IsNotMountPoint(targetPath) if err != nil { if os.IsNotExist(err) { if isBlock { @@ -137,10 +135,7 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu targetPathMutex.LockKey(targetPath) defer targetPathMutex.UnlockKey(targetPath) - // IsLikelyNotMountPoint doesn't return right result to bind mount of device file - // So, just use it to check if file exists, not a mount point - // TODO: Need to fix this to a proper check - _, err := ns.mounter.IsLikelyNotMountPoint(targetPath) + notMnt, err := ns.mounter.IsNotMountPoint(targetPath) if err != nil { if os.IsNotExist(err) { // targetPath has already been deleted @@ -149,12 +144,34 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu } return nil, status.Error(codes.NotFound, err.Error()) } + if notMnt { + // TODO should consider deleting path instead of returning error, + // once all codes become ready for csi 1.0. + return nil, status.Error(codes.NotFound, "Volume not mounted") + } devicePath, cnt, err := mount.GetDeviceNameFromMount(ns.mounter, targetPath) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } + // Bind mounted device needs to be resolved by using resolveBindMountedBlockDevice + if devicePath == "devtmpfs" { + var err error + devicePath, err = resolveBindMountedBlockDevice(targetPath) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + glog.V(4).Infof("NodeUnpublishVolume: devicePath: %s, (original)cnt: %d\n", devicePath, cnt) + // cnt for GetDeviceNameFromMount is broken for bind mouted device, + // it counts total number of mounted "devtmpfs", instead of counting this device. + // So, forcibly setting cnt to 1 here. + // TODO : fix this properly + cnt = 1 + } + + glog.V(4).Infof("NodeUnpublishVolume: targetPath: %s, devicePath: %s\n", targetPath, devicePath) + // Unmounting the image err = ns.mounter.Unmount(targetPath) if err != nil { @@ -164,6 +181,7 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu cnt-- if cnt != 0 { + // TODO should this be fixed not to success, so that driver can retry unmounting? return &csi.NodeUnpublishVolumeResponse{}, nil } From 0e60dabca34abc802e607a5e700395376f624d51 Mon Sep 17 00:00:00 2001 From: Masaki Kimura Date: Wed, 7 Nov 2018 02:05:19 +0000 Subject: [PATCH 4/6] Move resolving bind mount logic from k8s --- pkg/rbd/nodeserver.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/pkg/rbd/nodeserver.go b/pkg/rbd/nodeserver.go index a2b7785f1..3cea36dc7 100644 --- a/pkg/rbd/nodeserver.go +++ b/pkg/rbd/nodeserver.go @@ -19,6 +19,8 @@ package rbd import ( "fmt" "os" + "os/exec" + "regexp" "strings" "github.com/golang/glog" @@ -215,3 +217,30 @@ func (ns *nodeServer) NodeUnstageVolume( return nil, status.Error(codes.Unimplemented, "") } + +func resolveBindMountedBlockDevice(mountPath string) (string, error) { + cmd := exec.Command("findmnt", "-n", "-o", "SOURCE", "--first-only", "--target", mountPath) + out, err := cmd.CombinedOutput() + if err != nil { + glog.V(2).Infof("Failed findmnt command for path %s: %s %v", mountPath, out, err) + return "", err + } + return parseFindMntResolveSource(string(out)) +} + +// parse output of "findmnt -o SOURCE --first-only --target" and return just the SOURCE +func parseFindMntResolveSource(out string) (string, error) { + // cut trailing newline + out = strings.TrimSuffix(out, "\n") + // Check if out is a mounted device + reMnt := regexp.MustCompile("^(/[^/]+(?:/[^/]*)*)$") + if match := reMnt.FindStringSubmatch(out); match != nil { + return match[1], nil + } + // Check if out is a block device + reBlk := regexp.MustCompile("^devtmpfs\\[(/[^/]+(?:/[^/]*)*)\\]$") + if match := reBlk.FindStringSubmatch(out); match != nil { + return fmt.Sprintf("/dev%s", match[1]), nil + } + return "", fmt.Errorf("parseFindMntResolveSource: %s doesn't match to any expected findMnt output", out) +} From 263c45bb458ed8bf05fa5c4148fa76d83ca3871e Mon Sep 17 00:00:00 2001 From: Huamin Chen Date: Wed, 16 Jan 2019 13:52:45 -0500 Subject: [PATCH 5/6] enable csi block; use canary external-provisioner image to pick up block volume provisioning Signed-off-by: Huamin Chen --- deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml | 3 +-- pkg/rbd/controllerserver.go | 5 ----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml b/deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml index 966b49260..fd7e3c634 100644 --- a/deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml +++ b/deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml @@ -27,9 +27,8 @@ spec: serviceAccount: csi-provisioner containers: - name: csi-provisioner - image: quay.io/k8scsi/csi-provisioner:v1.0.0 + image: quay.io/k8scsi/csi-provisioner:canary args: - - "--provisioner=csi-rbdplugin" - "--csi-address=$(ADDRESS)" - "--v=5" env: diff --git a/pkg/rbd/controllerserver.go b/pkg/rbd/controllerserver.go index 8dff0c35e..bccfe6e4a 100644 --- a/pkg/rbd/controllerserver.go +++ b/pkg/rbd/controllerserver.go @@ -77,11 +77,6 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol if req.VolumeCapabilities == nil { return nil, status.Error(codes.InvalidArgument, "Volume Capabilities cannot be empty") } - for _, cap := range req.VolumeCapabilities { - if cap.GetBlock() != nil { - return nil, status.Error(codes.Unimplemented, "Block Volume not supported") - } - } volumeNameMutex.LockKey(req.GetName()) defer volumeNameMutex.UnlockKey(req.GetName()) From 48407e2484dd69750b709c772149e15c095c5af9 Mon Sep 17 00:00:00 2001 From: Huamin Chen Date: Thu, 17 Jan 2019 08:57:18 -0500 Subject: [PATCH 6/6] add csi volume device mount path to csi plugin Signed-off-by: Huamin Chen --- deploy/rbd/kubernetes/csi-rbdplugin.yaml | 3 +++ pkg/rbd/nodeserver.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/deploy/rbd/kubernetes/csi-rbdplugin.yaml b/deploy/rbd/kubernetes/csi-rbdplugin.yaml index 264104ac7..721565873 100644 --- a/deploy/rbd/kubernetes/csi-rbdplugin.yaml +++ b/deploy/rbd/kubernetes/csi-rbdplugin.yaml @@ -72,6 +72,9 @@ spec: - name: pods-mount-dir mountPath: /var/lib/kubelet/pods mountPropagation: "Bidirectional" + - name: plugin-mount-dir + mountPath: /var/lib/kubelet/plugins/kubernetes.io/csi/volumeDevices/ + mountPropagation: "Bidirectional" - mountPath: /dev name: host-dev - mountPath: /rootfs diff --git a/pkg/rbd/nodeserver.go b/pkg/rbd/nodeserver.go index 3cea36dc7..9ea5138e4 100644 --- a/pkg/rbd/nodeserver.go +++ b/pkg/rbd/nodeserver.go @@ -55,7 +55,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis } else { // Get volName from targetPath if !strings.HasSuffix(targetPath, "/mount") { - return nil, fmt.Errorf("rnd: malformed the value of target path: %s", targetPath) + return nil, fmt.Errorf("rbd: malformed the value of target path: %s", targetPath) } s := strings.Split(strings.TrimSuffix(targetPath, "/mount"), "/") volName = s[len(s)-1]