Merge pull request #1297 from CecileRobertMichon/cherry-pick-1293

[backport] Fix VM provider ID to match node ID

cloud/converters/identity.go

@@ -19,6 +19,8 @@ package converters
 import (
 	"strings"
 
+	azure "sigs.k8s.io/cluster-api-provider-azure/cloud"
+
 	"github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute"
 	"github.com/pkg/errors"
 
@@ -59,7 +61,7 @@ func UserAssignedIdentitiesToVMSSSDK(identities []infrav1.UserAssignedIdentity)
 	return userIdentitiesMap, nil
 }
 
-// sanitized removes "azure:///" prefix from the given id
+// sanitized removes "azure://" prefix from the given id
 func sanitized(id string) string {
-	return strings.TrimPrefix(id, "azure:///")
+	return strings.TrimPrefix(id, azure.ProviderIDPrefix)
 }

cloud/converters/identity_test.go

@@ -38,14 +38,14 @@ var sampleSubjectFactory = []infrav1.UserAssignedIdentity{
 }
 
 var expectedVMSDKObject = map[string]*compute.VirtualMachineIdentityUserAssignedIdentitiesValue{
-	"foo":             {},
-	"bar":             {},
+	"/foo":            {},
+	"/bar":            {},
 	"/without/prefix": {},
 }
 
 var expectedVMSSSDKObject = map[string]*compute.VirtualMachineScaleSetIdentityUserAssignedIdentitiesValue{
-	"foo":             {},
-	"bar":             {},
+	"/foo":            {},
+	"/bar":            {},
 	"/without/prefix": {},
 }
 

cloud/defaults.go

@@ -65,6 +65,12 @@ const (
 	ControlPlaneNodeGroup = "control-plane"
 )
 
+const (
+	// ProviderIDPrefix will be appended to the beginning of Azure resource IDs to form the Kubernetes Provider ID.
+	// NOTE: this format matches the 2 slashes format used in cloud-provider and cluster-autoscaler.
+	ProviderIDPrefix = "azure://"
+)
+
 // GenerateBackendAddressPoolName generates a load balancer backend address pool name.
 func GenerateBackendAddressPoolName(lbName string) string {
 	return fmt.Sprintf("%s-%s", lbName, "backendPool")

cloud/scope/machinepool.go

@@ -19,7 +19,6 @@ package scope
 import (
 	"context"
 	"encoding/base64"
-	"fmt"
 
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/go-logr/logr"
@@ -168,7 +167,7 @@ func (m *MachinePoolScope) UpdateInstanceStatuses(ctx context.Context, instances
 
 	providerIDs := make([]string, len(instances))
 	for i, instance := range instances {
-		providerIDs[i] = fmt.Sprintf("azure://%s", instance.ID)
+		providerIDs[i] = azure.ProviderIDPrefix + instance.ID
 	}
 
 	nodeStatusByProviderID, err := m.getNodeStatusByProviderID(ctx, providerIDs)
@@ -180,7 +179,7 @@ func (m *MachinePoolScope) UpdateInstanceStatuses(ctx context.Context, instances
 	instanceStatuses := make([]*infrav1exp.AzureMachinePoolInstanceStatus, len(instances))
 	for i, instance := range instances {
 		instanceStatuses[i] = &infrav1exp.AzureMachinePoolInstanceStatus{
-			ProviderID:        fmt.Sprintf("azure://%s", instance.ID),
+			ProviderID:        azure.ProviderIDPrefix + instance.ID,
 			InstanceID:        instance.InstanceID,
 			InstanceName:      instance.Name,
 			ProvisioningState: &instance.State,

cloud/services/scalesets/scalesets.go

@@ -117,7 +117,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
 		}
 	default:
 		// just in case, set the provider ID if the instance exists
-		s.Scope.SetProviderID(fmt.Sprintf("azure://%s", fetchedVMSS.ID))
+		s.Scope.SetProviderID(azure.ProviderIDPrefix + fetchedVMSS.ID)
 	}
 
 	// Try to get the VMSS to update status if we have created a long running operation. If the VMSS is still in a long
@@ -193,7 +193,7 @@ func (s *Service) patchVMSSIfNeeded(ctx context.Context, infraVMSS *infrav1exp.V
 	ctx, span := tele.Tracer().Start(ctx, "scalesets.Service.patchVMSSIfNeeded")
 	defer span.End()
 
-	s.Scope.SetProviderID(fmt.Sprintf("azure://%s", infraVMSS.ID))
+	s.Scope.SetProviderID(azure.ProviderIDPrefix + infraVMSS.ID)
 
 	spec := s.Scope.ScaleSetSpec()
 	result, err := s.buildVMSSFromSpec(ctx, spec)

cloud/services/scalesets/scalesets_test.go

@@ -18,7 +18,6 @@ package scalesets
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"testing"
 
@@ -258,7 +257,7 @@ func TestReconcileVMSS(t *testing.T) {
 				createdVMSS := newDefaultVMSS()
 				instances := newDefaultInstances()
 				createdVMSS = setupDefaultVMSSInProgressOperationDoneExpectations(g, s, m, createdVMSS, instances)
-				s.SetProviderID(fmt.Sprintf("azure://%s", *createdVMSS.ID))
+				s.SetProviderID(azure.ProviderIDPrefix + *createdVMSS.ID)
 				s.SetLongRunningOperationState(nil)
 				s.SetProvisioningState(infrav1.VMStateSucceeded)
 				s.NeedsK8sVersionUpdate().Return(false)
@@ -279,7 +278,7 @@ func TestReconcileVMSS(t *testing.T) {
 				vmss := newDefaultVMSS()
 				instances := newDefaultInstances()
 				vmss = setupDefaultVMSSInProgressOperationDoneExpectations(g, s, m, vmss, instances)
-				s.SetProviderID(fmt.Sprintf("azure://%s", *vmss.ID))
+				s.SetProviderID(azure.ProviderIDPrefix + *vmss.ID)
 				s.SetProvisioningState(infrav1.VMStateUpdating)
 
 				// create a VMSS patch with an updated hash to match the spec
@@ -387,7 +386,7 @@ func TestReconcileVMSS(t *testing.T) {
 				spec.Identity = infrav1.VMIdentityUserAssigned
 				spec.UserAssignedIdentities = []infrav1.UserAssignedIdentity{
 					{
-						ProviderID: "azure:////subscriptions/123/resourcegroups/456/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id1",
+						ProviderID: "azure:///subscriptions/123/resourcegroups/456/providers/Microsoft.ManagedIdentity/userAssignedIdentities/id1",
 					},
 				}
 				s.ScaleSetSpec().Return(spec).AnyTimes()
@@ -1009,7 +1008,7 @@ func setupDefaultVMSSExpectations(s *mock_scalesets.MockScaleSetScopeMockRecorde
 
 func setupDefaultVMSSUpdateExpectations(s *mock_scalesets.MockScaleSetScopeMockRecorder) {
 	setupDefaultVMSSExpectations(s)
-	s.SetProviderID("azure://vmss-id")
+	s.SetProviderID(azure.ProviderIDPrefix + "vmss-id")
 	s.SetProvisioningState(infrav1.VMStateUpdating)
 	s.GetLongRunningOperationState().Return(nil)
 }

cloud/services/virtualmachines/virtualmachines.go

@@ -93,7 +93,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
 		return errors.Wrapf(err, "failed to get VM %s", vmSpec.Name)
 	case err == nil:
 		// VM already exists, update the spec and skip creation.
-		s.Scope.SetProviderID(fmt.Sprintf("azure:///%s", existingVM.ID))
+		s.Scope.SetProviderID(azure.ProviderIDPrefix + existingVM.ID)
 		s.Scope.SetAnnotation("cluster-api-provider-azure", "true")
 		s.Scope.SetAddresses(existingVM.Addresses)
 		s.Scope.SetVMState(existingVM.State)

exp/controllers/azuremanagedmachinepool_reconciler.go

@@ -152,7 +152,7 @@ func (s *azureManagedMachinePoolService) Reconcile(ctx context.Context, scope *s
 
 	var providerIDs = make([]string, len(instances))
 	for i := 0; i < len(instances); i++ {
-		providerIDs[i] = fmt.Sprintf("azure://%s", *instances[i].ID)
+		providerIDs[i] = azure.ProviderIDPrefix + *instances[i].ID
 	}
 
 	scope.InfraMachinePool.Spec.ProviderIDList = providerIDs