From a0ad5ca5b6a156c73b8cc8088f56c77c65443da1 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sun, 27 Dec 2020 03:59:09 +0000 Subject: [PATCH 1/2] fix: support mountOptions in CreateVolume --- pkg/mounter/safe_mounter_unix.go | 28 ----------------------- pkg/nfs/controllerserver.go | 23 ++++++++++++------- pkg/nfs/fake_mounter.go | 6 ----- pkg/nfs/nodeserver.go | 38 ++++++++++++++++++-------------- 4 files changed, 37 insertions(+), 58 deletions(-) delete mode 100644 pkg/mounter/safe_mounter_unix.go diff --git a/pkg/mounter/safe_mounter_unix.go b/pkg/mounter/safe_mounter_unix.go deleted file mode 100644 index 678c2950..00000000 --- a/pkg/mounter/safe_mounter_unix.go +++ /dev/null @@ -1,28 +0,0 @@ -// +build linux darwin - -/* -Copyright 2020 The Kubernetes Authors. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package mounter - -import ( - utilexec "k8s.io/utils/exec" - "k8s.io/utils/mount" -) - -func NewSafeMounter() (*mount.SafeFormatAndMount, error) { - return &mount.SafeFormatAndMount{ - Interface: mount.New(""), - Exec: utilexec.New(), - }, nil -} diff --git a/pkg/nfs/controllerserver.go b/pkg/nfs/controllerserver.go index bde023dc..f246ffe0 100644 --- a/pkg/nfs/controllerserver.go +++ b/pkg/nfs/controllerserver.go @@ -82,8 +82,12 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol return nil, status.Error(codes.InvalidArgument, err.Error()) } + var volCap *csi.VolumeCapability + if len(req.GetVolumeCapabilities()) > 0 { + volCap = req.GetVolumeCapabilities()[0] + } // Mount nfs base share so we can create a subdirectory - if err = cs.internalMount(ctx, nfsVol); err != nil { + if err = cs.internalMount(ctx, nfsVol, volCap); err != nil { return nil, status.Errorf(codes.Internal, "failed to mount nfs server: %v", err.Error()) } defer func() { @@ -116,7 +120,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol } // Mount nfs base share so we can delete the subdirectory - if err = cs.internalMount(ctx, nfsVol); err != nil { + if err = cs.internalMount(ctx, nfsVol, nil); err != nil { return nil, status.Errorf(codes.Internal, "failed to mount nfs server: %v", err.Error()) } defer func() { @@ -230,13 +234,16 @@ func (cs *ControllerServer) validateVolumeCapability(c *csi.VolumeCapability) er } // Mount nfs server at base-dir -func (cs *ControllerServer) internalMount(ctx context.Context, vol *nfsVolume) error { +func (cs *ControllerServer) internalMount(ctx context.Context, vol *nfsVolume, volCap *csi.VolumeCapability) error { sharePath := filepath.Join(string(filepath.Separator) + vol.baseDir) targetPath := cs.getInternalMountPath(vol) - stdVolCap := csi.VolumeCapability{ - AccessType: &csi.VolumeCapability_Mount{ - Mount: &csi.VolumeCapability_MountVolume{}, - }, + + if volCap == nil { + volCap = &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + } } glog.V(4).Infof("internally mounting %v:%v at %v", vol.server, sharePath, targetPath) @@ -246,7 +253,7 @@ func (cs *ControllerServer) internalMount(ctx context.Context, vol *nfsVolume) e paramServer: vol.server, paramShare: sharePath, }, - VolumeCapability: &stdVolCap, + VolumeCapability: volCap, VolumeId: vol.id, }) return err diff --git a/pkg/nfs/fake_mounter.go b/pkg/nfs/fake_mounter.go index f052c61f..b3210117 100644 --- a/pkg/nfs/fake_mounter.go +++ b/pkg/nfs/fake_mounter.go @@ -18,11 +18,8 @@ package nfs import ( "fmt" - "runtime" "strings" - "github.com/kubernetes-csi/csi-driver-nfs/pkg/mounter" - "k8s.io/utils/mount" ) @@ -64,9 +61,6 @@ func (f *fakeMounter) IsLikelyNotMountPoint(file string) (bool, error) { } func NewFakeMounter() (*mount.SafeFormatAndMount, error) { - if runtime.GOOS == "windows" { - return mounter.NewSafeMounter() - } return &mount.SafeFormatAndMount{ Interface: &fakeMounter{}, }, nil diff --git a/pkg/nfs/nodeserver.go b/pkg/nfs/nodeserver.go index ce5617c5..51ac58ed 100644 --- a/pkg/nfs/nodeserver.go +++ b/pkg/nfs/nodeserver.go @@ -21,28 +21,29 @@ import ( "os" "strings" - "github.com/golang/glog" - "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/golang/glog" "golang.org/x/net/context" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "k8s.io/utils/mount" ) +// NodeServer driver type NodeServer struct { Driver *Driver mounter mount.Interface } +// NodePublishVolume mount the volume func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { if req.GetVolumeCapability() == nil { return nil, status.Error(codes.InvalidArgument, "Volume capability missing in request") } - if len(req.GetVolumeId()) == 0 { + volumeID := req.GetVolumeId() + if len(volumeID) == 0 { return nil, status.Error(codes.InvalidArgument, "Volume ID missing in request") } - targetPath := req.GetTargetPath() if len(targetPath) == 0 { return nil, status.Error(codes.InvalidArgument, "Target path not provided") @@ -59,21 +60,21 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis return nil, status.Error(codes.Internal, err.Error()) } } - if !notMnt { return &csi.NodePublishVolumeResponse{}, nil } - mo := req.GetVolumeCapability().GetMount().GetMountFlags() + mountOptions := req.GetVolumeCapability().GetMount().GetMountFlags() if req.GetReadonly() { - mo = append(mo, "ro") + mountOptions = append(mountOptions, "ro") } s := req.GetVolumeContext()[paramServer] ep := req.GetVolumeContext()[paramShare] source := fmt.Sprintf("%s:%s", s, ep) - err = ns.mounter.Mount(source, targetPath, "nfs", mo) + glog.V(2).Infof("source(%s) targetPath(%s) volumeID(%v) mountflags(%v)", source, targetPath, volumeID, mountOptions) + err = ns.mounter.Mount(source, targetPath, "nfs", mountOptions) if err != nil { if os.IsPermission(err) { return nil, status.Error(codes.PermissionDenied, err.Error()) @@ -93,14 +94,16 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis return &csi.NodePublishVolumeResponse{}, nil } +// NodeUnpublishVolume unmount the volume func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) { - if len(req.GetVolumeId()) == 0 { + volumeID := req.GetVolumeId() + if len(volumeID) == 0 { return nil, status.Error(codes.InvalidArgument, "Volume ID missing in request") } - if len(req.GetTargetPath()) == 0 { + targetPath := req.GetTargetPath() + if len(targetPath) == 0 { return nil, status.Error(codes.InvalidArgument, "Target path missing in request") } - targetPath := req.GetTargetPath() notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath) if err != nil { @@ -113,7 +116,8 @@ func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu return nil, status.Error(codes.NotFound, "Volume not mounted") } - err = mount.CleanupMountPoint(req.GetTargetPath(), ns.mounter, false) + glog.V(2).Infof("NodeUnpublishVolume(%s): CleanupMountPoint %s", volumeID, targetPath) + err = mount.CleanupMountPoint(targetPath, ns.mounter, false) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -121,17 +125,15 @@ func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu return &csi.NodeUnpublishVolumeResponse{}, nil } +// NodeGetInfo return info of the node on which this plugin is running func (ns *NodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoRequest) (*csi.NodeGetInfoResponse, error) { - glog.V(5).Infof("Using default NodeGetInfo") - return &csi.NodeGetInfoResponse{ NodeId: ns.Driver.nodeID, }, nil } +// NodeGetCapabilities return the capabilities of the Node plugin func (ns *NodeServer) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetCapabilitiesRequest) (*csi.NodeGetCapabilitiesResponse, error) { - glog.V(5).Infof("Using default NodeGetCapabilities") - return &csi.NodeGetCapabilitiesResponse{ Capabilities: []*csi.NodeServiceCapability{ { @@ -145,18 +147,22 @@ func (ns *NodeServer) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetC }, nil } +// NodeGetVolumeStats get volume stats func (ns *NodeServer) NodeGetVolumeStats(ctx context.Context, in *csi.NodeGetVolumeStatsRequest) (*csi.NodeGetVolumeStatsResponse, error) { return nil, status.Error(codes.Unimplemented, "") } +// NodeUnstageVolume unstage volume func (ns *NodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVolumeRequest) (*csi.NodeUnstageVolumeResponse, error) { return &csi.NodeUnstageVolumeResponse{}, nil } +// NodeStageVolume stage volume func (ns *NodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRequest) (*csi.NodeStageVolumeResponse, error) { return &csi.NodeStageVolumeResponse{}, nil } +// NodeExpandVolume node expand volume func (ns *NodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolumeRequest) (*csi.NodeExpandVolumeResponse, error) { return nil, status.Error(codes.Unimplemented, "") } From 3622c703d6d5250b9d1475c8ae8ce8b4bdac1b41 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sun, 27 Dec 2020 12:54:25 +0000 Subject: [PATCH 2/2] chore: refactor logging --- pkg/nfs/controllerserver.go | 11 +++++------ pkg/nfs/nodeserver.go | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pkg/nfs/controllerserver.go b/pkg/nfs/controllerserver.go index f246ffe0..e3355054 100644 --- a/pkg/nfs/controllerserver.go +++ b/pkg/nfs/controllerserver.go @@ -29,6 +29,7 @@ import ( "google.golang.org/grpc/status" ) +// ControllerServer controller server setting type ControllerServer struct { Driver *Driver // Working directory for the provisioner to temporarily mount nfs shares at @@ -65,13 +66,12 @@ const ( totalIDElements // Always last ) +// CreateVolume create a volume func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) { - // Validate arguments name := req.GetName() if len(name) == 0 { return nil, status.Error(codes.InvalidArgument, "CreateVolume name must be provided") } - if err := cs.validateVolumeCapabilities(req.GetVolumeCapabilities()); err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -107,6 +107,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol return &csi.CreateVolumeResponse{Volume: cs.nfsVolToCSI(nfsVol, reqCapacity)}, nil } +// DeleteVolume delete a volume func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) { volumeID := req.GetVolumeId() if volumeID == "" { @@ -115,7 +116,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol nfsVol, err := cs.getNfsVolFromID(volumeID) if err != nil { // An invalid ID should be treated as doesn't exist - glog.V(5).Infof("failed to get nfs volume for volume id %v deletion: %v", volumeID, err) + glog.Warningf("failed to get nfs volume for volume id %v deletion: %v", volumeID, err) return &csi.DeleteVolumeResponse{}, nil } @@ -132,7 +133,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol // Delete subdirectory under base-dir internalVolumePath := cs.getInternalVolumePath(nfsVol) - glog.V(4).Infof("Removing subdirectory at %v", internalVolumePath) + glog.V(2).Infof("Removing subdirectory at %v", internalVolumePath) if err = os.RemoveAll(internalVolumePath); err != nil { return nil, status.Errorf(codes.Internal, "failed to delete subdirectory: %v", err.Error()) } @@ -175,8 +176,6 @@ func (cs *ControllerServer) GetCapacity(ctx context.Context, req *csi.GetCapacit // ControllerGetCapabilities implements the default GRPC callout. // Default supports all capabilities func (cs *ControllerServer) ControllerGetCapabilities(ctx context.Context, req *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) { - glog.V(5).Infof("Using default ControllerGetCapabilities") - return &csi.ControllerGetCapabilitiesResponse{ Capabilities: cs.Driver.cscap, }, nil diff --git a/pkg/nfs/nodeserver.go b/pkg/nfs/nodeserver.go index 51ac58ed..75b0068d 100644 --- a/pkg/nfs/nodeserver.go +++ b/pkg/nfs/nodeserver.go @@ -73,7 +73,7 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis ep := req.GetVolumeContext()[paramShare] source := fmt.Sprintf("%s:%s", s, ep) - glog.V(2).Infof("source(%s) targetPath(%s) volumeID(%v) mountflags(%v)", source, targetPath, volumeID, mountOptions) + glog.V(2).Infof("NodePublishVolume: volumeID(%v) source(%s) targetPath(%s) mountflags(%v)", volumeID, source, targetPath, mountOptions) err = ns.mounter.Mount(source, targetPath, "nfs", mountOptions) if err != nil { if os.IsPermission(err) { @@ -116,7 +116,7 @@ func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu return nil, status.Error(codes.NotFound, "Volume not mounted") } - glog.V(2).Infof("NodeUnpublishVolume(%s): CleanupMountPoint %s", volumeID, targetPath) + glog.V(2).Infof("NodeUnpublishVolume: CleanupMountPoint %s on volumeID(%s)", targetPath, volumeID) err = mount.CleanupMountPoint(targetPath, ns.mounter, false) if err != nil { return nil, status.Error(codes.Internal, err.Error())