Skip to content

Commit 806fe3a

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2600 from k8s-infra-cherrypick-robot/cherry-pick-2543-to-release-1.4
[release-1.4] AKS: enable isVnetManaged, add caching
2 parents 057332f + 9a95035 commit 806fe3a

File tree

5 files changed

+310
-3
lines changed

5 files changed

+310
-3
lines changed

‎azure/scope/cluster.go‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ type ClusterScopeParams struct {
5656
Client client.Client
5757
Cluster *clusterv1.Cluster
5858
AzureCluster *infrav1.AzureCluster
59+
Cache *ClusterCache
5960
}
6061

6162
// NewClusterScope creates a new Scope from the supplied parameters.
@@ -87,6 +88,10 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc
8788
}
8889
}
8990

91+
if params.Cache == nil {
92+
params.Cache = &ClusterCache{}
93+
}
94+
9095
helper, err := patch.NewHelper(params.AzureCluster, params.Client)
9196
if err != nil {
9297
return nil, errors.Errorf("failed to init patch helper: %v", err)
@@ -98,19 +103,26 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc
98103
Cluster: params.Cluster,
99104
AzureCluster: params.AzureCluster,
100105
patchHelper: helper,
106+
cache: params.Cache,
101107
}, nil
102108
}
103109

104110
// ClusterScope defines the basic context for an actuator to operate upon.
105111
type ClusterScope struct {
106112
Client client.Client
107113
patchHelper *patch.Helper
114+
cache *ClusterCache
108115

109116
AzureClients
110117
Cluster *clusterv1.Cluster
111118
AzureCluster *infrav1.AzureCluster
112119
}
113120

121+
// ClusterCache stores ClusterCache data locally so we don't have to hit the API multiple times within the same reconcile loop.
122+
type ClusterCache struct {
123+
isVnetManaged *bool
124+
}
125+
114126
// BaseURI returns the Azure ResourceManagerEndpoint.
115127
func (s *ClusterScope) BaseURI() string {
116128
return s.ResourceManagerEndpoint
@@ -524,7 +536,12 @@ func (s *ClusterScope) Vnet() *infrav1.VnetSpec {
524536

525537
// IsVnetManaged returns true if the vnet is managed.
526538
func (s *ClusterScope) IsVnetManaged() bool {
527-
return s.Vnet().ID == "" || s.Vnet().Tags.HasOwned(s.ClusterName())
539+
if s.cache.isVnetManaged != nil {
540+
return to.Bool(s.cache.isVnetManaged)
541+
}
542+
isVnetManaged := s.Vnet().ID == "" || s.Vnet().Tags.HasOwned(s.ClusterName())
543+
s.cache.isVnetManaged = to.BoolPtr(isVnetManaged)
544+
return isVnetManaged
528545
}
529546

530547
// IsIPv6Enabled returns true if IPv6 is enabled.

‎azure/scope/cluster_test.go‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,7 @@ func TestRouteTableSpecs(t *testing.T) {
796796
},
797797
},
798798
},
799+
cache: &ClusterCache{},
799800
},
800801
want: nil,
801802
},
@@ -831,6 +832,7 @@ func TestRouteTableSpecs(t *testing.T) {
831832
},
832833
},
833834
},
835+
cache: &ClusterCache{},
834836
},
835837
want: []azure.ResourceSpecGetter{
836838
&routetables.RouteTableSpec{
@@ -878,6 +880,7 @@ func TestNatGatewaySpecs(t *testing.T) {
878880
},
879881
},
880882
},
883+
cache: &ClusterCache{},
881884
},
882885
want: nil,
883886
},
@@ -925,6 +928,7 @@ func TestNatGatewaySpecs(t *testing.T) {
925928
},
926929
},
927930
},
931+
cache: &ClusterCache{},
928932
},
929933
want: []azure.ResourceSpecGetter{
930934
&natgateways.NatGatewaySpec{
@@ -1002,6 +1006,7 @@ func TestNatGatewaySpecs(t *testing.T) {
10021006
},
10031007
},
10041008
},
1009+
cache: &ClusterCache{},
10051010
},
10061011
want: []azure.ResourceSpecGetter{
10071012
&natgateways.NatGatewaySpec{
@@ -1078,6 +1083,7 @@ func TestNatGatewaySpecs(t *testing.T) {
10781083
},
10791084
},
10801085
},
1086+
cache: &ClusterCache{},
10811087
},
10821088
want: []azure.ResourceSpecGetter{
10831089
&natgateways.NatGatewaySpec{
@@ -1157,6 +1163,7 @@ func TestNSGSpecs(t *testing.T) {
11571163
},
11581164
},
11591165
},
1166+
cache: &ClusterCache{},
11601167
},
11611168
want: []azure.ResourceSpecGetter{
11621169
&securitygroups.NSGSpec{
@@ -1202,6 +1209,7 @@ func TestSubnetSpecs(t *testing.T) {
12021209
},
12031210
},
12041211
},
1212+
cache: &ClusterCache{},
12051213
},
12061214
want: []azure.ResourceSpecGetter{},
12071215
},
@@ -1263,6 +1271,7 @@ func TestSubnetSpecs(t *testing.T) {
12631271
},
12641272
},
12651273
},
1274+
cache: &ClusterCache{},
12661275
},
12671276
want: []azure.ResourceSpecGetter{
12681277
&subnets.SubnetSpec{
@@ -1365,6 +1374,7 @@ func TestSubnetSpecs(t *testing.T) {
13651374
},
13661375
},
13671376
},
1377+
cache: &ClusterCache{},
13681378
},
13691379
want: []azure.ResourceSpecGetter{
13701380
&subnets.SubnetSpec{
@@ -1407,6 +1417,122 @@ func TestSubnetSpecs(t *testing.T) {
14071417
}
14081418
}
14091419

1420+
func TestIsVnetManaged(t *testing.T) {
1421+
tests := []struct {
1422+
name string
1423+
clusterScope ClusterScope
1424+
want bool
1425+
}{
1426+
{
1427+
name: "VNET ID is empty",
1428+
clusterScope: ClusterScope{
1429+
Cluster: &clusterv1.Cluster{
1430+
ObjectMeta: metav1.ObjectMeta{
1431+
Name: "my-cluster",
1432+
},
1433+
},
1434+
AzureCluster: &infrav1.AzureCluster{
1435+
Spec: infrav1.AzureClusterSpec{
1436+
NetworkSpec: infrav1.NetworkSpec{
1437+
Vnet: infrav1.VnetSpec{
1438+
ID: "",
1439+
},
1440+
},
1441+
},
1442+
},
1443+
cache: &ClusterCache{},
1444+
},
1445+
want: true,
1446+
},
1447+
{
1448+
name: "Wrong tags",
1449+
clusterScope: ClusterScope{
1450+
Cluster: &clusterv1.Cluster{
1451+
ObjectMeta: metav1.ObjectMeta{
1452+
Name: "my-cluster",
1453+
},
1454+
},
1455+
AzureCluster: &infrav1.AzureCluster{
1456+
Spec: infrav1.AzureClusterSpec{
1457+
NetworkSpec: infrav1.NetworkSpec{
1458+
Vnet: infrav1.VnetSpec{
1459+
ID: "my-id",
1460+
VnetClassSpec: infrav1.VnetClassSpec{Tags: map[string]string{
1461+
"key": "value",
1462+
}},
1463+
},
1464+
},
1465+
},
1466+
},
1467+
cache: &ClusterCache{},
1468+
},
1469+
want: false,
1470+
},
1471+
{
1472+
name: "Has owning tags",
1473+
clusterScope: ClusterScope{
1474+
Cluster: &clusterv1.Cluster{
1475+
ObjectMeta: metav1.ObjectMeta{
1476+
Name: "my-cluster",
1477+
},
1478+
},
1479+
AzureCluster: &infrav1.AzureCluster{
1480+
Spec: infrav1.AzureClusterSpec{
1481+
NetworkSpec: infrav1.NetworkSpec{
1482+
Vnet: infrav1.VnetSpec{
1483+
ID: "my-id",
1484+
VnetClassSpec: infrav1.VnetClassSpec{Tags: map[string]string{
1485+
"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": "owned",
1486+
}},
1487+
},
1488+
},
1489+
},
1490+
},
1491+
cache: &ClusterCache{},
1492+
},
1493+
want: true,
1494+
},
1495+
{
1496+
name: "Has cached value of false",
1497+
clusterScope: ClusterScope{
1498+
AzureCluster: &infrav1.AzureCluster{
1499+
Spec: infrav1.AzureClusterSpec{},
1500+
},
1501+
cache: &ClusterCache{
1502+
isVnetManaged: to.BoolPtr(false),
1503+
},
1504+
},
1505+
want: false,
1506+
},
1507+
{
1508+
name: "Has cached value of true",
1509+
clusterScope: ClusterScope{
1510+
AzureCluster: &infrav1.AzureCluster{
1511+
Spec: infrav1.AzureClusterSpec{},
1512+
},
1513+
cache: &ClusterCache{
1514+
isVnetManaged: to.BoolPtr(true),
1515+
},
1516+
},
1517+
want: true,
1518+
},
1519+
}
1520+
1521+
for _, tt := range tests {
1522+
tt := tt
1523+
t.Run(tt.name, func(t *testing.T) {
1524+
t.Parallel()
1525+
got := tt.clusterScope.IsVnetManaged()
1526+
if !reflect.DeepEqual(got, tt.want) {
1527+
t.Errorf("IsVnetManaged() = \n%t, want \n%t", got, tt.want)
1528+
}
1529+
if to.Bool(tt.clusterScope.cache.isVnetManaged) != got {
1530+
t.Errorf("IsVnetManaged() = \n%t, cache = \n%t", got, to.Bool(tt.clusterScope.cache.isVnetManaged))
1531+
}
1532+
})
1533+
}
1534+
}
1535+
14101536
func TestAzureBastionSpec(t *testing.T) {
14111537
tests := []struct {
14121538
name string
@@ -1513,6 +1639,7 @@ func TestAzureBastionSpec(t *testing.T) {
15131639
},
15141640
},
15151641
},
1642+
cache: &ClusterCache{},
15161643
},
15171644
want: &bastionhosts.AzureBastionSpec{
15181645
Name: "fake-azure-bastion-1",

‎azure/scope/managedcontrolplane.go‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"strings"
2323

2424
"github.com/Azure/go-autorest/autorest"
25+
"github.com/Azure/go-autorest/autorest/to"
2526
"github.com/pkg/errors"
2627
"golang.org/x/mod/semver"
2728
corev1 "k8s.io/api/core/v1"
@@ -51,6 +52,7 @@ type ManagedControlPlaneScopeParams struct {
5152
Cluster *clusterv1.Cluster
5253
ControlPlane *infrav1exp.AzureManagedControlPlane
5354
ManagedMachinePools []ManagedMachinePool
55+
Cache *ManagedControlPlaneCache
5456
}
5557

5658
// NewManagedControlPlaneScope creates a new Scope from the supplied parameters.
@@ -82,6 +84,10 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
8284
}
8385
}
8486

87+
if params.Cache == nil {
88+
params.Cache = &ManagedControlPlaneCache{}
89+
}
90+
8591
helper, err := patch.NewHelper(params.ControlPlane, params.Client)
8692
if err != nil {
8793
return nil, errors.Wrap(err, "failed to init patch helper")
@@ -94,6 +100,7 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
94100
ControlPlane: params.ControlPlane,
95101
ManagedMachinePools: params.ManagedMachinePools,
96102
patchHelper: helper,
103+
cache: params.Cache,
97104
}, nil
98105
}
99106

@@ -102,13 +109,19 @@ type ManagedControlPlaneScope struct {
102109
Client client.Client
103110
patchHelper *patch.Helper
104111
kubeConfigData []byte
112+
cache *ManagedControlPlaneCache
105113

106114
AzureClients
107115
Cluster *clusterv1.Cluster
108116
ControlPlane *infrav1exp.AzureManagedControlPlane
109117
ManagedMachinePools []ManagedMachinePool
110118
}
111119

120+
// ManagedControlPlaneCache stores ManagedControlPlane data locally so we don't have to hit the API multiple times within the same reconcile loop.
121+
type ManagedControlPlaneCache struct {
122+
isVnetManaged *bool
123+
}
124+
112125
// ResourceGroup returns the managed control plane's resource group.
113126
func (s *ManagedControlPlaneScope) ResourceGroup() string {
114127
if s.ControlPlane == nil {
@@ -328,7 +341,20 @@ func (s *ManagedControlPlaneScope) IsIPv6Enabled() bool {
328341

329342
// IsVnetManaged returns true if the vnet is managed.
330343
func (s *ManagedControlPlaneScope) IsVnetManaged() bool {
331-
return true
344+
if s.cache.isVnetManaged != nil {
345+
return to.Bool(s.cache.isVnetManaged)
346+
}
347+
// TODO refactor `IsVnetManaged` so that it is able to use an upstream context
348+
// see https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/2581
349+
ctx := context.Background()
350+
ctx, log, done := tele.StartSpanWithLogger(ctx, "scope.ManagedControlPlaneScope.IsVnetManaged")
351+
defer done()
352+
isManaged, err := virtualnetworks.New(s).IsManaged(ctx)
353+
if err != nil {
354+
log.Error(err, "Unable to determine if ManagedControlPlaneScope VNET is managed by capz", "AzureManagedCluster", s.ClusterName())
355+
}
356+
s.cache.isVnetManaged = to.BoolPtr(isManaged)
357+
return isManaged
332358
}
333359

334360
// APIServerLBName returns the API Server LB spec.

0 commit comments

Comments
 (0)