From 0febb20d8e3adc7e5ff1d93a5cdd13192449f93e Mon Sep 17 00:00:00 2001 From: Pengfei Ni Date: Thu, 30 May 2019 20:10:14 -0700 Subject: [PATCH] Cleanup kubelet checking and cloudConfigScope option --- .../k8s.io/legacy-cloud-providers/azure/BUILD | 1 - .../legacy-cloud-providers/azure/azure.go | 51 +++------------- .../azure/azure_config.go | 44 ++------------ .../azure/azure_config_test.go | 60 ++++++++++--------- .../azure/azure_wrap.go | 15 ----- .../src/k8s.io/legacy-cloud-providers/go.mod | 1 - .../src/k8s.io/legacy-cloud-providers/go.sum | 2 - 7 files changed, 46 insertions(+), 128 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD b/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD index a3460b5500f..71509f32feb 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD @@ -69,7 +69,6 @@ go_library( "//vendor/github.com/Azure/go-autorest/autorest/adal:go_default_library", "//vendor/github.com/Azure/go-autorest/autorest/azure:go_default_library", "//vendor/github.com/Azure/go-autorest/autorest/to:go_default_library", - "//vendor/github.com/kardianos/osext:go_default_library", "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library", "//vendor/github.com/rubiojr/go-vhd/vhd:go_default_library", "//vendor/k8s.io/klog:go_default_library", diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go index d30a66a4166..161aed3621c 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go @@ -68,10 +68,6 @@ const ( externalResourceGroupLabel = "kubernetes.azure.com/resource-group" managedByAzureLabel = "kubernetes.azure.com/managed" - - // the prefix of secret for Azure cloud provider. The secret should include - // base64-encoded cloud config data with key 'cloud-config'. - azureSecretNamePrefix = "azure-cloud-provider" ) var ( @@ -162,8 +158,6 @@ type Config struct { // The cloud configure type for Azure cloud provider. Supported values are file, secret and merge. CloudConfigType cloudConfigType `json:"cloudConfigType,omitempty" yaml:"cloudConfigType,omitempty"` - // The cloud config scope for Azure cloud provider. Supported values are all, node and control-plane. - CloudConfigScope cloudConfigScope `json:"cloudConfigScope,omitempty" yaml:"cloudConfigScope,omitempty"` } var _ cloudprovider.Interface = (*Cloud)(nil) @@ -286,19 +280,6 @@ func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret bool) erro } } - if config.CloudConfigScope == "" { - // The default config scope is cloudConfigScopeAll. - config.CloudConfigScope = cloudConfigScopeAll - } else { - supportedCloudConfigScopes := sets.NewString( - string(cloudConfigScopeAll), - string(cloudConfigScopeNode), - string(cloudConfigScopeControlPlane)) - if !supportedCloudConfigScopes.Has(string(config.CloudConfigScope)) { - return fmt.Errorf("cloudConfigScope %v is not supported, supported values are %v", config.CloudConfigScope, supportedCloudConfigScopes.List()) - } - } - env, err := auth.ParseAzureEnvironment(config.Cloud) if err != nil { return err @@ -306,32 +287,18 @@ func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret bool) erro servicePrincipalToken, err := auth.GetServicePrincipalToken(&config.AzureAuthConfig, env) if err == auth.ErrorNoAuth { - runingAsKubelet, err := isRunningAsKubelet() - if err != nil { + // Only controller-manager would lazy-initialize from secret, and credentials are required for such case. + if fromSecret { + err := fmt.Errorf("No credentials provided for Azure cloud provider") + klog.Fatalf("%v", err) return err } - if runingAsKubelet { - // No credentials provided, useInstanceMetadata should be enabled for Kubelet. - if !config.UseInstanceMetadata { - return fmt.Errorf("useInstanceMetadata must be enabled without Azure credentials") - } - } else { - // Credentials are required for controller-manager for lazy initialization from secret. - if fromSecret { - err := fmt.Errorf("No credentials provided for Azure cloud provider") - klog.Fatalf("%v", err) - return err - } - - // Credentials are required if cloud config type is "file". - if az.Config.CloudConfigType == cloudConfigTypeFile { - return fmt.Errorf("no credentials provided for Azure cloud provider") - } - - // Controller manager could be initialized from secret. - klog.V(2).Infof("No credentials provided, lazy initialize from secret %s", getConfigSecretName(az.Config.CloudConfigScope)) - return nil + // No credentials provided, useInstanceMetadata should be enabled for Kubelet. + // TODO(feiskyer): print different error message for Kubelet and controller-manager, as they're + // requiring different credential settings. + if !config.UseInstanceMetadata && az.Config.CloudConfigType == cloudConfigTypeFile { + return fmt.Errorf("useInstanceMetadata must be enabled without Azure credentials") } klog.V(2).Infof("Azure cloud provider is starting without credentials") diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_config.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_config.go index 209e9fd5ea1..569f44c48b1 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_config.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_config.go @@ -25,25 +25,9 @@ import ( ) const ( - cloudConfigNamespace = "kube-system" - cloudConfigKey = "cloud-config" -) - -// The configure scope for Azure cloud provider secret. Supported values are: -// * all : configure applied for components (kubelet and controller-manager). This is the default value. -// * node : configure applied for nodes (kubelet). -// * control-plane : configure applied for control plane components (controller-manager). -// -// For different configure scope, the secret name would also be different: -// * all : secret name would be azure-cloud-provider. -// * node : secret name would azure-cloud-provider-node. -// * control-plane : secret name would be azure-cloud-provider-control-plane. -type cloudConfigScope string - -const ( - cloudConfigScopeAll cloudConfigScope = "all" - cloudConfigScopeNode cloudConfigScope = "node" - cloudConfigScopeControlPlane cloudConfigScope = "control-plane" + cloudConfigNamespace = "kube-system" + cloudConfigKey = "cloud-config" + cloudConfigSecretName = "azure-cloud-provider" ) // The config type for Azure cloud provider secret. Supported values are: @@ -82,15 +66,14 @@ func (az *Cloud) getConfigFromSecret() (*Config, error) { return nil, nil } - secretName := getConfigSecretName(az.Config.CloudConfigScope) - secret, err := az.kubeClient.CoreV1().Secrets(cloudConfigNamespace).Get(secretName, metav1.GetOptions{}) + secret, err := az.kubeClient.CoreV1().Secrets(cloudConfigNamespace).Get(cloudConfigSecretName, metav1.GetOptions{}) if err != nil { - return nil, fmt.Errorf("Failed to get secret %s: %v", secretName, err) + return nil, fmt.Errorf("Failed to get secret %s: %v", cloudConfigSecretName, err) } cloudConfigData, ok := secret.Data[cloudConfigKey] if !ok { - return nil, fmt.Errorf("cloud-config is not set in the secret (%s)", secretName) + return nil, fmt.Errorf("cloud-config is not set in the secret (%s)", cloudConfigSecretName) } config := Config{} @@ -106,18 +89,3 @@ func (az *Cloud) getConfigFromSecret() (*Config, error) { return &config, nil } - -func getConfigSecretName(scope cloudConfigScope) string { - switch scope { - case cloudConfigScopeAll: - return azureSecretNamePrefix - case cloudConfigScopeNode: - return fmt.Sprintf("%s-node", azureSecretNamePrefix) - case cloudConfigScopeControlPlane: - return fmt.Sprintf("%s-control-plane", azureSecretNamePrefix) - - default: - // default secret name is azure-cloud-provider. - return azureSecretNamePrefix - } -} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_config_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_config_test.go index 52d5b20c59d..e505900d491 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_config_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_config_test.go @@ -129,38 +129,40 @@ func TestGetConfigFromSecret(t *testing.T) { } for _, test := range tests { - az := &Cloud{ - kubeClient: fakeclient.NewSimpleClientset(), - } - if test.existingConfig != nil { - az.Config = *test.existingConfig - } - if test.secretConfig != nil { - secret := &v1.Secret{ - Type: v1.SecretTypeOpaque, - ObjectMeta: metav1.ObjectMeta{ - Name: "azure-cloud-provider", - Namespace: "kube-system", - }, + t.Run(test.name, func(t *testing.T) { + az := &Cloud{ + kubeClient: fakeclient.NewSimpleClientset(), } - if test.secretConfig != emptyConfig { - secretData, err := yaml.Marshal(test.secretConfig) - assert.NoError(t, err, test.name) - secret.Data = map[string][]byte{ - "cloud-config": secretData, + if test.existingConfig != nil { + az.Config = *test.existingConfig + } + if test.secretConfig != nil { + secret := &v1.Secret{ + Type: v1.SecretTypeOpaque, + ObjectMeta: metav1.ObjectMeta{ + Name: "azure-cloud-provider", + Namespace: "kube-system", + }, } + if test.secretConfig != emptyConfig { + secretData, err := yaml.Marshal(test.secretConfig) + assert.NoError(t, err, test.name) + secret.Data = map[string][]byte{ + "cloud-config": secretData, + } + } + _, err := az.kubeClient.CoreV1().Secrets(cloudConfigNamespace).Create(secret) + assert.NoError(t, err, test.name) } - _, err := az.kubeClient.CoreV1().Secrets(cloudConfigNamespace).Create(secret) + + real, err := az.getConfigFromSecret() + if test.expectErr { + assert.Error(t, err, test.name) + return + } + assert.NoError(t, err, test.name) - } - - real, err := az.getConfigFromSecret() - if test.expectErr { - assert.Error(t, err, test.name) - continue - } - - assert.NoError(t, err, test.name) - assert.Equal(t, test.expected, real, test.name) + assert.Equal(t, test.expected, real, test.name) + }) } } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_wrap.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_wrap.go index 7b6f270c489..98f58b29b47 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_wrap.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_wrap.go @@ -19,7 +19,6 @@ package azure import ( "fmt" "net/http" - "path/filepath" "regexp" "strings" "time" @@ -27,7 +26,6 @@ import ( "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-07-01/network" "github.com/Azure/go-autorest/autorest" - "github.com/kardianos/osext" "k8s.io/apimachinery/pkg/types" cloudprovider "k8s.io/cloud-provider" "k8s.io/klog" @@ -364,16 +362,3 @@ func isBackendPoolOnSameLB(newBackendPoolID string, existingBackendPools []strin return true, "", nil } - -func isRunningAsKubelet() (bool, error) { - exe, err := osext.Executable() - if err != nil { - return false, fmt.Errorf("cloud not find the service executable: %v", err) - } - - if strings.Contains(filepath.Base(exe), "kubelet") { - return true, nil - } - - return false, nil -} diff --git a/staging/src/k8s.io/legacy-cloud-providers/go.mod b/staging/src/k8s.io/legacy-cloud-providers/go.mod index 50d242e7718..61b90c135fc 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/go.mod +++ b/staging/src/k8s.io/legacy-cloud-providers/go.mod @@ -11,7 +11,6 @@ require ( github.com/GoogleCloudPlatform/k8s-cloud-provider v0.0.0-20181220005116-f8e995905100 github.com/aws/aws-sdk-go v1.16.26 github.com/dnaeon/go-vcr v1.0.1 // indirect - github.com/kardianos/osext v0.0.0-20150410034420-8fef92e41e22 github.com/marstr/guid v0.0.0-20170427235115-8bdf7d1a087c // indirect github.com/prometheus/client_golang v0.9.2 github.com/rubiojr/go-vhd v0.0.0-20160810183302-0bfd3b39853c diff --git a/staging/src/k8s.io/legacy-cloud-providers/go.sum b/staging/src/k8s.io/legacy-cloud-providers/go.sum index 87f72f4bae8..8e6a72c3505 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/go.sum +++ b/staging/src/k8s.io/legacy-cloud-providers/go.sum @@ -48,8 +48,6 @@ github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af h1:pmfjZENx5i github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/json-iterator/go v0.0.0-20180701071628-ab8a2e0c74be h1:AHimNtVIpiBjPUhEF5KNCkrUyqTSA5zWUl8sQ2bfGBE= github.com/json-iterator/go v0.0.0-20180701071628-ab8a2e0c74be/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU= -github.com/kardianos/osext v0.0.0-20150410034420-8fef92e41e22 h1:eLCQd4nxsC7sumkwNg4OiB6bGiD7I5l1MSfBAxmxkKQ= -github.com/kardianos/osext v0.0.0-20150410034420-8fef92e41e22/go.mod h1:1NbS8ALrpOvjt0rHPNLyCIeMtbizbir8U//inJ+zuB8= github.com/marstr/guid v0.0.0-20170427235115-8bdf7d1a087c h1:N7uWGS2fTwH/4BwxbHiJZNAFTSJ5yPU0emHsQWvkxEY= github.com/marstr/guid v0.0.0-20170427235115-8bdf7d1a087c/go.mod h1:74gB1z2wpxxInTG6yaqA7KrtM0NZ+RbrcqDvYHefzho= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU=