Applied Vishu's Comments

Author: HigashikataZhangsuke

azure/const.go

@@ -56,4 +56,10 @@ const (
 	// ReplicasManagedByAutoscalerAnnotation is set to true in the corresponding capi machine pool
 	// when an external autoscaler manages the node count of the associated machine pool.
 	ReplicasManagedByAutoscalerAnnotation = "cluster.x-k8s.io/replicas-managed-by-autoscaler"
+
+	// DisablePrivateDNSAnnotation is the key for the Azure Cluster object annotation
+	// which disables private DNS zone creation when set to "true".
+	// See https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/
+	// for annotation formatting rules.
+	DisablePrivateDNSAnnotation = "capz.io/disable-private-dns"
 )

azure/scope/azure_secret_cloud.go

@@ -69,9 +69,6 @@ func init() {
 
 				if env, err := azure.EnvironmentFromFile(filePath); err == nil {
 					azure.SetEnvironment(env.Name, env)
-					//fmt.Printf("CAPZ: Successfully loaded Azure environment: %s\n", env.Name)
-					//fmt.Printf("CAPZ: ResourceManagerEndpoint: %s\n", env.ResourceManagerEndpoint)
-					//fmt.Printf("CAPZ: ActiveDirectoryEndpoint: %s\n", env.ActiveDirectoryEndpoint)
 					log.Info("loaded Azure environment from file", "EnvName", env.Name)
 					log.Info("loaded Azure environment from file", "filename", file.Name())
 				} else {

azure/scope/cluster.go

@@ -565,7 +565,7 @@ func (s *ClusterScope) VNetSpec() azure.ASOResourceSpecGetter[*asonetworkv1api20
 func (s *ClusterScope) PrivateDNSSpec() (zoneSpec azure.ResourceSpecGetter, linkSpec, recordSpec []azure.ResourceSpecGetter) {
 	// Check for bypass annotation
 	if s.AzureCluster.Annotations != nil {
-		if bypass, exists := s.AzureCluster.Annotations["capz.io/disable-private-dns"]; exists && bypass == "true" {
+		if bypass, exists := s.AzureCluster.Annotations[azure.DisablePrivateDNSAnnotation]; exists && bypass == "true" {
 			return nil, nil, nil
 		}
 	}

azure/services/managedclusters/managedclusters.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/cluster-api/util/secret"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/conversion"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
@@ -142,35 +143,34 @@ func postCreateOrUpdateResourceHook(ctx context.Context, scope ManagedClusterSco
   The user needs to ensure to provide service principal with admin AAD privileges.
 */
 func reconcileKubeconfig(ctx context.Context, scope ManagedClusterScope, namespace string) (adminKubeConfigData []byte, userKubeConfigData []byte, err error) {
-	fmt.Printf("=== DEBUG: reconcileKubeconfig() called for cluster: %s ===\n", scope.ClusterName())
-	fmt.Printf("DEBUG: Namespace: %s\n", namespace)
-	fmt.Printf("DEBUG: IsAADEnabled: %v\n", scope.IsAADEnabled())
-	fmt.Printf("DEBUG: AreLocalAccountsDisabled: %v\n", scope.AreLocalAccountsDisabled())
+	logger := log.FromContext(ctx)
+	logger.V(1).Info("reconcileKubeconfig called", "cluster", scope.ClusterName())
+	logger.V(1).Info("kubeconfig reconciliation details", "namespace", namespace, "isAADEnabled", scope.IsAADEnabled(), "areLocalAccountsDisabled", scope.AreLocalAccountsDisabled())
 
 	if scope.IsAADEnabled() {
-		fmt.Printf("DEBUG: AAD is enabled, getting user kubeconfig data\n")
+		logger.V(1).Info("AAD is enabled, getting user kubeconfig data")
 		if userKubeConfigData, err = getUserKubeconfigData(ctx, scope, namespace); err != nil {
-			fmt.Printf("DEBUG: ERROR - Failed to get user kubeconfig: %v\n", err)
+			logger.Error(err, "failed to get user kubeconfig")
 			return nil, nil, errors.Wrap(err, "error while trying to get user kubeconfig")
 		}
-		fmt.Printf("DEBUG: Got user kubeconfig data: %d bytes\n", len(userKubeConfigData))
+		logger.V(1).Info("got user kubeconfig data", "bytes", len(userKubeConfigData))
 	}
 
 	if scope.AreLocalAccountsDisabled() {
-		fmt.Printf("DEBUG: Local accounts disabled, using user kubeconfig with token path\n")
+		logger.V(1).Info("local accounts disabled, using user kubeconfig with token path")
 		userKubeconfigWithToken, err := getUserKubeConfigWithToken(ctx, userKubeConfigData, scope)
 		if err != nil {
-			fmt.Printf("DEBUG: ERROR - Failed to get user kubeconfig with token: %v\n", err)
+			logger.Error(err, "failed to get user kubeconfig with token")
 			return nil, nil, errors.Wrap(err, "error while trying to get user kubeconfig with token")
 		}
-		fmt.Printf("DEBUG: Successfully got user kubeconfig with token: %d bytes\n", len(userKubeconfigWithToken))
+		logger.V(1).Info("successfully got user kubeconfig with token", "bytes", len(userKubeconfigWithToken))
 		return userKubeconfigWithToken, userKubeConfigData, nil
 	}
 
-	fmt.Printf("DEBUG: Using admin kubeconfig path (local accounts enabled)\n")
+	logger.V(1).Info("using admin kubeconfig path (local accounts enabled)")
 	asoSecret := &corev1.Secret{}
 	secretName := adminKubeconfigSecretName(scope.ClusterName())
-	fmt.Printf("DEBUG: Looking for ASO admin kubeconfig secret: %s/%s\n", namespace, secretName)
+	logger.V(1).Info("looking for ASO admin kubeconfig secret", "namespace", namespace, "secretName", secretName)
 
 	err = scope.GetClient().Get(
 		ctx,
@@ -181,15 +181,14 @@ func reconcileKubeconfig(ctx context.Context, scope ManagedClusterScope, namespa
 		asoSecret,
 	)
 	if err != nil {
-		fmt.Printf("DEBUG: ERROR - Failed to get ASO admin kubeconfig secret: %v\n", err)
+		logger.Error(err, "failed to get ASO admin kubeconfig secret")
 		return nil, nil, errors.Wrap(err, "failed to get ASO admin kubeconfig secret")
 	}
 
 	adminKubeConfigData = asoSecret.Data[secret.KubeconfigDataName]
-	fmt.Printf("DEBUG: Retrieved admin kubeconfig from ASO secret: %d bytes\n", len(adminKubeConfigData))
+	logger.V(1).Info("retrieved admin kubeconfig from ASO secret", "bytes", len(adminKubeConfigData))
 
-	fmt.Printf("DEBUG: reconcileKubeconfig() completed - admin: %d bytes, user: %d bytes\n",
-		len(adminKubeConfigData), len(userKubeConfigData))
+	logger.V(1).Info("reconcileKubeconfig completed", "adminBytes", len(adminKubeConfigData), "userBytes", len(userKubeConfigData))
 	return adminKubeConfigData, userKubeConfigData, nil
 }
 
@@ -232,10 +231,3 @@ func getUserKubeConfigWithToken(ctx context.Context, userKubeConfigData []byte,
 	}
 	return kubeconfig, nil
 }
-
-func min(a, b int) int {
-	if a < b {
-		return a
-	}
-	return b
-}

azure/services/privatedns/link_reconciler.go

@@ -80,12 +80,12 @@ func (s *Service) reconcileLinks(ctx context.Context, links []azure.ResourceSpec
 			}
 			for _, link := range links {
 				if link.Properties != nil && link.Properties.VirtualNetwork != nil && link.Properties.VirtualNetwork.ID != nil && *link.Properties.VirtualNetwork.ID == vnetID {
-					// log.V(1).Info("Skipping vnet link creation as VNet is already linked to a zone with the same name across resource groups",
-					// 	"vnet link", linkSpec.ResourceName(),
-					// 	"private dns zone", zoneName,
-					// 	"vnet", linkSpec.(LinkSpec).VNetName,
-					// 	"resource group", linkSpec.ResourceGroupName(),
-					// 	"zone resource group", zoneRG)
+					log.V(1).Info("Skipping vnet link creation as VNet is already linked to a zone with the same name across resource groups",
+						"vnet link", linkSpec.ResourceName(),
+						"private dns zone", zoneName,
+						"vnet", linkSpec.(LinkSpec).VNetName,
+						"resource group", linkSpec.ResourceGroupName(),
+						"zone resource group", zoneRG)
 					alreadyLinked = true
 					break
 				}

controllers/azureasomanagedcontrolplane_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math"
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
@@ -417,7 +418,7 @@ func getCustomCACertificateForCluster(clusterName string) []byte {
 		if len(azure.AzSecretCertData) > 0 {
 			fmt.Printf("DEBUG: ASO - Found certificate data, returning %d bytes\n", len(azure.AzSecretCertData))
 			fmt.Printf("DEBUG: ASO - Certificate data preview (first 100 chars): %s...\n",
-				string(azure.AzSecretCertData[:min(100, len(azure.AzSecretCertData))]))
+				string(azure.AzSecretCertData[:int(math.Min(100, float64(len(azure.AzSecretCertData))))]))
 			return azure.AzSecretCertData
 		} else {
 			fmt.Printf("DEBUG: ASO - Certificate data is empty, returning nil\n")
@@ -430,13 +431,6 @@ func getCustomCACertificateForCluster(clusterName string) []byte {
 	return nil
 }
 
-func min(a, b int) int {
-	if a < b {
-		return a
-	}
-	return b
-}
-
 func (r *AzureASOManagedControlPlaneReconciler) reconcilePaused(ctx context.Context, asoManagedControlPlane *infrav1alpha.AzureASOManagedControlPlane) (ctrl.Result, error) {
 	ctx, log, done := tele.StartSpanWithLogger(ctx, "controllers.AzureASOManagedControlPlaneReconciler.reconcilePaused")
 	defer done()

controllers/azuremanagedcontrolplane_reconciler.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"math"
 
 	"github.com/pkg/errors"
 	"k8s.io/client-go/tools/clientcmd"
@@ -262,7 +263,7 @@ func getCustomCACertificateForAMCP(clusterName string) []byte {
 		if len(azure.AzSecretCertData) > 0 {
 			fmt.Printf("DEBUG: AMCP - Found certificate data, returning %d bytes\n", len(azure.AzSecretCertData))
 			fmt.Printf("DEBUG: AMCP - Certificate data preview (first 100 chars): %s...\n",
-				string(azure.AzSecretCertData[:min(100, len(azure.AzSecretCertData))]))
+				string(azure.AzSecretCertData[:int(math.Min(100, float64(len(azure.AzSecretCertData))))]))
 			return azure.AzSecretCertData
 		} else {
 			fmt.Printf("DEBUG: AMCP - Certificate data is empty, returning nil\n")