Merge pull request #383 from aramase/create-target-path

feat: create target path in node publish

Author: k8s-ci-robot

go.mod

@@ -4,20 +4,20 @@ go 1.15
 
 require (
 	github.com/blang/semver v3.5.0+incompatible
-	github.com/container-storage-interface/spec v1.0.0
+	github.com/container-storage-interface/spec v1.3.0
 	github.com/go-logr/logr v0.2.1 // indirect
 	github.com/golang/protobuf v1.4.3
 	github.com/google/go-cmp v0.5.2
-	github.com/kubernetes-csi/csi-lib-utils v0.6.1
-	github.com/kubernetes-csi/csi-test v1.1.0
+	github.com/kubernetes-csi/csi-lib-utils v0.7.1
+	github.com/kubernetes-csi/csi-test/v4 v4.0.2
 	github.com/onsi/gomega v1.10.1
 	github.com/prometheus/client_golang v1.8.0 // indirect
 	github.com/stretchr/testify v1.6.1
 	go.opentelemetry.io/otel v0.13.0
 	go.opentelemetry.io/otel/exporters/metric/prometheus v0.13.0
 	golang.org/x/net v0.0.0-20200707034311-ab3426394381
 	golang.org/x/sys v0.0.0-20201107080550-4d91cf3a1aaf // indirect
-	google.golang.org/grpc v1.27.1
+	google.golang.org/grpc v1.29.1
 	google.golang.org/protobuf v1.25.0
 	k8s.io/api v0.19.3
 	k8s.io/apimachinery v0.19.3

go.sum

@@ -74,3 +74,11 @@ func (cs *DefaultControllerServer) DeleteSnapshot(ctx context.Context, req *csi.
 func (cs *DefaultControllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsRequest) (*csi.ListSnapshotsResponse, error) {
 	return nil, status.Error(codes.Unimplemented, "")
 }
+
+func (cs *DefaultControllerServer) ControllerExpandVolume(ctx context.Context, req *csi.ControllerExpandVolumeRequest) (*csi.ControllerExpandVolumeResponse, error) {
+	return &csi.ControllerExpandVolumeResponse{}, status.Error(codes.Unimplemented, "ControllerExpandVolume is not implemented")
+}
+
+func (cs *DefaultControllerServer) ControllerGetVolume(ctx context.Context, req *csi.ControllerGetVolumeRequest) (*csi.ControllerGetVolumeResponse, error) {
+	return &csi.ControllerGetVolumeResponse{}, status.Error(codes.Unimplemented, "ControllerGetVolume is not implemented")
+}

pkg/csi-common/controllerserver-default.go

@@ -71,30 +71,6 @@ func NewControllerServiceCapability(cap csi.ControllerServiceCapability_RPC_Type
 	}
 }
 
-func RunNodePublishServer(endpoint string, d *CSIDriver, ns csi.NodeServer) {
-	ids := NewDefaultIdentityServer(d)
-
-	s := NewNonBlockingGRPCServer()
-	s.Start(context.Background(), endpoint, ids, nil, ns)
-	s.Wait()
-}
-
-func RunControllerPublishServer(endpoint string, d *CSIDriver, cs csi.ControllerServer) {
-	ids := NewDefaultIdentityServer(d)
-
-	s := NewNonBlockingGRPCServer()
-	s.Start(context.Background(), endpoint, ids, cs, nil)
-	s.Wait()
-}
-
-func RunControllerandNodePublishServer(endpoint string, d *CSIDriver, cs csi.ControllerServer, ns csi.NodeServer) {
-	ids := NewDefaultIdentityServer(d)
-
-	s := NewNonBlockingGRPCServer()
-	s.Start(context.Background(), endpoint, ids, cs, ns)
-	s.Wait()
-}
-
 func logGRPC(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
 	klog.V(3).Infof("GRPC call: %s", info.FullMethod)
 	klog.V(3).Infof("GRPC request: %s", pbSanitizer.StripSecrets(req).String())

pkg/csi-common/utils.go

@@ -25,22 +25,18 @@ import (
 	"os/exec"
 	"runtime"
 
-	"github.com/container-storage-interface/spec/lib/go/csi"
-
-	"sigs.k8s.io/controller-runtime/pkg/client"
-
 	csicommon "sigs.k8s.io/secrets-store-csi-driver/pkg/csi-common"
 	internalerrors "sigs.k8s.io/secrets-store-csi-driver/pkg/errors"
 	"sigs.k8s.io/secrets-store-csi-driver/pkg/util/fileutil"
 	"sigs.k8s.io/secrets-store-csi-driver/pkg/version"
 
+	"github.com/container-storage-interface/spec/lib/go/csi"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
-
-	"k8s.io/utils/mount"
-
 	"k8s.io/klog/v2"
+	"k8s.io/utils/mount"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 type nodeServer struct {
@@ -115,15 +111,23 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 
 	mounted, err = ns.ensureMountPoint(targetPath)
 	if err != nil {
-		errorReason = internalerrors.FailedToEnsureMountPoint
-		return nil, status.Errorf(codes.Internal, "Could not mount target %q: %v", targetPath, err)
+		// kubelet will not create the CSI NodePublishVolume target directory in 1.20+, in accordance with the CSI specification.
+		// CSI driver needs to properly create and process the target path
+		if os.IsNotExist(err) {
+			if err = os.MkdirAll(targetPath, 0750); err != nil {
+				return nil, status.Errorf(codes.Internal, "failed to create target path %s, err: %v", targetPath, err)
+			}
+		} else {
+			errorReason = internalerrors.FailedToEnsureMountPoint
+			return nil, status.Errorf(codes.Internal, "failed to check if target path %s is mount point, err: %v", targetPath, err)
+		}
 	}
 	if mounted {
 		klog.InfoS("target path is already mounted", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName})
 		return &csi.NodePublishVolumeResponse{}, nil
 	}
 
-	klog.V(2).InfoS("node publish volume", "target", targetPath, "volumeId", volumeID, "attributes", attrib, "mountflags", mountFlags)
+	klog.V(2).InfoS("node publish volume", "target", targetPath, "volumeId", volumeID, "attributes", attrib, "mount flags", mountFlags)
 
 	if isMockProvider(providerName) {
 		// mock provider is used only for running sanity tests against the driver
@@ -206,8 +210,6 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 }
 
 func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublishVolumeRequest) (nuvr *csi.NodeUnpublishVolumeResponse, err error) {
-	var podUID string
-
 	defer func() {
 		if err != nil {
 			ns.reporter.ReportNodeUnPublishErrorCtMetric()
@@ -231,27 +233,23 @@ func (ns *nodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpu
 		return &csi.NodeUnpublishVolumeResponse{}, nil
 	}
 
-	podUID = fileutil.GetPodUIDFromTargetPath(targetPath)
-	if len(podUID) == 0 {
-		return nil, status.Error(codes.InvalidArgument, "Cannot get podUID from Target path")
-	}
 	// remove files
 	if runtime.GOOS == "windows" {
 		for _, file := range files {
 			err = os.RemoveAll(file)
 			if err != nil {
-				klog.ErrorS(err, "failed to remove file from target path", "file", file, "podUID", podUID)
+				klog.ErrorS(err, "failed to remove file from target path", "file", file)
 				return nil, status.Error(codes.Internal, err.Error())
 			}
 		}
 	}
 	err = mount.CleanupMountPoint(targetPath, ns.mounter, false)
-	if err != nil {
-		klog.ErrorS(err, "failed to clean and unmount target path", "targetPath", targetPath, "podUID", podUID)
+	if err != nil && !os.IsNotExist(err) {
+		klog.ErrorS(err, "failed to clean and unmount target path", "targetPath", targetPath)
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
-	klog.InfoS("node unpublish volume complete", "targetPath", targetPath, "podUID", podUID)
+	klog.InfoS("node unpublish volume complete", "targetPath", targetPath)
 	return &csi.NodeUnpublishVolumeResponse{}, nil
 }
 
@@ -357,3 +355,7 @@ func (ns *nodeServer) mountSecretsStoreObjectContent(ctx context.Context, provid
 	}
 	return nil, "", nil
 }
+
+func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolumeRequest) (*csi.NodeExpandVolumeResponse, error) {
+	return nil, status.Error(codes.Unimplemented, "NodeExpandVolume is not implemented")
+}

pkg/secrets-store/nodeserver.go

@@ -23,22 +23,21 @@ import (
 	"path/filepath"
 	"testing"
 
+	"sigs.k8s.io/secrets-store-csi-driver/apis/v1alpha1"
 	internalerrors "sigs.k8s.io/secrets-store-csi-driver/pkg/errors"
-
-	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/secrets-store-csi-driver/pkg/secrets-store/mocks"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/utils/mount"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
-	"sigs.k8s.io/secrets-store-csi-driver/apis/v1alpha1"
-	"sigs.k8s.io/secrets-store-csi-driver/pkg/secrets-store/mocks"
 )
 
 func testNodeServer(mountPoints []mount.MountPoint, client client.Client, grpcSupportProviders string, reporter StatsReporter, providerBinaryName string) (*nodeServer, error) {
@@ -462,17 +461,6 @@ func TestNodeUnpublishVolume(t *testing.T) {
 			RPCCode:            codes.InvalidArgument,
 			shouldRetryUnmount: true,
 		},
-		{
-			name: "Failure: target path does not contain valid podUID",
-			nodeUnpublishVolReq: csi.NodeUnpublishVolumeRequest{
-				VolumeId:   "testvolid1",
-				TargetPath: getTestTargetPath("", t),
-			},
-			wantsErr:           true,
-			wantsRPCCode:       true,
-			RPCCode:            codes.InvalidArgument,
-			shouldRetryUnmount: true,
-		},
 		{
 			name: "Success for a mounted volume with a retry",
 			nodeUnpublishVolReq: csi.NodeUnpublishVolumeRequest{

pkg/secrets-store/nodeserver_test.go

@@ -19,7 +19,6 @@ package secretsstore
 import (
 	"fmt"
 	"io/ioutil"
-	"os"
 	"runtime"
 	"strings"
 
@@ -53,7 +52,7 @@ func normalizeWindowsPath(path string) string {
 // ensureMountPoint ensures mount point is valid
 func (ns *nodeServer) ensureMountPoint(target string) (bool, error) {
 	notMnt, err := ns.mounter.IsLikelyNotMountPoint(target)
-	if err != nil && !os.IsNotExist(err) {
+	if err != nil {
 		return !notMnt, err
 	}
 

pkg/secrets-store/utils.go

@@ -15,16 +15,17 @@ package sanity
 
 import (
 	"context"
+	"fmt"
+	"os"
+	"path/filepath"
 	"testing"
 
-	"github.com/kubernetes-csi/csi-test/pkg/sanity"
+	"github.com/kubernetes-csi/csi-test/v4/pkg/sanity"
 
 	secretsstore "sigs.k8s.io/secrets-store-csi-driver/pkg/secrets-store"
 )
 
 const (
-	mountPath          = "/tmp/csi/mount"
-	stagePath          = "/tmp/csi/stage"
 	socket             = "/tmp/csi.sock"
 	endpoint           = "unix://" + socket
 	providerVolumePath = "/etc/kubernetes/secrets-store-csi-providers"
@@ -36,10 +37,29 @@ func TestSanity(t *testing.T) {
 		driver.Run(context.Background(), "secrets-store.csi.k8s.io", "somenodeid", endpoint, providerVolumePath, "provider1=0.0.2,provider2=0.0.4", "", nil)
 	}()
 
-	config := &sanity.Config{
-		TargetPath:  mountPath,
-		StagingPath: stagePath,
-		Address:     endpoint,
+	tmpPath := filepath.Join(os.TempDir(), "csi")
+	config := sanity.NewTestConfig()
+	config.Address = endpoint
+	config.CreateTargetDir = func(targetPath string) (string, error) {
+		targetPath = filepath.Join(tmpPath, targetPath)
+		return targetPath, createTargetDir(targetPath)
+	}
+	config.RemoveTargetPath = func(targetPath string) error {
+		return os.RemoveAll(targetPath)
 	}
 	sanity.Test(t, config)
 }
+
+func createTargetDir(targetPath string) error {
+	fileInfo, err := os.Stat(targetPath)
+	if err != nil && os.IsNotExist(err) {
+		return os.MkdirAll(targetPath, 0755)
+	} else if err != nil {
+		return err
+	}
+	if !fileInfo.IsDir() {
+		return fmt.Errorf("target location %s is not a directory", targetPath)
+	}
+
+	return nil
+}