Merge pull request #308 from ZeroMagic/decouple

feat: remove dependency of azure.json in node server

Author: k8s-ci-robot

pkg/blob/azure.go

@@ -46,21 +46,24 @@ func IsAzureStackCloud(cloud *azureprovider.Cloud) bool {
 	return strings.EqualFold(cloud.Config.Cloud, "AZURESTACKCLOUD")
 }
 
-// GetCloudProvider get Azure Cloud Provider
-func GetCloudProvider(kubeconfig string) (*azureprovider.Cloud, error) {
+// getCloudProvider get Azure Cloud Provider
+func getCloudProvider(kubeconfig, nodeID string) (*azureprovider.Cloud, error) {
+	isController := (nodeID == "")
+
 	kubeClient, err := getKubeClient(kubeconfig)
 	if err != nil && !os.IsNotExist(err) && err != rest.ErrNotInCluster {
 		return nil, fmt.Errorf("failed to get KubeClient: %v", err)
 	}
 
 	az := &azureprovider.Cloud{}
+
 	if kubeClient != nil {
 		klog.V(2).Infof("reading cloud config from secret")
 		az.KubeClient = kubeClient
 		az.InitializeCloudFromSecret()
 	}
 
-	if az.TenantID == "" || az.SubscriptionID == "" {
+	if az.TenantID == "" || az.SubscriptionID == "" || az.ResourceGroup == "" {
 		klog.V(2).Infof("could not read cloud config from secret")
 		credFile, ok := os.LookupEnv(DefaultAzureCredentialFileEnv)
 		if ok {
@@ -70,22 +73,39 @@ func GetCloudProvider(kubeconfig string) (*azureprovider.Cloud, error) {
 			klog.V(2).Infof("use default %s env var: %v", DefaultAzureCredentialFileEnv, credFile)
 		}
 
-		f, err := os.Open(credFile)
+		var f *os.File
+		f, err = os.Open(credFile)
+		if f != nil {
+			defer f.Close()
+		}
 		if err != nil {
 			klog.Errorf("Failed to load config from file: %s", credFile)
-			return nil, fmt.Errorf("Failed to load config from file: %s, cloud not get azure cloud provider", credFile)
-		}
-		defer f.Close()
-
-		klog.V(2).Infof("read cloud config from file: %s successfully", credFile)
-		if az, err = azureprovider.NewCloudWithoutFeatureGates(f); err != nil {
-			return az, err
+			err = fmt.Errorf("Failed to load config from file: %s, cloud not get azure cloud provider", credFile)
+		} else {
+			klog.V(2).Infof("read cloud config from file: %s successfully", credFile)
+			az, err = azureprovider.NewCloudWithoutFeatureGates(f)
 		}
 	}
 
 	if kubeClient != nil {
 		az.KubeClient = kubeClient
 	}
+	if isController {
+		if err != nil {
+			return az, err
+		}
+		// Disable UseInstanceMetadata for controller to mitigate a timeout issue using IMDS
+		// https://github.com/kubernetes-sigs/azuredisk-csi-driver/issues/168
+		klog.Infoln("disable UseInstanceMetadata for controller")
+		az.Config.UseInstanceMetadata = false
+		klog.Infoln("Starting the controller server...")
+	} else {
+		if err != nil {
+			klog.V(2).Infof("there is no azure.json provided for node server, error: %v", err)
+		}
+		klog.V(2).Infof("Starting the node server, nodeID is(%s)", nodeID)
+	}
+
 	return az, nil
 }
 

pkg/blob/azure_test.go

@@ -30,9 +30,10 @@ import (
 	azure2 "sigs.k8s.io/cloud-provider-azure/pkg/provider"
 )
 
-// TestGetCloudProvider tests the func GetCloudProvider().
+// TestGetCloudProvider tests the func getCloudProvider().
 // To run this unit test successfully, need to ensure /etc/kubernetes/azure.json nonexistent.
 func TestGetCloudProvider(t *testing.T) {
+
 	fakeCredFile := "fake-cred-file.json"
 	fakeKubeConfig := "fake-kube-config"
 	emptyKubeConfig := "empty-kube-config"
@@ -74,31 +75,37 @@ users:
 	tests := []struct {
 		desc        string
 		kubeconfig  string
+		nodeID      string
 		expectedErr error
 	}{
 		{
 			desc:        "[failure] out of cluster, no kubeconfig, no credential file",
 			kubeconfig:  "",
+			nodeID:      "",
 			expectedErr: fmt.Errorf("Failed to load config from file: %s, cloud not get azure cloud provider", DefaultCredFilePath),
 		},
 		{
 			desc:        "[failure] out of cluster & in cluster, specify a non-exist kubeconfig, no credential file",
 			kubeconfig:  "/tmp/non-exist.json",
+			nodeID:      "",
 			expectedErr: fmt.Errorf("Failed to load config from file: %s, cloud not get azure cloud provider", DefaultCredFilePath),
 		},
 		{
 			desc:        "[failure] out of cluster & in cluster, specify a empty kubeconfig, no credential file",
 			kubeconfig:  emptyKubeConfig,
+			nodeID:      "",
 			expectedErr: fmt.Errorf("failed to get KubeClient: invalid configuration: no configuration has been provided, try setting KUBERNETES_MASTER environment variable"),
 		},
 		{
 			desc:        "[failure] out of cluster & in cluster, specify a fake kubeconfig, no credential file",
 			kubeconfig:  fakeKubeConfig,
+			nodeID:      "",
 			expectedErr: fmt.Errorf("Failed to load config from file: %s, cloud not get azure cloud provider", DefaultCredFilePath),
 		},
 		{
 			desc:        "[success] out of cluster & in cluster, no kubeconfig, a fake credential file",
 			kubeconfig:  "",
+			nodeID:      "",
 			expectedErr: nil,
 		},
 	}
@@ -138,7 +145,7 @@ users:
 			}
 			os.Setenv(DefaultAzureCredentialFileEnv, fakeCredFile)
 		}
-		_, err := GetCloudProvider(test.kubeconfig)
+		_, err := getCloudProvider(test.kubeconfig, test.nodeID)
 		if !reflect.DeepEqual(err, test.expectedErr) {
 			t.Errorf("desc: %s,\n input: %q, GetCloudProvider err: %v, expectedErr: %v", test.desc, test.kubeconfig, err, test.expectedErr)
 		}
@@ -226,6 +233,7 @@ func TestGetKeyVaultSecretContent(t *testing.T) {
 		t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
 	}
 }
+
 func createTestFile(path string) error {
 	f, err := os.Create(path)
 	if err != nil {

pkg/blob/blob.go

@@ -47,6 +47,7 @@ const (
 	volumeIDTemplate           = "%s#%s#%s"
 	secretNameTemplate         = "azure-storage-account-%s-secret"
 	serverNameField            = "server"
+	storageEndpointSuffixField = "storageendpointsuffix"
 	tagsField                  = "tags"
 	protocolField              = "protocol"
 	accountNameField           = "accountname"
@@ -120,18 +121,10 @@ func (d *Driver) Run(endpoint, kubeconfig string, testBool bool) {
 	}
 	klog.Infof("\nDRIVER INFORMATION:\n-------------------\n%s\n\nStreaming logs below:", versionMeta)
 
-	cloud, err := GetCloudProvider(kubeconfig)
-	if err != nil || cloud.TenantID == "" || cloud.SubscriptionID == "" {
+	d.cloud, err = getCloudProvider(kubeconfig, d.NodeID)
+	if err != nil {
 		klog.Fatalf("failed to get Azure Cloud Provider, error: %v", err)
 	}
-	d.cloud = cloud
-
-	if d.NodeID == "" {
-		// Disable UseInstanceMetadata for controller to mitigate a timeout issue using IMDS
-		// https://github.com/kubernetes-sigs/azuredisk-csi-driver/issues/168
-		klog.Infoln("disable UseInstanceMetadata for controller")
-		d.cloud.Config.UseInstanceMetadata = false
-	}
 
 	d.mounter = &mount.SafeFormatAndMount{
 		Interface: mount.New(""),

pkg/blob/nodeserver.go

@@ -150,13 +150,15 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 	attrib := req.GetVolumeContext()
 	secrets := req.GetSecrets()
 
-	var blobStorageEndPoint, protocol string
+	var serverAddress, storageEndpointSuffix, protocol string
 	for k, v := range attrib {
 		switch strings.ToLower(k) {
 		case serverNameField:
-			blobStorageEndPoint = v
+			serverAddress = v
 		case protocolField:
 			protocol = v
+		case storageEndpointSuffixField:
+			storageEndpointSuffix = v
 		}
 	}
 
@@ -165,16 +167,20 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 		return nil, err
 	}
 
-	if strings.TrimSpace(blobStorageEndPoint) == "" {
+	if strings.TrimSpace(storageEndpointSuffix) == "" {
+		storageEndpointSuffix = "core.windows.net"
+	}
+
+	if strings.TrimSpace(serverAddress) == "" {
 		// server address is "accountname.blob.core.windows.net" by default
-		blobStorageEndPoint = fmt.Sprintf("%s.blob.%s", accountName, d.cloud.Environment.StorageEndpointSuffix)
+		serverAddress = fmt.Sprintf("%s.blob.%s", accountName, storageEndpointSuffix)
 	}
 
 	if protocol == nfs {
-		klog.V(2).Infof("target %v\nprotocol %v\n\nvolumeId %v\ncontext %v\nmountflags %v\nblobStorageEndPoint %v",
-			targetPath, protocol, volumeID, attrib, mountFlags, blobStorageEndPoint)
+		klog.V(2).Infof("target %v\nprotocol %v\n\nvolumeId %v\ncontext %v\nmountflags %v\nserverAddress %v",
+			targetPath, protocol, volumeID, attrib, mountFlags, serverAddress)
 
-		source := fmt.Sprintf("%s:/%s/%s", blobStorageEndPoint, accountName, containerName)
+		source := fmt.Sprintf("%s:/%s/%s", serverAddress, accountName, containerName)
 		mountOptions := util.JoinMountOptions(mountFlags, []string{"sec=sys,vers=3,nolock"})
 		if err := wait.PollImmediate(1*time.Second, 2*time.Minute, func() (bool, error) {
 			return true, d.mounter.MountSensitive(source, targetPath, nfs, mountOptions, []string{})
@@ -201,12 +207,12 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 		args = args + " " + opt
 	}
 
-	klog.V(2).Infof("target %v\nprotocol %v\n\nvolumeId %v\ncontext %v\nmountflags %v\nmountOptions %v\nargs %v\nblobStorageEndPoint %v",
-		targetPath, protocol, volumeID, attrib, mountFlags, mountOptions, args, blobStorageEndPoint)
+	klog.V(2).Infof("target %v\nprotocol %v\n\nvolumeId %v\ncontext %v\nmountflags %v\nmountOptions %v\nargs %v\nserverAddress %v",
+		targetPath, protocol, volumeID, attrib, mountFlags, mountOptions, args, serverAddress)
 	cmd := exec.Command("blobfuse", strings.Split(args, " ")...)
 
 	cmd.Env = append(os.Environ(), "AZURE_STORAGE_ACCOUNT="+accountName)
-	cmd.Env = append(cmd.Env, "AZURE_STORAGE_BLOB_ENDPOINT="+blobStorageEndPoint)
+	cmd.Env = append(cmd.Env, "AZURE_STORAGE_BLOB_ENDPOINT="+serverAddress)
 	cmd.Env = append(cmd.Env, authEnv...)
 
 	output, err := cmd.CombinedOutput()

pkg/blobplugin/main.go

@@ -54,9 +54,6 @@ func main() {
 		fmt.Println(info)
 		os.Exit(0)
 	}
-	if *nodeID == "" {
-		klog.Warning("nodeid is empty")
-	}
 
 	exportMetrics()
 	handle()

test/sanity/run-test.sh

@@ -25,14 +25,16 @@ function cleanup {
 
 trap cleanup EXIT
 
-readonly endpoint="unix:///tmp/csi.sock"
+readonly controllerendpoint="unix:///tmp/csi-controller.sock"
+readonly nodeendpoint="unix:///tmp/csi-node.sock"
 nodeid="CSINode"
 if [[ "$#" -gt 0 ]] && [[ -n "$1" ]]; then
   nodeid="$1"
 fi
 
-_output/blobplugin --endpoint "$endpoint" --nodeid "$nodeid" -v=5 &
+_output/blobplugin --endpoint "$controllerendpoint" -v=5 &
+_output/blobplugin --endpoint "$nodeendpoint" --nodeid "$nodeid" -v=5 &
 
 echo "Begin to run sanity test..."
 readonly CSI_SANITY_BIN='csi-sanity'
-"$CSI_SANITY_BIN" --ginkgo.v --csi.endpoint=$endpoint -ginkgo.skip="should fail when requesting to create a volume with already existing name and different capacity|should be idempotent"
+"$CSI_SANITY_BIN" --ginkgo.v --csi.endpoint=$nodeendpoint --csi.controllerendpoint=$controllerendpoint -ginkgo.skip="should fail when requesting to create a volume with already existing name and different capacity|should be idempotent"