Skip to content

Commit

Permalink
Merge pull request #2543 from jackfrancis/subnet-vnet-ismanaged-ratel…
Browse files Browse the repository at this point in the history 
…imit

AKS: enable isVnetManaged, add caching
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Aug 24, 2022
2 parents 3e4628b + 0f321e4 commit c93fbfb
Show file tree
Hide file tree
Showing 5 changed files with 310 additions and 3 deletions.
19 changes: 18 additions & 1 deletion azure/scope/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type ClusterScopeParams struct {
Client client.Client
Cluster *clusterv1.Cluster
AzureCluster *infrav1.AzureCluster
Cache *ClusterCache
}

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

if params.Cache == nil {
params.Cache = &ClusterCache{}
}

helper, err := patch.NewHelper(params.AzureCluster, params.Client)
if err != nil {
return nil, errors.Errorf("failed to init patch helper: %v", err)
Expand All @@ -98,19 +103,26 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc
Cluster: params.Cluster,
AzureCluster: params.AzureCluster,
patchHelper: helper,
cache: params.Cache,
}, nil
}

// ClusterScope defines the basic context for an actuator to operate upon.
type ClusterScope struct {
Client client.Client
patchHelper *patch.Helper
cache *ClusterCache

AzureClients
Cluster *clusterv1.Cluster
AzureCluster *infrav1.AzureCluster
}

// ClusterCache stores ClusterCache data locally so we don't have to hit the API multiple times within the same reconcile loop.
type ClusterCache struct {
isVnetManaged *bool
}

// BaseURI returns the Azure ResourceManagerEndpoint.
func (s *ClusterScope) BaseURI() string {
return s.ResourceManagerEndpoint
Expand Down Expand Up @@ -524,7 +536,12 @@ func (s *ClusterScope) Vnet() *infrav1.VnetSpec {

// IsVnetManaged returns true if the vnet is managed.
func (s *ClusterScope) IsVnetManaged() bool {
return s.Vnet().ID == "" || s.Vnet().Tags.HasOwned(s.ClusterName())
if s.cache.isVnetManaged != nil {
return to.Bool(s.cache.isVnetManaged)
}
isVnetManaged := s.Vnet().ID == "" || s.Vnet().Tags.HasOwned(s.ClusterName())
s.cache.isVnetManaged = to.BoolPtr(isVnetManaged)
return isVnetManaged
}

// IsIPv6Enabled returns true if IPv6 is enabled.
Expand Down
127 changes: 127 additions & 0 deletions azure/scope/cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,7 @@ func TestRouteTableSpecs(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: nil,
},
Expand Down Expand Up @@ -828,6 +829,7 @@ func TestRouteTableSpecs(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: []azure.ResourceSpecGetter{
&routetables.RouteTableSpec{
Expand Down Expand Up @@ -875,6 +877,7 @@ func TestNatGatewaySpecs(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: nil,
},
Expand Down Expand Up @@ -922,6 +925,7 @@ func TestNatGatewaySpecs(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: []azure.ResourceSpecGetter{
&natgateways.NatGatewaySpec{
Expand Down Expand Up @@ -999,6 +1003,7 @@ func TestNatGatewaySpecs(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: []azure.ResourceSpecGetter{
&natgateways.NatGatewaySpec{
Expand Down Expand Up @@ -1075,6 +1080,7 @@ func TestNatGatewaySpecs(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: []azure.ResourceSpecGetter{
&natgateways.NatGatewaySpec{
Expand Down Expand Up @@ -1154,6 +1160,7 @@ func TestNSGSpecs(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: []azure.ResourceSpecGetter{
&securitygroups.NSGSpec{
Expand Down Expand Up @@ -1199,6 +1206,7 @@ func TestSubnetSpecs(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: []azure.ResourceSpecGetter{},
},
Expand Down Expand Up @@ -1260,6 +1268,7 @@ func TestSubnetSpecs(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: []azure.ResourceSpecGetter{
&subnets.SubnetSpec{
Expand Down Expand Up @@ -1362,6 +1371,7 @@ func TestSubnetSpecs(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: []azure.ResourceSpecGetter{
&subnets.SubnetSpec{
Expand Down Expand Up @@ -1404,6 +1414,122 @@ func TestSubnetSpecs(t *testing.T) {
}
}

func TestIsVnetManaged(t *testing.T) {
tests := []struct {
name string
clusterScope ClusterScope
want bool
}{
{
name: "VNET ID is empty",
clusterScope: ClusterScope{
Cluster: &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "my-cluster",
},
},
AzureCluster: &infrav1.AzureCluster{
Spec: infrav1.AzureClusterSpec{
NetworkSpec: infrav1.NetworkSpec{
Vnet: infrav1.VnetSpec{
ID: "",
},
},
},
},
cache: &ClusterCache{},
},
want: true,
},
{
name: "Wrong tags",
clusterScope: ClusterScope{
Cluster: &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "my-cluster",
},
},
AzureCluster: &infrav1.AzureCluster{
Spec: infrav1.AzureClusterSpec{
NetworkSpec: infrav1.NetworkSpec{
Vnet: infrav1.VnetSpec{
ID: "my-id",
VnetClassSpec: infrav1.VnetClassSpec{Tags: map[string]string{
"key": "value",
}},
},
},
},
},
cache: &ClusterCache{},
},
want: false,
},
{
name: "Has owning tags",
clusterScope: ClusterScope{
Cluster: &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "my-cluster",
},
},
AzureCluster: &infrav1.AzureCluster{
Spec: infrav1.AzureClusterSpec{
NetworkSpec: infrav1.NetworkSpec{
Vnet: infrav1.VnetSpec{
ID: "my-id",
VnetClassSpec: infrav1.VnetClassSpec{Tags: map[string]string{
"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": "owned",
}},
},
},
},
},
cache: &ClusterCache{},
},
want: true,
},
{
name: "Has cached value of false",
clusterScope: ClusterScope{
AzureCluster: &infrav1.AzureCluster{
Spec: infrav1.AzureClusterSpec{},
},
cache: &ClusterCache{
isVnetManaged: to.BoolPtr(false),
},
},
want: false,
},
{
name: "Has cached value of true",
clusterScope: ClusterScope{
AzureCluster: &infrav1.AzureCluster{
Spec: infrav1.AzureClusterSpec{},
},
cache: &ClusterCache{
isVnetManaged: to.BoolPtr(true),
},
},
want: true,
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got := tt.clusterScope.IsVnetManaged()
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("IsVnetManaged() = \n%t, want \n%t", got, tt.want)
}
if to.Bool(tt.clusterScope.cache.isVnetManaged) != got {
t.Errorf("IsVnetManaged() = \n%t, cache = \n%t", got, to.Bool(tt.clusterScope.cache.isVnetManaged))
}
})
}
}

func TestAzureBastionSpec(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -1510,6 +1636,7 @@ func TestAzureBastionSpec(t *testing.T) {
},
},
},
cache: &ClusterCache{},
},
want: &bastionhosts.AzureBastionSpec{
Name: "fake-azure-bastion-1",
Expand Down
28 changes: 27 additions & 1 deletion azure/scope/managedcontrolplane.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strings"

"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/to"
"github.com/pkg/errors"
"golang.org/x/mod/semver"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -51,6 +52,7 @@ type ManagedControlPlaneScopeParams struct {
Cluster *clusterv1.Cluster
ControlPlane *infrav1exp.AzureManagedControlPlane
ManagedMachinePools []ManagedMachinePool
Cache *ManagedControlPlaneCache
}

// NewManagedControlPlaneScope creates a new Scope from the supplied parameters.
Expand Down Expand Up @@ -82,6 +84,10 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
}
}

if params.Cache == nil {
params.Cache = &ManagedControlPlaneCache{}
}

helper, err := patch.NewHelper(params.ControlPlane, params.Client)
if err != nil {
return nil, errors.Wrap(err, "failed to init patch helper")
Expand All @@ -94,6 +100,7 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
ControlPlane: params.ControlPlane,
ManagedMachinePools: params.ManagedMachinePools,
patchHelper: helper,
cache: params.Cache,
}, nil
}

Expand All @@ -102,13 +109,19 @@ type ManagedControlPlaneScope struct {
Client client.Client
patchHelper *patch.Helper
kubeConfigData []byte
cache *ManagedControlPlaneCache

AzureClients
Cluster *clusterv1.Cluster
ControlPlane *infrav1exp.AzureManagedControlPlane
ManagedMachinePools []ManagedMachinePool
}

// ManagedControlPlaneCache stores ManagedControlPlane data locally so we don't have to hit the API multiple times within the same reconcile loop.
type ManagedControlPlaneCache struct {
isVnetManaged *bool
}

// ResourceGroup returns the managed control plane's resource group.
func (s *ManagedControlPlaneScope) ResourceGroup() string {
if s.ControlPlane == nil {
Expand Down Expand Up @@ -328,7 +341,20 @@ func (s *ManagedControlPlaneScope) IsIPv6Enabled() bool {

// IsVnetManaged returns true if the vnet is managed.
func (s *ManagedControlPlaneScope) IsVnetManaged() bool {
return true
if s.cache.isVnetManaged != nil {
return to.Bool(s.cache.isVnetManaged)
}
// TODO refactor `IsVnetManaged` so that it is able to use an upstream context
// see https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/2581
ctx := context.Background()
ctx, log, done := tele.StartSpanWithLogger(ctx, "scope.ManagedControlPlaneScope.IsVnetManaged")
defer done()
isManaged, err := virtualnetworks.New(s).IsManaged(ctx)
if err != nil {
log.Error(err, "Unable to determine if ManagedControlPlaneScope VNET is managed by capz", "AzureManagedCluster", s.ClusterName())
}
s.cache.isVnetManaged = to.BoolPtr(isManaged)
return isManaged
}

// APIServerLBName returns the API Server LB spec.
Expand Down
Loading

0 comments on commit c93fbfb

Please sign in to comment.