Skip to content

Commit 1676fca

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3322 from willie-yao/fix-clusterclass-test
Fetch AzureCluster name from OwnerCluster instead of assuming ClusterName = AzureCluster.Name
2 parents 49072ad + 2e65807 commit 1676fca

File tree

5 files changed

+158
-31
lines changed

5 files changed

+158
-31
lines changed

‎api/v1beta1/azuremachine_default.go‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,29 +174,51 @@ func (s *AzureMachineSpec) SetNetworkInterfacesDefaults() {
174174
}
175175
}
176176

177-
// GetSubscriptionID returns the subscription ID for the given cluster name and namespace.
178-
func GetSubscriptionID(cli client.Client, clusterName string, namespace string, maxAttempts int) (string, error) {
177+
// GetOwnerAzureClusterNameAndNamespace returns the owner azure cluster's name and namespace for the given cluster name and namespace.
178+
func GetOwnerAzureClusterNameAndNamespace(cli client.Client, clusterName string, namespace string, maxAttempts int) (azureClusterName string, azureClusterNamespace string, err error) {
179179
ctx := context.Background()
180180

181-
ownerCluster := &AzureCluster{}
181+
ownerCluster := &clusterv1.Cluster{}
182182
key := client.ObjectKey{
183183
Namespace: namespace,
184184
Name: clusterName,
185185
}
186186

187187
for i := 1; ; i++ {
188-
err := cli.Get(ctx, key, ownerCluster)
189-
if err != nil {
188+
if err := cli.Get(ctx, key, ownerCluster); err != nil {
190189
if i > maxAttempts {
191-
return "", errors.Wrapf(err, "failed to find owner cluster for %s/%s", namespace, clusterName)
190+
return "", "", errors.Wrapf(err, "failed to find owner cluster for %s/%s", namespace, clusterName)
191+
}
192+
time.Sleep(1 * time.Second)
193+
continue
194+
}
195+
break
196+
}
197+
198+
return ownerCluster.Spec.InfrastructureRef.Name, ownerCluster.Spec.InfrastructureRef.Namespace, nil
199+
}
200+
201+
// GetSubscriptionID returns the subscription ID for the AzureCluster given the cluster name and namespace.
202+
func GetSubscriptionID(cli client.Client, ownerAzureClusterName string, ownerAzureClusterNamespace string, maxAttempts int) (string, error) {
203+
ctx := context.Background()
204+
205+
ownerAzureCluster := &AzureCluster{}
206+
key := client.ObjectKey{
207+
Namespace: ownerAzureClusterNamespace,
208+
Name: ownerAzureClusterName,
209+
}
210+
for i := 1; ; i++ {
211+
if err := cli.Get(ctx, key, ownerAzureCluster); err != nil {
212+
if i >= maxAttempts {
213+
return "", errors.Wrapf(err, "failed to find AzureCluster for owner cluster %s/%s", ownerAzureClusterNamespace, ownerAzureClusterName)
192214
}
193215
time.Sleep(1 * time.Second)
194216
continue
195217
}
196218
break
197219
}
198220

199-
return ownerCluster.Spec.SubscriptionID, nil
221+
return ownerAzureCluster.Spec.SubscriptionID, nil
200222
}
201223

202224
// SetDefaults sets to the defaults for the AzureMachineSpec.
@@ -212,7 +234,12 @@ func (m *AzureMachine) SetDefaults(client client.Client) error {
212234
errs = append(errs, errors.Errorf("failed to fetch ClusterName for AzureMachine %s/%s", m.Namespace, m.Name))
213235
}
214236

215-
subscriptionID, err := GetSubscriptionID(client, clusterName, m.Namespace, 5)
237+
ownerAzureClusterName, ownerAzureClusterNamespace, err := GetOwnerAzureClusterNameAndNamespace(client, clusterName, m.Namespace, 5)
238+
if err != nil {
239+
errs = append(errs, errors.Wrapf(err, "failed to fetch owner cluster for AzureMachine %s/%s", m.Namespace, m.Name))
240+
}
241+
242+
subscriptionID, err := GetSubscriptionID(client, ownerAzureClusterName, ownerAzureClusterNamespace, 5)
216243
if err != nil {
217244
errs = append(errs, errors.Wrapf(err, "failed to fetch subscription ID for AzureMachine %s/%s", m.Namespace, m.Name))
218245
}

‎api/v1beta1/azuremachine_default_test.go‎

Lines changed: 93 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/google/uuid"
2727
. "github.com/onsi/gomega"
2828
"github.com/pkg/errors"
29+
corev1 "k8s.io/api/core/v1"
2930
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3031
"k8s.io/utils/pointer"
3132
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -401,39 +402,101 @@ func TestAzureMachineSpec_SetNetworkInterfacesDefaults(t *testing.T) {
401402
}
402403
}
403404

405+
func TestAzureMachineSpec_GetOwnerCluster(t *testing.T) {
406+
tests := []struct {
407+
name string
408+
maxAttempts int
409+
wantedName string
410+
wantedNamespace string
411+
wantErr bool
412+
}{
413+
{
414+
name: "ownerCluster is returned",
415+
maxAttempts: 1,
416+
wantedName: "test-cluster",
417+
wantedNamespace: "default",
418+
wantErr: false,
419+
},
420+
{
421+
name: "ownerCluster is returned after 2 attempts",
422+
maxAttempts: 2,
423+
wantedName: "test-cluster",
424+
wantedNamespace: "default",
425+
wantErr: false,
426+
},
427+
{
428+
name: "ownerCluster is not returned after 5 attempts",
429+
maxAttempts: 5,
430+
wantedName: "test-cluster",
431+
wantedNamespace: "default",
432+
wantErr: true,
433+
},
434+
}
435+
436+
for _, tc := range tests {
437+
t.Run(tc.name, func(t *testing.T) {
438+
g := NewWithT(t)
439+
client := mockClient{ReturnError: tc.wantErr}
440+
name, namespace, err := GetOwnerAzureClusterNameAndNamespace(client, "test-cluster", "default", tc.maxAttempts)
441+
if tc.wantErr {
442+
g.Expect(err).To(HaveOccurred())
443+
} else {
444+
g.Expect(err).NotTo(HaveOccurred())
445+
g.Expect(name).To(Equal(tc.wantedName))
446+
g.Expect(namespace).To(Equal(tc.wantedNamespace))
447+
}
448+
})
449+
}
450+
}
451+
404452
func TestAzureMachineSpec_GetSubscriptionID(t *testing.T) {
405453
g := NewWithT(t)
406454

407455
tests := []struct {
408-
name string
409-
maxAttempts int
410-
want string
411-
wantErr bool
456+
name string
457+
maxAttempts int
458+
ownerAzureClusterName string
459+
ownerAzureClusterNamespace string
460+
want string
461+
wantErr bool
412462
}{
413463
{
414-
name: "subscription ID is returned",
415-
maxAttempts: 1,
416-
want: "test-subscription-id",
417-
wantErr: false,
464+
name: "empty owner cluster name returns error",
465+
maxAttempts: 1,
466+
ownerAzureClusterName: "",
467+
want: "test-subscription-id",
468+
wantErr: true,
418469
},
419470
{
420-
name: "subscription ID is returned after 2 attempts",
421-
maxAttempts: 2,
422-
want: "test-subscription-id",
423-
wantErr: false,
471+
name: "subscription ID is returned",
472+
maxAttempts: 1,
473+
ownerAzureClusterName: "test-cluster",
474+
ownerAzureClusterNamespace: "default",
475+
want: "test-subscription-id",
476+
wantErr: false,
424477
},
425478
{
426-
name: "subscription ID is not returned after 5 attempts",
427-
maxAttempts: 5,
428-
want: "",
429-
wantErr: true,
479+
name: "subscription ID is returned after 2 attempts",
480+
maxAttempts: 2,
481+
ownerAzureClusterName: "test-cluster",
482+
ownerAzureClusterNamespace: "default",
483+
want: "test-subscription-id",
484+
wantErr: false,
485+
},
486+
{
487+
name: "subscription ID is not returned after 5 attempts",
488+
maxAttempts: 5,
489+
ownerAzureClusterName: "test-cluster",
490+
ownerAzureClusterNamespace: "default",
491+
want: "",
492+
wantErr: true,
430493
},
431494
}
432495

433496
for _, tc := range tests {
434497
t.Run(tc.name, func(t *testing.T) {
435498
client := mockClient{ReturnError: tc.wantErr}
436-
result, err := GetSubscriptionID(client, "test-cluster", "default", tc.maxAttempts)
499+
result, err := GetSubscriptionID(client, tc.ownerAzureClusterName, tc.ownerAzureClusterNamespace, tc.maxAttempts)
437500
if tc.wantErr {
438501
g.Expect(err).To(HaveOccurred())
439502
} else {
@@ -453,9 +516,19 @@ func (m mockClient) Get(ctx context.Context, key client.ObjectKey, obj client.Ob
453516
if m.ReturnError {
454517
return errors.New("AzureCluster not found: failed to find owner cluster for test-cluster")
455518
}
456-
ac := &AzureCluster{}
457-
ac.Spec.SubscriptionID = "test-subscription-id"
458-
obj.(*AzureCluster).Spec.SubscriptionID = ac.Spec.SubscriptionID
519+
// Check if we're calling Get on an AzureCluster or a Cluster
520+
switch obj := obj.(type) {
521+
case *AzureCluster:
522+
obj.Spec.SubscriptionID = "test-subscription-id"
523+
case *clusterv1.Cluster:
524+
obj.Spec.InfrastructureRef = &corev1.ObjectReference{
525+
Kind: "AzureCluster",
526+
Name: "test-cluster",
527+
Namespace: "default",
528+
}
529+
default:
530+
return errors.New("unexpected object type")
531+
}
459532

460533
return nil
461534
}

‎api/v1beta1/azuremachine_webhook_test.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import (
2222

2323
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute"
2424
. "github.com/onsi/gomega"
25+
"github.com/pkg/errors"
26+
corev1 "k8s.io/api/core/v1"
2527
"k8s.io/apimachinery/pkg/api/resource"
2628
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2729
"k8s.io/utils/pointer"
@@ -716,7 +718,17 @@ type mockDefaultClient struct {
716718
}
717719

718720
func (m mockDefaultClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
719-
obj.(*AzureCluster).Spec.SubscriptionID = m.SubscriptionID
721+
switch obj := obj.(type) {
722+
case *AzureCluster:
723+
obj.Spec.SubscriptionID = m.SubscriptionID
724+
case *clusterv1.Cluster:
725+
obj.Spec.InfrastructureRef = &corev1.ObjectReference{
726+
Kind: "AzureCluster",
727+
Name: "test-cluster",
728+
}
729+
default:
730+
return errors.New("invalid object type")
731+
}
720732
return nil
721733
}
722734

‎exp/api/v1beta1/azuremachinepool_default.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,12 @@ func (amp *AzureMachinePool) SetDefaults(client client.Client) error {
4242
errs = append(errs, errors.Wrap(err, "failed to find parent machine pool"))
4343
}
4444

45-
subscriptionID, err := infrav1.GetSubscriptionID(client, machinePool.Spec.ClusterName, machinePool.Namespace, 5)
45+
ownerAzureClusterName, ownerAzureClusterNamespace, err := infrav1.GetOwnerAzureClusterNameAndNamespace(client, machinePool.Spec.ClusterName, machinePool.Namespace, 5)
46+
if err != nil {
47+
errs = append(errs, errors.Wrap(err, "failed to get owner cluster"))
48+
}
49+
50+
subscriptionID, err := infrav1.GetSubscriptionID(client, ownerAzureClusterName, ownerAzureClusterNamespace, 5)
4651
if err != nil {
4752
errs = append(errs, errors.Wrap(err, "failed to get subscription ID"))
4853
}

‎exp/api/v1beta1/azuremachinepool_webhook_test.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,17 @@ type mockDefaultClient struct {
264264
}
265265

266266
func (m mockDefaultClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
267-
obj.(*infrav1.AzureCluster).Spec.SubscriptionID = m.SubscriptionID
267+
switch obj := obj.(type) {
268+
case *infrav1.AzureCluster:
269+
obj.Spec.SubscriptionID = m.SubscriptionID
270+
case *clusterv1.Cluster:
271+
obj.Spec.InfrastructureRef = &corev1.ObjectReference{
272+
Kind: "AzureCluster",
273+
Name: "test-cluster",
274+
}
275+
default:
276+
return errors.New("invalid object type")
277+
}
268278
return nil
269279
}
270280

0 commit comments

Comments
 (0)