From b79a7a12c8c78aa168de69fa2a444942b1df377f Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Tue, 4 Jul 2017 10:20:10 +0200 Subject: [PATCH 1/9] Fix the Azure file to work within different cloud environments --- pkg/volume/azure_file/azure_file.go | 18 +++++++++++++++++- pkg/volume/azure_file/azure_file_test.go | 22 +++++++++++++++++++++- pkg/volume/testing/testing.go | 10 +++++++++- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index ff91f657f4e..d486c7d12c0 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -28,6 +28,8 @@ import ( "k8s.io/kubernetes/pkg/volume" "github.com/golang/glog" + "k8s.io/kubernetes/pkg/cloudprovider" + "k8s.io/kubernetes/pkg/cloudprovider/providers/azure" "k8s.io/kubernetes/pkg/volume/util" ) @@ -207,7 +209,12 @@ func (b *azureFileMounter) SetUpAt(dir string, fsGroup *int64) error { return err } os.MkdirAll(dir, 0750) - source := fmt.Sprintf("//%s.file.core.windows.net/%s", accountName, b.shareName) + + azure, err := getAzureCloud(b.plugin.host.GetCloudProvider()) + if err != nil { + return err + } + source := fmt.Sprintf("//%s.file.%s/%s", accountName, azure.Environment.StorageEndpointSuffix, b.shareName) // parameters suggested by https://azure.microsoft.com/en-us/documentation/articles/storage-how-to-use-files-linux/ options := []string{fmt.Sprintf("vers=3.0,username=%s,password=%s,dir_mode=0777,file_mode=0777", accountName, accountKey)} if b.readOnly { @@ -268,3 +275,12 @@ func getVolumeSource( return nil, false, fmt.Errorf("Spec does not reference an AzureFile volume type") } + +func getAzureCloud(cloudProvider cloudprovider.Interface) (*azure.Cloud, error) { + azure, ok := cloudProvider.(*azure.Cloud) + if !ok || azure == nil { + return nil, fmt.Errorf("Failed to get Azure Cloud Provider. GetCloudProvider returned %v instead", cloudProvider) + } + + return azure, nil +} diff --git a/pkg/volume/azure_file/azure_file_test.go b/pkg/volume/azure_file/azure_file_test.go index 646fdad345e..c209c0440b9 100644 --- a/pkg/volume/azure_file/azure_file_test.go +++ b/pkg/volume/azure_file/azure_file_test.go @@ -20,12 +20,15 @@ import ( "io/ioutil" "os" "path" + "strings" "testing" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/fake" + "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/fake" + "k8s.io/kubernetes/pkg/cloudprovider/providers/azure" "k8s.io/kubernetes/pkg/util/mount" "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" @@ -82,6 +85,23 @@ func contains(modes []v1.PersistentVolumeAccessMode, mode v1.PersistentVolumeAcc return false } +func getTestCloud(t *testing.T) *azure.Cloud { + config := `{ + "aadClientId": "--aad-client-id--", + "aadClientSecret": "--aad-client-secret--" + }` + configReader := strings.NewReader(config) + cloud, err := azure.NewCloud(configReader) + if err != nil { + t.Error(err) + } + azureCloud, ok := cloud.(*azure.Cloud) + if !ok { + t.Error("NewCloud returned incorrect type") + } + return azureCloud +} + func TestPlugin(t *testing.T) { tmpDir, err := ioutil.TempDir(os.TempDir(), "azurefileTest") if err != nil { @@ -89,7 +109,7 @@ func TestPlugin(t *testing.T) { } defer os.RemoveAll(tmpDir) plugMgr := volume.VolumePluginMgr{} - plugMgr.InitPlugins(ProbeVolumePlugins(), volumetest.NewFakeVolumeHost(tmpDir, nil, nil)) + plugMgr.InitPlugins(ProbeVolumePlugins(), volumetest.NewFakeVolumeHostWithCloudProvider(tmpDir, nil, nil, getTestCloud(t))) plug, err := plugMgr.FindPluginByName("kubernetes.io/azure-file") if err != nil { diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 2ff1d1259c7..9ccd840c893 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -53,7 +53,15 @@ type fakeVolumeHost struct { } func NewFakeVolumeHost(rootDir string, kubeClient clientset.Interface, plugins []VolumePlugin) *fakeVolumeHost { - host := &fakeVolumeHost{rootDir: rootDir, kubeClient: kubeClient, cloud: nil} + return newFakeVolumeHost(rootDir, kubeClient, plugins, nil) +} + +func NewFakeVolumeHostWithCloudProvider(rootDir string, kubeClient clientset.Interface, plugins []VolumePlugin, cloud cloudprovider.Interface) *fakeVolumeHost { + return newFakeVolumeHost(rootDir, kubeClient, plugins, cloud) +} + +func newFakeVolumeHost(rootDir string, kubeClient clientset.Interface, plugins []VolumePlugin, cloud cloudprovider.Interface) *fakeVolumeHost { + host := &fakeVolumeHost{rootDir: rootDir, kubeClient: kubeClient, cloud: cloud} host.mounter = &mount.FakeMounter{} host.writer = &io.StdWriter{} host.pluginMgr.InitPlugins(plugins, host) From 599ab98f86e8a0cfbf9be9a1ccf4f6d4e49b5b2d Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Tue, 4 Jul 2017 10:29:35 +0200 Subject: [PATCH 2/9] Add the azure cloud provider dependency to azure file plugin --- pkg/volume/azure_file/BUILD | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/volume/azure_file/BUILD b/pkg/volume/azure_file/BUILD index 412c1d88fbe..ebeb07352ad 100644 --- a/pkg/volume/azure_file/BUILD +++ b/pkg/volume/azure_file/BUILD @@ -40,6 +40,8 @@ go_test( library = ":go_default_library", tags = ["automanaged"], deps = [ + "//pkg/client/clientset_generated/clientset/fake:go_default_library", + "//pkg/cloudprovider/providers/azure:go_default_library", "//pkg/util/mount:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/testing:go_default_library", From a3506c8e16ebb528bdbf80679805fe69b8c0904e Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Mon, 17 Jul 2017 09:05:03 +0200 Subject: [PATCH 3/9] Fall back on Azure public cloud endpoint when no Azure cloud provider is found --- pkg/volume/azure_file/azure_file.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index d486c7d12c0..5c5fb4395c2 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -210,11 +210,7 @@ func (b *azureFileMounter) SetUpAt(dir string, fsGroup *int64) error { } os.MkdirAll(dir, 0750) - azure, err := getAzureCloud(b.plugin.host.GetCloudProvider()) - if err != nil { - return err - } - source := fmt.Sprintf("//%s.file.%s/%s", accountName, azure.Environment.StorageEndpointSuffix, b.shareName) + source := fmt.Sprintf("//%s.file.%s/%s", accountName, getStorageEndpointSuffix(b.plugin.host.GetCloudProvider()), b.shareName) // parameters suggested by https://azure.microsoft.com/en-us/documentation/articles/storage-how-to-use-files-linux/ options := []string{fmt.Sprintf("vers=3.0,username=%s,password=%s,dir_mode=0777,file_mode=0777", accountName, accountKey)} if b.readOnly { @@ -284,3 +280,13 @@ func getAzureCloud(cloudProvider cloudprovider.Interface) (*azure.Cloud, error) return azure, nil } + +func getStorageEndpointSuffix(cloudprovider cloudprovider.Interface) string { + const publicCloudStorageEndpointSuffix = "core.windows.net" + azure, err := getAzureCloud(cloudprovider) + if err == nil { + glog.Warningf("No Azure cloud provider found. Using the Azure public cloud endpoint: %s", publicCloudStorageEndpointSuffix) + return publicCloudStorageEndpointSuffix + } + return azure.Environment.StorageEndpointSuffix +} From 44210092c1fbb60f57cc2360c2d9763ba5462598 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Mon, 17 Jul 2017 09:19:26 +0200 Subject: [PATCH 4/9] Fix comment to conform to golint --- pkg/volume/azure_file/azure_file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index 5c5fb4395c2..a4ba73faa63 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -33,7 +33,7 @@ import ( "k8s.io/kubernetes/pkg/volume/util" ) -// This is the primary entrypoint for volume plugins. +// ProbeVolumePlugins is the primary endpoint for volume plugins func ProbeVolumePlugins() []volume.VolumePlugin { return []volume.VolumePlugin{&azureFilePlugin{nil}} } From 4378c7ae8eb94b47db31b9846bf72a0e2e020270 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Mon, 17 Jul 2017 09:25:31 +0200 Subject: [PATCH 5/9] Restrict the dir and file permissions of the mounted volume --- pkg/volume/azure_file/azure_file.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index a4ba73faa63..e85d48bdceb 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -208,11 +208,11 @@ func (b *azureFileMounter) SetUpAt(dir string, fsGroup *int64) error { if accountName, accountKey, err = b.util.GetAzureCredentials(b.plugin.host, b.pod.Namespace, b.secretName); err != nil { return err } - os.MkdirAll(dir, 0750) + os.MkdirAll(dir, 0700) source := fmt.Sprintf("//%s.file.%s/%s", accountName, getStorageEndpointSuffix(b.plugin.host.GetCloudProvider()), b.shareName) // parameters suggested by https://azure.microsoft.com/en-us/documentation/articles/storage-how-to-use-files-linux/ - options := []string{fmt.Sprintf("vers=3.0,username=%s,password=%s,dir_mode=0777,file_mode=0777", accountName, accountKey)} + options := []string{fmt.Sprintf("vers=3.0,username=%s,password=%s,dir_mode=0700,file_mode=0700", accountName, accountKey)} if b.readOnly { options = append(options, "ro") } From 5c4290d4f234cbd4958e4ec5daacbbae6ffdefe8 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Mon, 17 Jul 2017 13:41:06 +0200 Subject: [PATCH 6/9] Add tests for other cloud providers --- pkg/volume/azure_file/azure_file.go | 2 +- pkg/volume/azure_file/azure_file_test.go | 29 +++++++++++++++++++++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index e85d48bdceb..426900a1f28 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -284,7 +284,7 @@ func getAzureCloud(cloudProvider cloudprovider.Interface) (*azure.Cloud, error) func getStorageEndpointSuffix(cloudprovider cloudprovider.Interface) string { const publicCloudStorageEndpointSuffix = "core.windows.net" azure, err := getAzureCloud(cloudprovider) - if err == nil { + if err != nil { glog.Warningf("No Azure cloud provider found. Using the Azure public cloud endpoint: %s", publicCloudStorageEndpointSuffix) return publicCloudStorageEndpointSuffix } diff --git a/pkg/volume/azure_file/azure_file_test.go b/pkg/volume/azure_file/azure_file_test.go index c209c0440b9..4b4e0771a4c 100644 --- a/pkg/volume/azure_file/azure_file_test.go +++ b/pkg/volume/azure_file/azure_file_test.go @@ -29,6 +29,7 @@ import ( "k8s.io/client-go/kubernetes/fake" "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/fake" "k8s.io/kubernetes/pkg/cloudprovider/providers/azure" + fakecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/fake" "k8s.io/kubernetes/pkg/util/mount" "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" @@ -85,7 +86,7 @@ func contains(modes []v1.PersistentVolumeAccessMode, mode v1.PersistentVolumeAcc return false } -func getTestCloud(t *testing.T) *azure.Cloud { +func getAzureTestCloud(t *testing.T) *azure.Cloud { config := `{ "aadClientId": "--aad-client-id--", "aadClientSecret": "--aad-client-secret--" @@ -102,14 +103,36 @@ func getTestCloud(t *testing.T) *azure.Cloud { return azureCloud } -func TestPlugin(t *testing.T) { +func getTestTempDir(t *testing.T) string { tmpDir, err := ioutil.TempDir(os.TempDir(), "azurefileTest") if err != nil { t.Fatalf("can't make a temp dir: %v", err) } + return tmpDir +} + +func TestPluginAzureCloudProvider(t *testing.T) { + tmpDir := getTestTempDir(t) defer os.RemoveAll(tmpDir) + testPlugin(t, tmpDir, volumetest.NewFakeVolumeHostWithCloudProvider(tmpDir, nil, nil, getAzureTestCloud(t))) +} + +func TestPluginWithoutCloudProvider(t *testing.T) { + tmpDir := getTestTempDir(t) + defer os.RemoveAll(tmpDir) + testPlugin(t, tmpDir, volumetest.NewFakeVolumeHost(tmpDir, nil, nil)) +} + +func TestPluginWithOtherCloudProvider(t *testing.T) { + tmpDir := getTestTempDir(t) + defer os.RemoveAll(tmpDir) + cloud := &fakecloud.FakeCloud{} + testPlugin(t, tmpDir, volumetest.NewFakeVolumeHostWithCloudProvider(tmpDir, nil, nil, cloud)) +} + +func testPlugin(t *testing.T, tmpDir string, volumeHost volume.VolumeHost) { plugMgr := volume.VolumePluginMgr{} - plugMgr.InitPlugins(ProbeVolumePlugins(), volumetest.NewFakeVolumeHostWithCloudProvider(tmpDir, nil, nil, getTestCloud(t))) + plugMgr.InitPlugins(ProbeVolumePlugins(), volumeHost) plug, err := plugMgr.FindPluginByName("kubernetes.io/azure-file") if err != nil { From 6c3a8531499ff1a641194017117e5d999a5baad8 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Mon, 17 Jul 2017 14:32:05 +0200 Subject: [PATCH 7/9] Add the fake cloud provider to azure file build --- pkg/volume/azure_file/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/volume/azure_file/BUILD b/pkg/volume/azure_file/BUILD index ebeb07352ad..f22fa037c88 100644 --- a/pkg/volume/azure_file/BUILD +++ b/pkg/volume/azure_file/BUILD @@ -42,6 +42,7 @@ go_test( deps = [ "//pkg/client/clientset_generated/clientset/fake:go_default_library", "//pkg/cloudprovider/providers/azure:go_default_library", + "//pkg/cloudprovider/providers/fake:go_default_library", "//pkg/util/mount:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/testing:go_default_library", From 7ae381207e051f859c54c052b6f8fcc57d6f77c4 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Wed, 19 Jul 2017 08:59:32 +0200 Subject: [PATCH 8/9] Remove unused import after rebase --- pkg/volume/azure_file/azure_file_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/volume/azure_file/azure_file_test.go b/pkg/volume/azure_file/azure_file_test.go index 4b4e0771a4c..bb1a5ad3ad2 100644 --- a/pkg/volume/azure_file/azure_file_test.go +++ b/pkg/volume/azure_file/azure_file_test.go @@ -27,7 +27,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/fake" - "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/fake" "k8s.io/kubernetes/pkg/cloudprovider/providers/azure" fakecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/fake" "k8s.io/kubernetes/pkg/util/mount" From 95cf81f8330799c5e42ba12857c7aca659b40c91 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Wed, 19 Jul 2017 10:18:51 +0200 Subject: [PATCH 9/9] Remove clientset from azure file test build --- pkg/volume/azure_file/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/volume/azure_file/BUILD b/pkg/volume/azure_file/BUILD index f22fa037c88..cb14f5d3e78 100644 --- a/pkg/volume/azure_file/BUILD +++ b/pkg/volume/azure_file/BUILD @@ -40,7 +40,6 @@ go_test( library = ":go_default_library", tags = ["automanaged"], deps = [ - "//pkg/client/clientset_generated/clientset/fake:go_default_library", "//pkg/cloudprovider/providers/azure:go_default_library", "//pkg/cloudprovider/providers/fake:go_default_library", "//pkg/util/mount:go_default_library",