Repository: kubernetes-sigs/cloud-provider-azure
Pull request #662 (2021-06-16 08:25:40)

chore: set default config secret name and namespace
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/release.md#issue-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://git.k8s.io/community/contributors/devel/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
6. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
> Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespaces from that line:
>
> /kind api-change
> /kind bug
> /kind cleanup
> /kind design
> /kind documentation
> /kind failing-test
> /kind feature
> /kind flake

/kind cleanup

**What this PR does / why we need it**:

chore: set default config secret name and namespace

**Which issue(s) this PR fixes**:
<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

**Special notes for your reviewer**:


**Release note**:
```
chore: set default config secret name and namespace
```


k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/cloud-provider-azure/pull/662#" title="Author self-approved">nilo19</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fcloud-provider-azure).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/cloud-provider-azure/blob/master/OWNERS)~~ [nilo19]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
k8s-ci-robot: @nilo19: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-cloud-provider-azure-check | 985f9654341b560a63b1f1b834ac9cbfb4783fee | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cloud-provider-azure/662/pull-cloud-provider-azure-check/1405160689990897664) | `/test pull-cloud-provider-azure-check`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=cloud-provider-azure&pr=662). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Anilo19). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/cloud-provider-azure/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
coveralls: 
[![Coverage Status](https://coveralls.io/builds/40634409/badge)](https://coveralls.io/builds/40634409)

Coverage decreased (-0.04%) to 79.256% when pulling **eb7c1884e21b404420be8741f3fe16db12f2224b on nilo19:chore/default-secret** into **47fb9f1ab4694bf51677ef60aa170918b944f387 on kubernetes-sigs:master**.

k8s-ci-robot: @nilo19: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-cloud-provider-azure-e2e-ccm | eb7c1884e21b404420be8741f3fe16db12f2224b | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cloud-provider-azure/662/pull-cloud-provider-azure-e2e-ccm/1405161713417850880) | `/test pull-cloud-provider-azure-e2e-ccm`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=cloud-provider-azure&pr=662). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Anilo19). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/cloud-provider-azure/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
nilo19: /retest

Patch:
diff --git a/README.md b/README.md
index 722579326..d79d4cd99 100644
--- a/README.md
+++ b/README.md
@@ -1,7 +1,6 @@
 # Cloud provider for Azure
 
 [![Go Report Card](https://goreportcard.com/badge/sigs.k8s.io/cloud-provider-azure)](https://goreportcard.com/report/sigs.k8s.io/cloud-provider-azure)
-[![Coverage Status](https://coveralls.io/repos/github/kubernetes-sigs/cloud-provider-azure/badge.svg?branch=master)](https://coveralls.io/github/kubernetes-sigs/cloud-provider-azure?branch=master)
 [![GitHub stars](https://img.shields.io/github/stars/kubernetes-sigs/cloud-provider-azure.svg)](https://github.com/kubernetes-sigs/cloud-provider-azure/stargazers)
 [![GitHub stars](https://img.shields.io/badge/contributions-welcome-orange.svg)](https://github.com/kubernetes-sigs/cloud-provider-azure/blob/master/CONTRIBUTING.md)
 
diff --git a/cmd/cloud-controller-manager/app/options/dynamic.go b/cmd/cloud-controller-manager/app/options/dynamic.go
index 05ee991d7..10ae67e2c 100644
--- a/cmd/cloud-controller-manager/app/options/dynamic.go
+++ b/cmd/cloud-controller-manager/app/options/dynamic.go
@@ -37,7 +37,7 @@ func (o *DynamicReloadingOptions) AddFlags(fs *pflag.FlagSet) {
 	}
 
 	fs.BoolVar(&o.EnableDynamicReloading, "enable-dynamic-reloading", false, "Enable re-configuring cloud controller manager from secret without restarting")
-	fs.StringVar(&o.CloudConfigSecretName, "cloud-config-secret-name", "cloud-provider-config", "The name of the cloud config secret, default to 'cloud-provider-config'")
+	fs.StringVar(&o.CloudConfigSecretName, "cloud-config-secret-name", "azure-cloud-provider", "The name of the cloud config secret, default to 'cloud-provider-config'")
 	fs.StringVar(&o.CloudConfigSecretNamespace, "cloud-config-secret-namespace", "kube-system", "The k8s namespace of the cloud config secret, default to 'kube-system'")
 	fs.StringVar(&o.CloudConfigKey, "cloud-config-key", "cloud-config", "The key of the config data in the cloud config secret, default to 'cloud-config'")
 }
@@ -64,8 +64,8 @@ func (o *DynamicReloadingOptions) Validate() []error {
 func defaultDynamicReloadingOptions() *DynamicReloadingOptions {
 	return &DynamicReloadingOptions{
 		EnableDynamicReloading:     false,
-		CloudConfigSecretName:      "",
-		CloudConfigSecretNamespace: "",
+		CloudConfigSecretName:      "azure-cloud-provider",
+		CloudConfigSecretNamespace: "kube-system",
 		CloudConfigKey:             "",
 	}
 }
diff --git a/pkg/consts/consts.go b/pkg/consts/consts.go
index ecd02d4ed..af1f983bf 100644
--- a/pkg/consts/consts.go
+++ b/pkg/consts/consts.go
@@ -340,3 +340,10 @@ const (
 	RouteNameFmt       = "%s____%s"
 	RouteNameSeparator = "____"
 )
+
+// cloud provider config secret
+const (
+	DefaultCloudProviderConfigSecName      = "azure-cloud-provider"
+	DefaultCloudProviderConfigSecNamespace = "kube-system"
+	DefaultCloudProviderConfigSecKey       = "cloud-config"
+)
diff --git a/pkg/provider/azure.go b/pkg/provider/azure.go
index dbe954a28..c06295f8e 100644
--- a/pkg/provider/azure.go
+++ b/pkg/provider/azure.go
@@ -372,6 +372,24 @@ func NewCloudFromConfigFile(configFilePath string, syncZones bool) (cloudprovide
 	return cloud, nil
 }
 
+func (az *Cloud) configSecretMetadata(secretName, secretNamespace, cloudConfigKey string) {
+	if secretName == "" {
+		secretName = consts.DefaultCloudProviderConfigSecName
+	}
+	if secretNamespace == "" {
+		secretNamespace = consts.DefaultCloudProviderConfigSecNamespace
+	}
+	if cloudConfigKey == "" {
+		cloudConfigKey = consts.DefaultCloudProviderConfigSecKey
+	}
+
+	az.InitSecretConfig = InitSecretConfig{
+		SecretName:      secretName,
+		SecretNamespace: secretNamespace,
+		CloudConfigKey:  cloudConfigKey,
+	}
+}
+
 func NewCloudFromSecret(clientBuilder cloudprovider.ControllerClientBuilder, secretName, secretNamespace, cloudConfigKey string) (cloudprovider.Interface, error) {
 	az := &Cloud{
 		nodeNames:          sets.NewString(),
@@ -379,13 +397,10 @@ func NewCloudFromSecret(clientBuilder cloudprovider.ControllerClientBuilder, sec
 		nodeResourceGroups: map[string]string{},
 		unmanagedNodes:     sets.NewString(),
 		routeCIDRs:         map[string]string{},
-		InitSecretConfig: InitSecretConfig{
-			SecretName:      secretName,
-			SecretNamespace: secretNamespace,
-			CloudConfigKey:  cloudConfigKey,
-		},
 	}
 
+	az.configSecretMetadata(secretName, secretNamespace, cloudConfigKey)
+
 	az.Initialize(clientBuilder, wait.NeverStop)
 
 	err := az.InitializeCloudFromSecret()
@@ -552,7 +567,7 @@ func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret, syncZones
 		// wait for the success first time of syncing zones
 		err = az.syncRegionZonesMap()
 		if err != nil {
-			klog.Errorf("InitializeCloudFromConfig: failed to sync regional zones map for the first time: %s", err.Error())
+			klog.Errorf("InitializeCloudFromConfig: failed eto sync regional zones map for the first time: %s", err.Error())
 			return err
 		}
 
diff --git a/pkg/provider/azure_routes.go b/pkg/provider/azure_routes.go
index 6b2b32747..ea025e6ff 100644
--- a/pkg/provider/azure_routes.go
+++ b/pkg/provider/azure_routes.go
@@ -146,17 +146,12 @@ func (d *delayedRouteUpdater) updateRoutes() {
 	}
 
 	// reconcile routes.
-	dirty, onlyUpdateTags := false, true
+	dirty := false
 	routes := []network.Route{}
 	if routeTable.RouteTablePropertiesFormat != nil && routeTable.RouteTablePropertiesFormat.Routes != nil {
 		routes = *routeTable.Routes
 	}
-
-	routes, dirty = d.cleanupOutdatedRoutes(routes)
-	if dirty {
-		onlyUpdateTags = false
-	}
-
+	onlyUpdateTags := true
 	for _, rt := range d.routesToUpdate {
 		if rt.operation == routeTableOperationUpdateTags {
 			routeTable.Tags = rt.routeTableTags
@@ -209,34 +204,6 @@ func (d *delayedRouteUpdater) updateRoutes() {
 	}
 }
 
-// cleanupOutdatedRoutes deletes all non-dualstack routes when dualstack is enabled,
-// and deletes all dualstack routes when dualstack is not enabled.
-func (d *delayedRouteUpdater) cleanupOutdatedRoutes(existingRoutes []network.Route) (routes []network.Route, changed bool) {
-	for i := len(existingRoutes) - 1; i >= 0; i-- {
-		existingRouteName := to.String(existingRoutes[i].Name)
-		split := strings.Split(existingRouteName, consts.RouteNameSeparator)
-
-		// filter out unmanaged routes
-		deleteRoute := false
-		if d.az.nodeNames.Has(split[0]) {
-			if d.az.ipv6DualStackEnabled && len(split) == 1 {
-				klog.V(2).Infof("cleanupOutdatedRoutes: deleting outdated non-dualstack route %s", existingRouteName)
-				deleteRoute = true
-			} else if !d.az.ipv6DualStackEnabled && len(split) == 2 {
-				klog.V(2).Infof("cleanupOutdatedRoutes: deleting outdated dualstack route %s", existingRouteName)
-				deleteRoute = true
-			}
-
-			if deleteRoute {
-				existingRoutes = append(existingRoutes[:i], existingRoutes[i+1:]...)
-				changed = true
-			}
-		}
-	}
-
-	return existingRoutes, changed
-}
-
 // addRouteOperation adds the routeOperation to delayedRouteUpdater and returns a delayedRouteOperation.
 func (d *delayedRouteUpdater) addRouteOperation(operation routeOperation, route network.Route) (*delayedRouteOperation, error) {
 	d.lock.Lock()


Test patch:
diff --git a/.github/workflows/test-coverage.yml b/.github/workflows/test-coverage.yml
deleted file mode 100644
index c768c9362..000000000
--- a/.github/workflows/test-coverage.yml
+++ /dev/null
@@ -1,27 +0,0 @@
-name: Test Coverage
-on:
-    pull_request: {}
-    push: {}
-jobs:
-  build:
-    name: Build
-    runs-on: ubuntu-latest
-    steps:
-
-    - name: Set up Go 1.x
-      uses: actions/setup-go@v2
-      with:
-        go-version: ^1.16
-      id: go
-
-    - name: Check out code into the Go module directory
-      uses: actions/checkout@v2
-
-    - name: Build Test
-      run: sudo go test -covermode=count -coverprofile=profile.cov ./pkg/...
-    - name: Send coverage
-      env:
-        COVERALLS_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-      run: |
-        GO111MODULE=off go get github.com/mattn/goveralls
-        $(go env GOPATH)/bin/goveralls -coverprofile=profile.cov -service=github
diff --git a/cmd/cloud-controller-manager/app/options/options_test.go b/cmd/cloud-controller-manager/app/options/options_test.go
index deb07a9d0..ce705e78b 100644
--- a/cmd/cloud-controller-manager/app/options/options_test.go
+++ b/cmd/cloud-controller-manager/app/options/options_test.go
@@ -141,8 +141,8 @@ func TestDefaultFlags(t *testing.T) {
 		NodeStatusUpdateFrequency: metav1.Duration{Duration: 5 * time.Minute},
 		DynamicReloading: &DynamicReloadingOptions{
 			EnableDynamicReloading:     false,
-			CloudConfigSecretName:      "",
-			CloudConfigSecretNamespace: "",
+			CloudConfigSecretName:      "azure-cloud-provider",
+			CloudConfigSecretNamespace: "kube-system",
 			CloudConfigKey:             "",
 		},
 	}
diff --git a/pkg/provider/azure_config_test.go b/pkg/provider/azure_config_test.go
index b9b087157..68350af17 100644
--- a/pkg/provider/azure_config_test.go
+++ b/pkg/provider/azure_config_test.go
@@ -20,6 +20,8 @@ import (
 	"context"
 	"testing"
 
+	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
+
 	"github.com/golang/mock/gomock"
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/zoneclient/mockzoneclient"
 
@@ -286,3 +288,36 @@ func TestInitializeCloudFromSecret(t *testing.T) {
 		})
 	}
 }
+
+func TestConfigSecretMetadata(t *testing.T) {
+	for _, testCase := range []struct {
+		description                                                         string
+		secretName, secretNamespace, cloudConfigKey                         string
+		expectedsecretName, expectedSsecretNamespace, expectedClouConfigKey string
+	}{
+		{
+			description:              "configSecretMetadata should set the secret metadata from the given parameters",
+			secretName:               "cloud-provider-config",
+			secretNamespace:          "123456",
+			cloudConfigKey:           "azure.json",
+			expectedsecretName:       "cloud-provider-config",
+			expectedSsecretNamespace: "123456",
+			expectedClouConfigKey:    "azure.json",
+		},
+		{
+			description:              "configSecretMetadata should set the secret metadata from the default values",
+			expectedsecretName:       consts.DefaultCloudProviderConfigSecName,
+			expectedSsecretNamespace: consts.DefaultCloudProviderConfigSecNamespace,
+			expectedClouConfigKey:    consts.DefaultCloudProviderConfigSecKey,
+		},
+	} {
+		t.Run(testCase.description, func(t *testing.T) {
+			az := &Cloud{}
+			az.configSecretMetadata(testCase.secretName, testCase.secretNamespace, testCase.cloudConfigKey)
+
+			assert.Equal(t, testCase.expectedsecretName, az.SecretName)
+			assert.Equal(t, testCase.expectedSsecretNamespace, az.SecretNamespace)
+			assert.Equal(t, testCase.expectedClouConfigKey, az.CloudConfigKey)
+		})
+	}
+}
diff --git a/pkg/provider/azure_routes_test.go b/pkg/provider/azure_routes_test.go
index 93b27113b..ab68ae2c2 100644
--- a/pkg/provider/azure_routes_test.go
+++ b/pkg/provider/azure_routes_test.go
@@ -79,7 +79,7 @@ func TestDeleteRoute(t *testing.T) {
 			Routes: &[]network.Route{},
 		},
 	}
-	routeTableClient.EXPECT().Get(gomock.Any(), cloud.RouteTableResourceGroup, cloud.RouteTableName, "").Return(routeTables, nil).AnyTimes()
+	routeTableClient.EXPECT().Get(gomock.Any(), cloud.RouteTableResourceGroup, cloud.RouteTableName, "").Return(routeTables, nil)
 	routeTableClient.EXPECT().CreateOrUpdate(gomock.Any(), cloud.RouteTableResourceGroup, cloud.RouteTableName, routeTablesAfterDeletion, "").Return(nil)
 	err := cloud.DeleteRoute(context.TODO(), "cluster", &route)
 	if err != nil {
@@ -664,81 +664,3 @@ func TestListRoutes(t *testing.T) {
 		}
 	}
 }
-
-func TestCleanupOutdatedRoutes(t *testing.T) {
-	ctrl := gomock.NewController(t)
-	defer ctrl.Finish()
-
-	for _, testCase := range []struct {
-		description                          string
-		existingRoutes, expectedRoutes       []network.Route
-		existingNodeNames                    sets.String
-		expectedChanged, enableIPV6DualStack bool
-	}{
-		{
-			description: "cleanupOutdatedRoutes should delete outdated non-dualstack routes when dualstack is enabled",
-			existingRoutes: []network.Route{
-				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
-				{Name: to.StringPtr("aks-node1-vmss000000")},
-			},
-			expectedRoutes: []network.Route{
-				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
-			},
-			existingNodeNames:   sets.NewString("aks-node1-vmss000000"),
-			enableIPV6DualStack: true,
-			expectedChanged:     true,
-		},
-		{
-			description: "cleanupOutdatedRoutes should delete outdated dualstack routes when dualstack is disabled",
-			existingRoutes: []network.Route{
-				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
-				{Name: to.StringPtr("aks-node1-vmss000000")},
-			},
-			expectedRoutes: []network.Route{
-				{Name: to.StringPtr("aks-node1-vmss000000")},
-			},
-			existingNodeNames: sets.NewString("aks-node1-vmss000000"),
-			expectedChanged:   true,
-		},
-		{
-			description: "cleanupOutdatedRoutes should not delete unmanaged routes when dualstack is enabled",
-			existingRoutes: []network.Route{
-				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
-				{Name: to.StringPtr("aks-node1-vmss000000")},
-			},
-			expectedRoutes: []network.Route{
-				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
-				{Name: to.StringPtr("aks-node1-vmss000000")},
-			},
-			existingNodeNames:   sets.NewString("aks-node1-vmss000001"),
-			enableIPV6DualStack: true,
-		},
-		{
-			description: "cleanupOutdatedRoutes should not delete unmanaged routes when dualstack is disabled",
-			existingRoutes: []network.Route{
-				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
-				{Name: to.StringPtr("aks-node1-vmss000000")},
-			},
-			expectedRoutes: []network.Route{
-				{Name: to.StringPtr("aks-node1-vmss000000____xxx")},
-				{Name: to.StringPtr("aks-node1-vmss000000")},
-			},
-			existingNodeNames: sets.NewString("aks-node1-vmss000001"),
-		},
-	} {
-		t.Run(testCase.description, func(t *testing.T) {
-			cloud := &Cloud{
-				ipv6DualStackEnabled: testCase.enableIPV6DualStack,
-				nodeNames:            testCase.existingNodeNames,
-			}
-
-			d := &delayedRouteUpdater{
-				az: cloud,
-			}
-
-			routes, changed := d.cleanupOutdatedRoutes(testCase.existingRoutes)
-			assert.Equal(t, testCase.expectedChanged, changed)
-			assert.Equal(t, testCase.expectedRoutes, routes)
-		})
-	}
-}