Skip to content

Commit 2b91b72

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#5012 from hrbasic/issue_4930
Update managed vnet check logic
2 parents cc3e8e9 + af19a3f commit 2b91b72

File tree

2 files changed

+152
-29
lines changed

2 files changed

+152
-29
lines changed

‎azure/scope/cluster.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -582,9 +582,21 @@ func (s *ClusterScope) IsVnetManaged() bool {
582582
if s.cache.isVnetManaged != nil {
583583
return ptr.Deref(s.cache.isVnetManaged, false)
584584
}
585-
isVnetManaged := s.Vnet().ID == "" || s.Vnet().Tags.HasOwned(s.ClusterName())
586-
s.cache.isVnetManaged = ptr.To(isVnetManaged)
587-
return isVnetManaged
585+
ctx := context.Background()
586+
ctx, log, done := tele.StartSpanWithLogger(ctx, "scope.ClusterScope.IsVnetManaged")
587+
defer done()
588+
589+
vnet := s.VNetSpec().ResourceRef()
590+
vnet.SetNamespace(s.ASOOwner().GetNamespace())
591+
err := s.Client.Get(ctx, client.ObjectKeyFromObject(vnet), vnet)
592+
if err != nil {
593+
log.Error(err, "Unable to determine if ClusterScope VNET is managed by capz, assuming unmanaged", "AzureCluster", s.ClusterName())
594+
return false
595+
}
596+
597+
isManaged := infrav1.Tags(vnet.Status.Tags).HasOwned(s.ClusterName())
598+
s.cache.isVnetManaged = ptr.To(isManaged)
599+
return isManaged
588600
}
589601

590602
// IsIPv6Enabled returns true if IPv6 is enabled.

‎azure/scope/cluster_test.go

Lines changed: 137 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -930,9 +930,15 @@ func TestRouteTableSpecs(t *testing.T) {
930930
}
931931

932932
func TestNatGatewaySpecs(t *testing.T) {
933+
scheme := runtime.NewScheme()
934+
_ = asonetworkv1api20201101.AddToScheme(scheme)
935+
_ = corev1.AddToScheme(scheme)
936+
_ = infrav1.AddToScheme(scheme)
937+
933938
tests := []struct {
934939
name string
935940
clusterScope ClusterScope
941+
vnet asonetworkv1api20201101.VirtualNetwork
936942
want []azure.ASOResourceSpecGetter[*asonetworkv1api20220701.NatGateway]
937943
}{
938944
{
@@ -993,11 +999,24 @@ func TestNatGatewaySpecs(t *testing.T) {
993999
},
9941000
},
9951001
},
1002+
Vnet: infrav1.VnetSpec{
1003+
Name: "fake-vnet-1",
1004+
},
9961005
},
9971006
},
9981007
},
9991008
cache: &ClusterCache{},
10001009
},
1010+
vnet: asonetworkv1api20201101.VirtualNetwork{
1011+
ObjectMeta: metav1.ObjectMeta{
1012+
Name: "fake-vnet-1",
1013+
},
1014+
Status: asonetworkv1api20201101.VirtualNetwork_STATUS{
1015+
Tags: map[string]string{
1016+
"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": "owned",
1017+
},
1018+
},
1019+
},
10011020
want: []azure.ASOResourceSpecGetter[*asonetworkv1api20220701.NatGateway]{
10021021
&natgateways.NatGatewaySpec{
10031022
Name: "fake-nat-gateway-1",
@@ -1075,11 +1094,24 @@ func TestNatGatewaySpecs(t *testing.T) {
10751094
},
10761095
},
10771096
},
1097+
Vnet: infrav1.VnetSpec{
1098+
Name: "fake-vnet-1",
1099+
},
10781100
},
10791101
},
10801102
},
10811103
cache: &ClusterCache{},
10821104
},
1105+
vnet: asonetworkv1api20201101.VirtualNetwork{
1106+
ObjectMeta: metav1.ObjectMeta{
1107+
Name: "fake-vnet-1",
1108+
},
1109+
Status: asonetworkv1api20201101.VirtualNetwork_STATUS{
1110+
Tags: map[string]string{
1111+
"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": "owned",
1112+
},
1113+
},
1114+
},
10831115
want: []azure.ASOResourceSpecGetter[*asonetworkv1api20220701.NatGateway]{
10841116
&natgateways.NatGatewaySpec{
10851117
Name: "fake-nat-gateway-1",
@@ -1156,11 +1188,24 @@ func TestNatGatewaySpecs(t *testing.T) {
11561188
},
11571189
},
11581190
},
1191+
Vnet: infrav1.VnetSpec{
1192+
Name: "fake-vnet-1",
1193+
},
11591194
},
11601195
},
11611196
},
11621197
cache: &ClusterCache{},
11631198
},
1199+
vnet: asonetworkv1api20201101.VirtualNetwork{
1200+
ObjectMeta: metav1.ObjectMeta{
1201+
Name: "fake-vnet-1",
1202+
},
1203+
Status: asonetworkv1api20201101.VirtualNetwork_STATUS{
1204+
Tags: map[string]string{
1205+
"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": "owned",
1206+
},
1207+
},
1208+
},
11641209
want: []azure.ASOResourceSpecGetter[*asonetworkv1api20220701.NatGateway]{
11651210
&natgateways.NatGatewaySpec{
11661211
Name: "fake-nat-gateway-1",
@@ -1182,6 +1227,23 @@ func TestNatGatewaySpecs(t *testing.T) {
11821227
tt := tt
11831228
t.Run(tt.name, func(t *testing.T) {
11841229
t.Parallel()
1230+
fakeIdentity := &infrav1.AzureClusterIdentity{
1231+
ObjectMeta: metav1.ObjectMeta{
1232+
Name: "fake-identity",
1233+
Namespace: "default",
1234+
},
1235+
Spec: infrav1.AzureClusterIdentitySpec{
1236+
Type: infrav1.ServicePrincipal,
1237+
ClientID: fakeClientID,
1238+
TenantID: fakeTenantID,
1239+
},
1240+
}
1241+
fakeSecret := &corev1.Secret{Data: map[string][]byte{"clientSecret": []byte("fooSecret")}}
1242+
1243+
initObjects := []runtime.Object{&tt.vnet, fakeIdentity, fakeSecret}
1244+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(initObjects...).Build()
1245+
tt.clusterScope.Client = fakeClient
1246+
11851247
if got := tt.clusterScope.NatGatewaySpecs(); !reflect.DeepEqual(got, tt.want) {
11861248
t.Errorf("NatGatewaySpecs() = %s, want %s", specArrayToString(got), specArrayToString(tt.want))
11871249
}
@@ -1349,9 +1411,15 @@ func TestNSGSpecs(t *testing.T) {
13491411
}
13501412

13511413
func TestSubnetSpecs(t *testing.T) {
1414+
scheme := runtime.NewScheme()
1415+
_ = asonetworkv1api20201101.AddToScheme(scheme)
1416+
_ = corev1.AddToScheme(scheme)
1417+
_ = infrav1.AddToScheme(scheme)
1418+
13521419
tests := []struct {
13531420
name string
13541421
clusterScope ClusterScope
1422+
vnet asonetworkv1api20201101.VirtualNetwork
13551423
want []azure.ASOResourceSpecGetter[*asonetworkv1api20201101.VirtualNetworksSubnet]
13561424
}{
13571425
{
@@ -1431,6 +1499,11 @@ func TestSubnetSpecs(t *testing.T) {
14311499
},
14321500
cache: &ClusterCache{},
14331501
},
1502+
vnet: asonetworkv1api20201101.VirtualNetwork{
1503+
ObjectMeta: metav1.ObjectMeta{
1504+
Name: "fake-vnet-1",
1505+
},
1506+
},
14341507
want: []azure.ASOResourceSpecGetter[*asonetworkv1api20201101.VirtualNetworksSubnet]{
14351508
&subnets.SubnetSpec{
14361509
Name: "fake-subnet-1",
@@ -1536,6 +1609,11 @@ func TestSubnetSpecs(t *testing.T) {
15361609
},
15371610
cache: &ClusterCache{},
15381611
},
1612+
vnet: asonetworkv1api20201101.VirtualNetwork{
1613+
ObjectMeta: metav1.ObjectMeta{
1614+
Name: "fake-vnet-1",
1615+
},
1616+
},
15391617
want: []azure.ASOResourceSpecGetter[*asonetworkv1api20201101.VirtualNetworksSubnet]{
15401618
&subnets.SubnetSpec{
15411619
Name: "fake-subnet-1",
@@ -1568,6 +1646,23 @@ func TestSubnetSpecs(t *testing.T) {
15681646
tt := tt
15691647
t.Run(tt.name, func(t *testing.T) {
15701648
t.Parallel()
1649+
fakeIdentity := &infrav1.AzureClusterIdentity{
1650+
ObjectMeta: metav1.ObjectMeta{
1651+
Name: "fake-identity",
1652+
Namespace: "default",
1653+
},
1654+
Spec: infrav1.AzureClusterIdentitySpec{
1655+
Type: infrav1.ServicePrincipal,
1656+
ClientID: fakeClientID,
1657+
TenantID: fakeTenantID,
1658+
},
1659+
}
1660+
fakeSecret := &corev1.Secret{Data: map[string][]byte{"clientSecret": []byte("fooSecret")}}
1661+
1662+
initObjects := []runtime.Object{&tt.vnet, fakeIdentity, fakeSecret}
1663+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(initObjects...).Build()
1664+
tt.clusterScope.Client = fakeClient
1665+
15711666
if got := tt.clusterScope.SubnetSpecs(); !reflect.DeepEqual(got, tt.want) {
15721667
t.Errorf("SubnetSpecs() = \n%s, want \n%s", specArrayToString(got), specArrayToString(tt.want))
15731668
}
@@ -1576,13 +1671,19 @@ func TestSubnetSpecs(t *testing.T) {
15761671
}
15771672

15781673
func TestIsVnetManaged(t *testing.T) {
1674+
scheme := runtime.NewScheme()
1675+
_ = asonetworkv1api20201101.AddToScheme(scheme)
1676+
_ = corev1.AddToScheme(scheme)
1677+
_ = infrav1.AddToScheme(scheme)
1678+
15791679
tests := []struct {
15801680
name string
15811681
clusterScope ClusterScope
1682+
vnet asonetworkv1api20201101.VirtualNetwork
15821683
want bool
15831684
}{
15841685
{
1585-
name: "VNET ID is empty",
1686+
name: "Wrong tags",
15861687
clusterScope: ClusterScope{
15871688
Cluster: &clusterv1.Cluster{
15881689
ObjectMeta: metav1.ObjectMeta{
@@ -1593,36 +1694,22 @@ func TestIsVnetManaged(t *testing.T) {
15931694
Spec: infrav1.AzureClusterSpec{
15941695
NetworkSpec: infrav1.NetworkSpec{
15951696
Vnet: infrav1.VnetSpec{
1596-
ID: "",
1697+
Name: "fake-vnet-1",
15971698
},
15981699
},
15991700
},
16001701
},
16011702
cache: &ClusterCache{},
16021703
},
1603-
want: true,
1604-
},
1605-
{
1606-
name: "Wrong tags",
1607-
clusterScope: ClusterScope{
1608-
Cluster: &clusterv1.Cluster{
1609-
ObjectMeta: metav1.ObjectMeta{
1610-
Name: "my-cluster",
1611-
},
1704+
vnet: asonetworkv1api20201101.VirtualNetwork{
1705+
ObjectMeta: metav1.ObjectMeta{
1706+
Name: "fake-vnet-1",
16121707
},
1613-
AzureCluster: &infrav1.AzureCluster{
1614-
Spec: infrav1.AzureClusterSpec{
1615-
NetworkSpec: infrav1.NetworkSpec{
1616-
Vnet: infrav1.VnetSpec{
1617-
ID: "my-id",
1618-
VnetClassSpec: infrav1.VnetClassSpec{Tags: map[string]string{
1619-
"key": "value",
1620-
}},
1621-
},
1622-
},
1708+
Status: asonetworkv1api20201101.VirtualNetwork_STATUS{
1709+
Tags: map[string]string{
1710+
"key": "value",
16231711
},
16241712
},
1625-
cache: &ClusterCache{},
16261713
},
16271714
want: false,
16281715
},
@@ -1638,16 +1725,23 @@ func TestIsVnetManaged(t *testing.T) {
16381725
Spec: infrav1.AzureClusterSpec{
16391726
NetworkSpec: infrav1.NetworkSpec{
16401727
Vnet: infrav1.VnetSpec{
1641-
ID: "my-id",
1642-
VnetClassSpec: infrav1.VnetClassSpec{Tags: map[string]string{
1643-
"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": "owned",
1644-
}},
1728+
Name: "fake-vnet-1",
16451729
},
16461730
},
16471731
},
16481732
},
16491733
cache: &ClusterCache{},
16501734
},
1735+
vnet: asonetworkv1api20201101.VirtualNetwork{
1736+
ObjectMeta: metav1.ObjectMeta{
1737+
Name: "fake-vnet-1",
1738+
},
1739+
Status: asonetworkv1api20201101.VirtualNetwork_STATUS{
1740+
Tags: map[string]string{
1741+
"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": "owned",
1742+
},
1743+
},
1744+
},
16511745
want: true,
16521746
},
16531747
{
@@ -1680,6 +1774,23 @@ func TestIsVnetManaged(t *testing.T) {
16801774
tt := tt
16811775
t.Run(tt.name, func(t *testing.T) {
16821776
t.Parallel()
1777+
fakeIdentity := &infrav1.AzureClusterIdentity{
1778+
ObjectMeta: metav1.ObjectMeta{
1779+
Name: "fake-identity",
1780+
Namespace: "default",
1781+
},
1782+
Spec: infrav1.AzureClusterIdentitySpec{
1783+
Type: infrav1.ServicePrincipal,
1784+
ClientID: fakeClientID,
1785+
TenantID: fakeTenantID,
1786+
},
1787+
}
1788+
fakeSecret := &corev1.Secret{Data: map[string][]byte{"clientSecret": []byte("fooSecret")}}
1789+
1790+
initObjects := []runtime.Object{&tt.vnet, fakeIdentity, fakeSecret}
1791+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(initObjects...).Build()
1792+
tt.clusterScope.Client = fakeClient
1793+
16831794
got := tt.clusterScope.IsVnetManaged()
16841795
if !reflect.DeepEqual(got, tt.want) {
16851796
t.Errorf("IsVnetManaged() = \n%t, want \n%t", got, tt.want)

0 commit comments

Comments
 (0)