Skip to content

Commit 46f5aa4

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2541 from k8s-infra-cherrypick-robot/cherry-pick-2536-to-release-1.4
[release-1.4] enforce lowercase providerID RG to match cloud-provider-azure
2 parents 06b60a8 + 8b162c5 commit 46f5aa4

File tree

7 files changed

+150
-11
lines changed

7 files changed

+150
-11
lines changed

‎azure/services/scalesets/scalesets.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"sigs.k8s.io/cluster-api-provider-azure/azure"
3030
"sigs.k8s.io/cluster-api-provider-azure/azure/converters"
3131
"sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus"
32+
azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure"
3233
"sigs.k8s.io/cluster-api-provider-azure/util/generators"
3334
"sigs.k8s.io/cluster-api-provider-azure/util/slice"
3435
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
@@ -104,7 +105,12 @@ func (s *Service) Reconcile(ctx context.Context) (retErr error) {
104105
}
105106

106107
if fetchedVMSS != nil {
107-
s.Scope.SetProviderID(azure.ProviderIDPrefix + fetchedVMSS.ID)
108+
// Transform the VMSS resource representation to conform to the cloud-provider-azure representation
109+
providerID, err := azureutil.ConvertResourceGroupNameToLower(azure.ProviderIDPrefix + fetchedVMSS.ID)
110+
if err != nil {
111+
log.Error(err, "failed to parse VMSS ID", "ID", fetchedVMSS.ID)
112+
}
113+
s.Scope.SetProviderID(providerID)
108114
s.Scope.SetVMSSState(fetchedVMSS)
109115
}
110116
}()

‎azure/services/scalesets/scalesets_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,7 @@ func newWindowsVMSSSpec() azure.ScaleSetSpec {
948948

949949
func newDefaultExistingVMSS(vmSize string) compute.VirtualMachineScaleSet {
950950
vmss := newDefaultVMSS(vmSize)
951-
vmss.ID = to.StringPtr("vmss-id")
951+
vmss.ID = to.StringPtr("subscriptions/1234/resourceGroups/my_resource_group/providers/Microsoft.Compute/virtualMachines/my-vm")
952952
return vmss
953953
}
954954

@@ -1192,7 +1192,7 @@ func newDefaultInstances() []compute.VirtualMachineScaleSetVM {
11921192
}
11931193

11941194
func setupDefaultVMSSInProgressOperationDoneExpectations(s *mock_scalesets.MockScaleSetScopeMockRecorder, m *mock_scalesets.MockClientMockRecorder, createdVMSS compute.VirtualMachineScaleSet, instances []compute.VirtualMachineScaleSetVM) {
1195-
createdVMSS.ID = to.StringPtr("vmss-id")
1195+
createdVMSS.ID = to.StringPtr("subscriptions/1234/resourceGroups/my_resource_group/providers/Microsoft.Compute/virtualMachines/my-vm")
11961196
createdVMSS.ProvisioningState = to.StringPtr(string(infrav1.Succeeded))
11971197
setupDefaultVMSSExpectations(s)
11981198
future := &infrav1.Future{
@@ -1282,7 +1282,7 @@ func setupVMSSExpectationsWithoutVMImage(s *mock_scalesets.MockScaleSetScopeMock
12821282

12831283
func setupDefaultVMSSUpdateExpectations(s *mock_scalesets.MockScaleSetScopeMockRecorder) {
12841284
setupUpdateVMSSExpectations(s)
1285-
s.SetProviderID(azure.ProviderIDPrefix + "vmss-id")
1285+
s.SetProviderID(azure.ProviderIDPrefix + "subscriptions/1234/resourceGroups/my_resource_group/providers/Microsoft.Compute/virtualMachines/my-vm")
12861286
s.GetLongRunningOperationState(defaultVMSSName, serviceName).Return(nil)
12871287
s.MaxSurge().Return(1, nil)
12881288
s.SetVMSSState(gomock.Any())

‎azure/services/virtualmachines/virtualmachines.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"sigs.k8s.io/cluster-api-provider-azure/azure/services/async"
3232
"sigs.k8s.io/cluster-api-provider-azure/azure/services/networkinterfaces"
3333
"sigs.k8s.io/cluster-api-provider-azure/azure/services/publicips"
34+
azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure"
3435
"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
3536
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
3637
)
@@ -98,7 +99,12 @@ func (s *Service) Reconcile(ctx context.Context) error {
9899
if err != nil {
99100
return err
100101
}
101-
s.Scope.SetProviderID(azure.ProviderIDPrefix + infraVM.ID)
102+
// Transform the VM resource representation to conform to the cloud-provider-azure representation
103+
providerID, err := azureutil.ConvertResourceGroupNameToLower(azure.ProviderIDPrefix + infraVM.ID)
104+
if err != nil {
105+
return errors.Wrapf(err, "failed to parse VM ID %s", infraVM.ID)
106+
}
107+
s.Scope.SetProviderID(providerID)
102108
s.Scope.SetAnnotation("cluster-api-provider-azure", "true")
103109

104110
// Discover addresses for NICs associated with the VM

‎azure/services/virtualmachines/virtualmachines_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ var (
5353
BootstrapData: "fake data",
5454
}
5555
fakeExistingVM = compute.VirtualMachine{
56-
ID: to.StringPtr("test-vm-id"),
56+
ID: to.StringPtr("subscriptions/123/resourceGroups/my_resource_group/providers/Microsoft.Compute/virtualMachines/my-vm"),
5757
Name: to.StringPtr("test-vm-name"),
5858
VirtualMachineProperties: &compute.VirtualMachineProperties{
5959
ProvisioningState: to.StringPtr("Succeeded"),
@@ -131,7 +131,7 @@ func TestReconcileVM(t *testing.T) {
131131
r.CreateResource(gomockinternal.AContext(), &fakeVMSpec, serviceName).Return(fakeExistingVM, nil)
132132
s.UpdatePutStatus(infrav1.VMRunningCondition, serviceName, nil)
133133
s.UpdatePutStatus(infrav1.DisksReadyCondition, serviceName, nil)
134-
s.SetProviderID("azure://test-vm-id")
134+
s.SetProviderID("azure://subscriptions/123/resourceGroups/my_resource_group/providers/Microsoft.Compute/virtualMachines/my-vm")
135135
s.SetAnnotation("cluster-api-provider-azure", "true")
136136
mnic.Get(gomockinternal.AContext(), &fakeNetworkInterfaceGetterSpec).Return(fakeNetworkInterface, nil)
137137
mpip.Get(gomockinternal.AContext(), &fakePublicIPSpec).Return(fakePublicIPs, nil)
@@ -157,7 +157,7 @@ func TestReconcileVM(t *testing.T) {
157157
r.CreateResource(gomockinternal.AContext(), &fakeVMSpec, serviceName).Return(fakeExistingVM, nil)
158158
s.UpdatePutStatus(infrav1.VMRunningCondition, serviceName, nil)
159159
s.UpdatePutStatus(infrav1.DisksReadyCondition, serviceName, nil)
160-
s.SetProviderID("azure://test-vm-id")
160+
s.SetProviderID("azure://subscriptions/123/resourceGroups/my_resource_group/providers/Microsoft.Compute/virtualMachines/my-vm")
161161
s.SetAnnotation("cluster-api-provider-azure", "true")
162162
mnic.Get(gomockinternal.AContext(), &fakeNetworkInterfaceGetterSpec).Return(network.Interface{}, internalError)
163163
},
@@ -170,7 +170,7 @@ func TestReconcileVM(t *testing.T) {
170170
r.CreateResource(gomockinternal.AContext(), &fakeVMSpec, serviceName).Return(fakeExistingVM, nil)
171171
s.UpdatePutStatus(infrav1.VMRunningCondition, serviceName, nil)
172172
s.UpdatePutStatus(infrav1.DisksReadyCondition, serviceName, nil)
173-
s.SetProviderID("azure://test-vm-id")
173+
s.SetProviderID("azure://subscriptions/123/resourceGroups/my_resource_group/providers/Microsoft.Compute/virtualMachines/my-vm")
174174
s.SetAnnotation("cluster-api-provider-azure", "true")
175175
mnic.Get(gomockinternal.AContext(), &fakeNetworkInterfaceGetterSpec).Return(fakeNetworkInterface, nil)
176176
mpip.Get(gomockinternal.AContext(), &fakePublicIPSpec).Return(network.PublicIPAddress{}, internalError)

‎exp/controllers/azuremanagedmachinepool_reconciler.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package controllers
1919
import (
2020
"context"
2121
"fmt"
22-
"strings"
2322
"time"
2423

2524
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
@@ -28,6 +27,7 @@ import (
2827
"sigs.k8s.io/cluster-api-provider-azure/azure/scope"
2928
"sigs.k8s.io/cluster-api-provider-azure/azure/services/agentpools"
3029
"sigs.k8s.io/cluster-api-provider-azure/azure/services/scalesets"
30+
azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure"
3131
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
3232
)
3333

@@ -133,7 +133,12 @@ func (s *azureManagedMachinePoolService) Reconcile(ctx context.Context) error {
133133

134134
var providerIDs = make([]string, len(instances))
135135
for i := 0; i < len(instances); i++ {
136-
providerIDs[i] = strings.ToLower(azure.ProviderIDPrefix + *instances[i].ID)
136+
// Transform the VMSS instance resource representation to conform to the cloud-provider-azure representation
137+
providerID, err := azureutil.ConvertResourceGroupNameToLower(azure.ProviderIDPrefix + *instances[i].ID)
138+
if err != nil {
139+
return errors.Wrapf(err, "failed to parse instance ID %s", *instances[i].ID)
140+
}
141+
providerIDs[i] = providerID
137142
}
138143

139144
s.scope.SetAgentPoolProviderIDList(providerIDs)

‎util/azure/azure.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
Copyright 2022 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package azure
18+
19+
import (
20+
"fmt"
21+
"regexp"
22+
"strings"
23+
)
24+
25+
var azureResourceGroupNameRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/(?:.*)`)
26+
27+
// ConvertResourceGroupNameToLower converts the resource group name in the resource ID to be lowered.
28+
// Inspired by https://github.com/kubernetes-sigs/cloud-provider-azure/blob/88c9b89611e7c1fcbd39266928cce8406eb0e728/pkg/provider/azure_wrap.go#L409
29+
func ConvertResourceGroupNameToLower(resourceID string) (string, error) {
30+
matches := azureResourceGroupNameRE.FindStringSubmatch(resourceID)
31+
if len(matches) != 2 {
32+
return "", fmt.Errorf("%q isn't in Azure resource ID format %q", resourceID, azureResourceGroupNameRE.String())
33+
}
34+
35+
resourceGroup := matches[1]
36+
return strings.Replace(resourceID, resourceGroup, strings.ToLower(resourceGroup), 1), nil
37+
}

‎util/azure/azure_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
Copyright 2022 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package azure
18+
19+
import (
20+
"testing"
21+
22+
. "github.com/onsi/gomega"
23+
)
24+
25+
func TestConvertResourceGroupNameToLower(t *testing.T) {
26+
tests := []struct {
27+
desc string
28+
resourceID string
29+
expected string
30+
expectError bool
31+
}{
32+
{
33+
desc: "empty string should report error",
34+
resourceID: "",
35+
expectError: true,
36+
},
37+
{
38+
desc: "resourceID not in Azure format should report error",
39+
resourceID: "invalid-id",
40+
expectError: true,
41+
},
42+
{
43+
desc: "providerID not in Azure format should report error",
44+
resourceID: "azure://invalid-id",
45+
expectError: true,
46+
},
47+
{
48+
desc: "resource group name in VM providerID should be converted",
49+
resourceID: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachines/k8s-agent-AAAAAAAA-0",
50+
expected: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachines/k8s-agent-AAAAAAAA-0",
51+
},
52+
{
53+
desc: "resource group name in VM resourceID should be converted",
54+
resourceID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachines/k8s-agent-AAAAAAAA-0",
55+
expected: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachines/k8s-agent-AAAAAAAA-0",
56+
},
57+
{
58+
desc: "resource group name in VMSS providerID should be converted",
59+
resourceID: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156",
60+
expected: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156",
61+
},
62+
{
63+
desc: "resource group name in VMSS resourceID should be converted",
64+
resourceID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156",
65+
expected: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156",
66+
},
67+
}
68+
69+
for _, test := range tests {
70+
test := test
71+
t.Run(test.desc, func(t *testing.T) {
72+
t.Parallel()
73+
g := NewWithT(t)
74+
var actual string
75+
var err error
76+
actual, err = ConvertResourceGroupNameToLower(test.resourceID)
77+
if test.expectError {
78+
g.Expect(err).NotTo(BeNil())
79+
} else {
80+
g.Expect(err).To(BeNil())
81+
g.Expect(actual).To(Equal(test.expected))
82+
}
83+
})
84+
}
85+
}

0 commit comments

Comments
 (0)