Fix the issue when ControlPlaneEndpoint was not updated on AzureManag…

…edControlPlane correctly. Before this fix, the issue happens when the create request failed/timed out and later update flow was not reading from the cluster fetched from cloud.
kubernetes-sigs · Mar 7, 2022 · 679375f · 679375f
1 parent 7f605d5
commit 679375f
Show file tree

Hide file tree

Showing 2 changed files with 117 additions and 4 deletions.
diff --git a/azure/services/managedclusters/managedclusters.go b/azure/services/managedclusters/managedclusters.go
@@ -295,8 +295,11 @@ func (s *Service) Reconcile(ctx context.Context) error {
 	}
 
 	customHeaders := maps.FilterByKeyPrefix(s.Scope.ManagedClusterAnnotations(), azure.CustomHeaderPrefix)
+	// Use the MC fetched from Azure if no update is needed. This is to ensure the read-only fields like Fqdn from the
+	// existing MC are used for updating the AzureManagedCluster.
+	result := existingMC
 	if isCreate {
-		managedCluster, err = s.Client.CreateOrUpdate(ctx, managedClusterSpec.ResourceGroupName, managedClusterSpec.Name, managedCluster, customHeaders)
+		result, err = s.Client.CreateOrUpdate(ctx, managedClusterSpec.ResourceGroupName, managedClusterSpec.Name, managedCluster, customHeaders)
 		if err != nil {
 			return fmt.Errorf("failed to create managed cluster, %w", err)
 		}
@@ -325,20 +328,23 @@ func (s *Service) Reconcile(ctx context.Context) error {
 		diff := computeDiffOfNormalizedClusters(managedCluster, existingMC)
 		if diff != "" {
 			klog.V(2).Infof("Update required (+new -old):\n%s", diff)
-			managedCluster, err = s.Client.CreateOrUpdate(ctx, managedClusterSpec.ResourceGroupName, managedClusterSpec.Name, managedCluster, customHeaders)
+			result, err = s.Client.CreateOrUpdate(ctx, managedClusterSpec.ResourceGroupName, managedClusterSpec.Name, managedCluster, customHeaders)
 			if err != nil {
 				return fmt.Errorf("failed to update managed cluster, %w", err)
 			}
 		}
 	}
 
 	// Update control plane endpoint.
-	if managedCluster.ManagedClusterProperties != nil && managedCluster.ManagedClusterProperties.Fqdn != nil {
+	if result.ManagedClusterProperties != nil && result.ManagedClusterProperties.Fqdn != nil {
 		endpoint := clusterv1.APIEndpoint{
-			Host: *managedCluster.ManagedClusterProperties.Fqdn,
+			Host: *result.ManagedClusterProperties.Fqdn,
 			Port: 443,
 		}
 		s.Scope.SetControlPlaneEndpoint(endpoint)
+	} else {
+		// Fail if cluster api endpoint is not available.
+		return fmt.Errorf("failed to get API endpoint for managed cluster")
 	}
 
 	// Update kubeconfig data

diff --git a/azure/services/managedclusters/managedclusters_test.go b/azure/services/managedclusters/managedclusters_test.go
@@ -29,6 +29,7 @@ import (
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/managedclusters/mock_managedclusters"
 	gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
 func TestReconcile(t *testing.T) {
@@ -123,16 +124,118 @@ func TestReconcile(t *testing.T) {
 		{
 			name:          "no managedcluster exists",
 			expectedError: "",
+			expect: func(m *mock_managedclusters.MockClientMockRecorder, s *mock_managedclusters.MockManagedClusterScopeMockRecorder) {
+				m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-managedcluster", gomock.Any(), gomock.Any()).Return(containerservice.ManagedCluster{ManagedClusterProperties: &containerservice.ManagedClusterProperties{
+					Fqdn:              pointer.String("my-managedcluster-fqdn"),
+					ProvisioningState: pointer.String("Succeeded"),
+				}}, nil).Times(1)
+				m.Get(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return(containerservice.ManagedCluster{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found"))
+				m.GetCredentials(gomockinternal.AContext(), "my-rg", "my-managedcluster").Times(1)
+				s.ClusterName().AnyTimes().Return("my-managedcluster")
+				s.ResourceGroup().AnyTimes().Return("my-rg")
+				s.ManagedClusterAnnotations().Times(1).Return(map[string]string{})
+				s.ManagedClusterSpec().AnyTimes().Return(azure.ManagedClusterSpec{
+					Name:              "my-managedcluster",
+					ResourceGroupName: "my-rg",
+				}, nil)
+				s.GetAllAgentPoolSpecs(gomockinternal.AContext()).AnyTimes().Return([]azure.AgentPoolSpec{
+					{
+						Name:         "my-agentpool",
+						SKU:          "Standard_D4s_v3",
+						Replicas:     1,
+						OSDiskSizeGB: 0,
+					},
+				}, nil)
+				s.SetControlPlaneEndpoint(gomock.Eq(clusterv1.APIEndpoint{
+					Host: "my-managedcluster-fqdn",
+					Port: 443,
+				})).Times(1)
+				s.SetKubeConfigData(gomock.Any()).Times(1)
+			},
+		},
+		{
+			name:          "missing fqdn after create",
+			expectedError: "failed to get API endpoint for managed cluster",
 			expect: func(m *mock_managedclusters.MockClientMockRecorder, s *mock_managedclusters.MockManagedClusterScopeMockRecorder) {
 				m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-managedcluster", gomock.Any(), gomock.Any()).Return(containerservice.ManagedCluster{ManagedClusterProperties: &containerservice.ManagedClusterProperties{}}, nil)
 				m.Get(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return(containerservice.ManagedCluster{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found"))
+				s.ClusterName().AnyTimes().Return("my-managedcluster")
+				s.ResourceGroup().AnyTimes().Return("my-rg")
+				s.ManagedClusterAnnotations().Times(1).Return(map[string]string{})
+				s.ManagedClusterSpec().AnyTimes().Return(azure.ManagedClusterSpec{
+					Name:              "my-managedcluster",
+					ResourceGroupName: "my-rg",
+				}, nil)
+				s.GetAllAgentPoolSpecs(gomockinternal.AContext()).AnyTimes().Return([]azure.AgentPoolSpec{
+					{
+						Name:         "my-agentpool",
+						SKU:          "Standard_D4s_v3",
+						Replicas:     1,
+						OSDiskSizeGB: 0,
+					},
+				}, nil)
+			},
+		},
+		{
+			name:          "set correct ControlPlaneEndpoint using fqdn from existing MC after update",
+			expectedError: "",
+			expect: func(m *mock_managedclusters.MockClientMockRecorder, s *mock_managedclusters.MockManagedClusterScopeMockRecorder) {
+				m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-managedcluster", gomock.Any(), gomock.Any()).Return(containerservice.ManagedCluster{
+					ManagedClusterProperties: &containerservice.ManagedClusterProperties{
+						Fqdn:              pointer.String("my-managedcluster-fqdn"),
+						ProvisioningState: pointer.String("Succeeded"),
+					},
+				}, nil)
+				m.Get(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return(containerservice.ManagedCluster{
+					ManagedClusterProperties: &containerservice.ManagedClusterProperties{
+						Fqdn:              pointer.String("my-managedcluster-fqdn"),
+						ProvisioningState: pointer.String("Succeeded"),
+						NetworkProfile:    &containerservice.NetworkProfile{},
+					},
+				}, nil).Times(1)
+				m.GetCredentials(gomockinternal.AContext(), "my-rg", "my-managedcluster").Times(1)
+				s.ClusterName().AnyTimes().Return("my-managedcluster")
+				s.ResourceGroup().AnyTimes().Return("my-rg")
+				s.ManagedClusterAnnotations().Times(1).Return(map[string]string{})
+				s.ManagedClusterSpec().AnyTimes().Return(azure.ManagedClusterSpec{
+					Name:              "my-managedcluster",
+					ResourceGroupName: "my-rg",
+				}, nil)
+				s.GetAllAgentPoolSpecs(gomockinternal.AContext()).AnyTimes().Return([]azure.AgentPoolSpec{
+					{
+						Name:         "my-agentpool",
+						SKU:          "Standard_D4s_v3",
+						Replicas:     1,
+						OSDiskSizeGB: 0,
+					},
+				}, nil)
+				s.SetControlPlaneEndpoint(gomock.Eq(clusterv1.APIEndpoint{
+					Host: "my-managedcluster-fqdn",
+					Port: 443,
+				})).Times(1)
+				s.SetKubeConfigData(gomock.Any()).Times(1)
+			},
+		},
+		{
+			name:          "no update needed - set correct ControlPlaneEndpoint using fqdn from existing MC",
+			expectedError: "",
+			expect: func(m *mock_managedclusters.MockClientMockRecorder, s *mock_managedclusters.MockManagedClusterScopeMockRecorder) {
+				m.Get(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return(containerservice.ManagedCluster{
+					ManagedClusterProperties: &containerservice.ManagedClusterProperties{
+						Fqdn:              pointer.String("my-managedcluster-fqdn"),
+						ProvisioningState: pointer.String("Succeeded"),
+						KubernetesVersion: pointer.String("1.1"),
+						NetworkProfile:    &containerservice.NetworkProfile{},
+					},
+				}, nil).Times(1)
 				m.GetCredentials(gomockinternal.AContext(), "my-rg", "my-managedcluster").Times(1)
 				s.ClusterName().AnyTimes().Return("my-managedcluster")
 				s.ResourceGroup().AnyTimes().Return("my-rg")
 				s.ManagedClusterAnnotations().Times(1).Return(map[string]string{})
 				s.ManagedClusterSpec().AnyTimes().Return(azure.ManagedClusterSpec{
 					Name:              "my-managedcluster",
 					ResourceGroupName: "my-rg",
+					Version:           "1.1",
 				}, nil)
 				s.GetAllAgentPoolSpecs(gomockinternal.AContext()).AnyTimes().Return([]azure.AgentPoolSpec{
 					{
@@ -142,6 +245,10 @@ func TestReconcile(t *testing.T) {
 						OSDiskSizeGB: 0,
 					},
 				}, nil)
+				s.SetControlPlaneEndpoint(gomock.Eq(clusterv1.APIEndpoint{
+					Host: "my-managedcluster-fqdn",
+					Port: 443,
+				})).Times(1)
 				s.SetKubeConfigData(gomock.Any()).Times(1)
 			},
 		},