Merge pull request #488 from yiannistri/132-fix-tags

[v2.9] Fix panic when updating tags
rancher · Apr 30, 2024 · 944b0de · 944b0de
2 parents 6476395 + 8aca81f
commit 944b0de
Show file tree

Hide file tree

Showing 15 changed files with 32 additions and 44 deletions.
diff --git a/controller/aks-cluster-config-handler.go b/controller/aks-cluster-config-handler.go
@@ -4,15 +4,13 @@ import (
 	"context"
 	"encoding/base64"
 	"fmt"
-	"net/http"
 	"reflect"
 	"regexp"
 	"strings"
 	"time"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
 	"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-11-01/containerservice"
 	"github.com/rancher/aks-operator/pkg/aks"
 	"github.com/rancher/aks-operator/pkg/aks/services"
@@ -26,7 +24,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
-	"k8s.io/client-go/util/retry"
 )
 
 const (
@@ -733,7 +730,7 @@ func (h *Handler) updateUpstreamClusterState(ctx context.Context, config *aksv1.
 		if !reflect.DeepEqual(config.Spec.Tags, upstreamSpec.Tags) {
 			logrus.Infof("Updating tags for cluster [%s]", config.Spec.ClusterName)
 			tags := armcontainerservice.TagsObject{
-				Tags: *aks.StringMapPtr(config.Spec.Tags),
+				Tags: aks.StringMapPtr(config.Spec.Tags),
 			}
 
 			poller, err := h.azureClients.clustersClient.BeginUpdateTags(ctx, config.Spec.ResourceGroup, config.Spec.ClusterName, tags, nil)
@@ -745,22 +742,13 @@ func (h *Handler) updateUpstreamClusterState(ctx context.Context, config *aksv1.
 			// have a good way to detect that policy. We handle this case by checking if Azure returns an unexpected
 			// state for the tags and if so, log the response and move on. Any upstream tags regenerated on the cluster
 			// by Azure will be synced back to rancher.
-			var response *http.Response
-			upstreamTags := armcontainerservice.TagsObject{}
-			if err := retry.OnError(retry.DefaultBackoff,
-				func(err error) bool {
-					return strings.HasSuffix(err.Error(), "asynchronous operation has not completed")
-				},
-				func() error {
-					resp, err := poller.PollUntilDone(runtime.WithCaptureResponse(ctx, &response), nil)
-					upstreamTags.Tags = resp.Tags
-					return err
-				}); err != nil {
+			res, err := poller.PollUntilDone(ctx, nil)
+			if err != nil {
 				return config, fmt.Errorf("failed to update tags for cluster [%s]: %w", config.Spec.ClusterName, err)
 			}
 
-			if !reflect.DeepEqual(tags, upstreamTags) && response.StatusCode == http.StatusOK {
-				logrus.Infof("Tags were not updated as expected for cluster [%s], expected %s, actual %s, moving on", config.Spec.ClusterName, aks.StringMap(tags.Tags), aks.StringMap(upstreamTags.Tags))
+			if !reflect.DeepEqual(tags, res.Tags) {
+				logrus.Infof("Tags were not updated as expected for cluster [%s], expected %s, actual %s, moving on", config.Spec.ClusterName, aks.StringMap(tags.Tags), aks.StringMap(res.Tags))
 			} else {
 				return h.enqueueUpdate(config)
 			}

diff --git a/controller/aks-cluster-config-handler_test.go b/controller/aks-cluster-config-handler_test.go
@@ -4,7 +4,7 @@ import (
 	"errors"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -806,7 +806,7 @@ var _ = Describe("buildUpstreamClusterState", func() {
 						MaxCount:            to.Ptr(int32(1)),
 						MinCount:            to.Ptr(int32(1)),
 						VnetSubnetID:        to.Ptr("test"),
-						NodeLabels:          *aks.StringMapPtr(map[string]string{"test": "test"}),
+						NodeLabels:          aks.StringMapPtr(map[string]string{"test": "test"}),
 						NodeTaints:          utils.ConvertToSliceOfPointers(to.Ptr([]string{"test"})),
 						UpgradeSettings: &armcontainerservice.AgentPoolUpgradeSettings{
 							MaxSurge: to.Ptr("test"),
@@ -848,7 +848,7 @@ var _ = Describe("buildUpstreamClusterState", func() {
 					PrivateDNSZone:       to.Ptr("test-private-dns-zone-id"),
 				},
 			},
-			Tags: *aks.StringMapPtr(map[string]string{"test": "test"}),
+			Tags: aks.StringMapPtr(map[string]string{"test": "test"}),
 		}
 
 		handler = &Handler{

diff --git a/go.mod b/go.mod
@@ -8,7 +8,7 @@ require (
 	github.com/Azure/azure-sdk-for-go v55.7.0+incompatible
 	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.11.1
 	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.5.2
-	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4 v4.8.0
+	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5 v5.0.0
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/operationalinsights/armoperationalinsights v1.2.0
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.2.0
 	github.com/Azure/go-autorest/autorest v0.11.29
@@ -35,7 +35,7 @@ require (
 )
 
 require (
-	github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.2 // indirect
+	github.com/Azure/azure-sdk-for-go/sdk/internal v1.6.0 // indirect
 	github.com/Azure/go-autorest v14.2.0+incompatible // indirect
 	github.com/Azure/go-autorest/autorest/adal v0.9.23 // indirect
 	github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect

diff --git a/go.sum b/go.sum
@@ -6,10 +6,10 @@ github.com/Azure/azure-sdk-for-go/sdk/azcore v1.11.1 h1:E+OJmp2tPvt1W+amx48v1eqb
 github.com/Azure/azure-sdk-for-go/sdk/azcore v1.11.1/go.mod h1:a6xsAQUZg+VsS3TJ05SRp524Hs4pZ/AeFSr5ENf0Yjo=
 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.5.2 h1:FDif4R1+UUR+00q6wquyX90K7A8dN+R5E8GEadoP7sU=
 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.5.2/go.mod h1:aiYBYui4BJ/BJCAIKs92XiPyQfTaBWqvHujDwKb6CBU=
-github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.2 h1:LqbJ/WzJUwBf8UiaSzgX7aMclParm9/5Vgp+TY51uBQ=
-github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.2/go.mod h1:yInRyqWXAuaPrgI7p70+lDDgh3mlBohis29jGMISnmc=
-github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4 v4.8.0 h1:0nGmzwBv5ougvzfGPCO2ljFRHvun57KpNrVCMrlk0ns=
-github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4 v4.8.0/go.mod h1:gYq8wyDgv6JLhGbAU6gg8amCPgQWRE+aCvrV2gyzdfs=
+github.com/Azure/azure-sdk-for-go/sdk/internal v1.6.0 h1:sUFnFjzDUie80h24I7mrKtwCKgLY9L8h5Tp2x9+TWqk=
+github.com/Azure/azure-sdk-for-go/sdk/internal v1.6.0/go.mod h1:52JbnQTp15qg5mRkMBHwp0j0ZFwHJ42Sx3zVV5RE9p0=
+github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5 v5.0.0 h1:5n7dPVqsWfVKw+ZiEKSd3Kzu7gwBkbEBkeXb8rgaE9Q=
+github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5 v5.0.0/go.mod h1:HcZY0PHPo/7d75p99lB6lK0qYOP4vLRJUBpiehYXtLQ=
 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/internal/v2 v2.0.0 h1:PTFGRSlMKCQelWwxUyYVEUqseBJVemLyqWJjvMyt0do=
 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/internal/v2 v2.0.0/go.mod h1:LRr2FzBTQlONPPa5HREE5+RjSCTXl7BwOvYOaWTqCaI=
 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/managementgroups/armmanagementgroups v1.0.0 h1:pPvTJ1dY0sA35JOeFq6TsY2xj6Z85Yo23Pj4wCCvu4o=

diff --git a/pkg/aks/convert.go b/pkg/aks/convert.go
@@ -30,13 +30,13 @@ func StringSlice(s *[]string) []string {
 	return nil
 }
 
-// StringMapPtr returns a pointer to a map of string pointers built from the passed map of strings.
-func StringMapPtr(ms map[string]string) *map[string]*string {
+// StringMapPtr returns a map of string pointers built from the passed map of strings.
+func StringMapPtr(ms map[string]string) map[string]*string {
 	msp := make(map[string]*string, len(ms))
 	for k, s := range ms {
 		msp[k] = to.Ptr(s)
 	}
-	return &msp
+	return msp
 }
 
 // StringMap returns a map of strings built from the map of string pointers. The empty string is

diff --git a/pkg/aks/create.go b/pkg/aks/create.go
@@ -6,7 +6,7 @@ import (
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
 	"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-11-01/containerservice"
 	"github.com/rancher/aks-operator/pkg/aks/services"

diff --git a/pkg/aks/create_test.go b/pkg/aks/create_test.go
@@ -5,7 +5,7 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/operationalinsights/armoperationalinsights"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
 	. "github.com/onsi/ginkgo/v2"

diff --git a/pkg/aks/delete_test.go b/pkg/aks/delete_test.go
@@ -1,7 +1,7 @@
 package aks
 
 import (
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/rancher/aks-operator/pkg/aks/services/mock_services"

diff --git a/pkg/aks/get.go b/pkg/aks/get.go
@@ -3,7 +3,7 @@ package aks
 import (
 	"context"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
 	"github.com/rancher/aks-operator/pkg/aks/services"
 )
 

diff --git a/pkg/aks/services/agentpools.go b/pkg/aks/services/agentpools.go
@@ -8,7 +8,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
 )
 
 type AgentPoolsClientInterface interface {

diff --git a/pkg/aks/services/managedclusters.go b/pkg/aks/services/managedclusters.go
@@ -8,7 +8,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
 )
 
 type Poller[T any] interface {
@@ -20,7 +20,7 @@ type ManagedClustersClientInterface interface {
 	Get(ctx context.Context, resourceGroupName string, resourceName string, options *armcontainerservice.ManagedClustersClientGetOptions) (armcontainerservice.ManagedClustersClientGetResponse, error)
 	BeginDelete(ctx context.Context, resourceGroupName string, resourceName string, options *armcontainerservice.ManagedClustersClientBeginDeleteOptions) (Poller[armcontainerservice.ManagedClustersClientDeleteResponse], error)
 	GetAccessProfile(ctx context.Context, resourceGroupName string, resourceName string, roleName string, options *armcontainerservice.ManagedClustersClientGetAccessProfileOptions) (armcontainerservice.ManagedClustersClientGetAccessProfileResponse, error)
-	BeginUpdateTags(ctx context.Context, resourceGroupName string, resourceName string, parameters armcontainerservice.TagsObject, options *armcontainerservice.ManagedClustersClientBeginUpdateTagsOptions) (*runtime.Poller[armcontainerservice.ManagedClustersClientUpdateTagsResponse], error)
+	BeginUpdateTags(ctx context.Context, resourceGroupName string, resourceName string, parameters armcontainerservice.TagsObject, options *armcontainerservice.ManagedClustersClientBeginUpdateTagsOptions) (Poller[armcontainerservice.ManagedClustersClientUpdateTagsResponse], error)
 }
 
 type managedClustersClient struct {
@@ -59,6 +59,6 @@ func (cl *managedClustersClient) GetAccessProfile(ctx context.Context, resourceG
 	return cl.armManagedClustersClient.GetAccessProfile(ctx, resourceGroupName, resourceName, roleName, options)
 }
 
-func (cl *managedClustersClient) BeginUpdateTags(ctx context.Context, resourceGroupName string, resourceName string, parameters armcontainerservice.TagsObject, options *armcontainerservice.ManagedClustersClientBeginUpdateTagsOptions) (*runtime.Poller[armcontainerservice.ManagedClustersClientUpdateTagsResponse], error) {
+func (cl *managedClustersClient) BeginUpdateTags(ctx context.Context, resourceGroupName string, resourceName string, parameters armcontainerservice.TagsObject, options *armcontainerservice.ManagedClustersClientBeginUpdateTagsOptions) (Poller[armcontainerservice.ManagedClustersClientUpdateTagsResponse], error) {
 	return cl.armManagedClustersClient.BeginUpdateTags(ctx, resourceGroupName, resourceName, parameters, options)
 }
diff --git a/pkg/aks/services/mock_services/agentpools_mock.go b/pkg/aks/services/mock_services/agentpools_mock.go
diff --git a/pkg/aks/services/mock_services/managedclusters_mock.go b/pkg/aks/services/mock_services/managedclusters_mock.go
diff --git a/pkg/aks/update.go b/pkg/aks/update.go
@@ -3,7 +3,7 @@ package aks
 import (
 	"context"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
 	"github.com/rancher/aks-operator/pkg/aks/services"
 	aksv1 "github.com/rancher/aks-operator/pkg/apis/aks.cattle.io/v1"
 	"github.com/sirupsen/logrus"

diff --git a/pkg/aks/update_test.go b/pkg/aks/update_test.go
@@ -4,7 +4,7 @@ import (
 	"errors"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
-	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v5"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/rancher/aks-operator/pkg/aks/services/mock_services"