Skip to content

Commit

Permalink
modify vnet cache
Browse files Browse the repository at this point in the history
  • Loading branch information
Cecile Robert-Michon committed Aug 31, 2021
1 parent c8ebe8d commit 590e79d
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 33 deletions.
2 changes: 1 addition & 1 deletion azure/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type Authorizer interface {
// NetworkDescriber is an interface which can get common Azure Cluster Networking information.
type NetworkDescriber interface {
Vnet() *infrav1.VnetSpec
IsVnetManaged(context.Context) (bool, error)
IsVnetManaged() (bool, error)
ControlPlaneSubnet() infrav1.SubnetSpec
Subnets() infrav1.Subnets
Subnet(string) infrav1.SubnetSpec
Expand Down
26 changes: 13 additions & 13 deletions azure/scope/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc
Cluster: params.Cluster,
AzureCluster: params.AzureCluster,
patchHelper: helper,
cache: &ClusterCache{},
cache: &clusterCache{},
}, nil
}

Expand All @@ -106,11 +106,11 @@ type ClusterScope struct {
AzureClients
Cluster *clusterv1.Cluster
AzureCluster *infrav1.AzureCluster
cache *ClusterCache
cache *clusterCache
}

// ClusterCache stores common cluster information so we don't have to hit the API multiple times within the same reconcile loop.
type ClusterCache struct {
// clusterCache stores common cluster information so we don't have to hit the API multiple times within the same reconcile loop.
type clusterCache struct {
IsVnetManaged *bool
}

Expand Down Expand Up @@ -361,16 +361,16 @@ func (s *ClusterScope) Vnet() *infrav1.VnetSpec {
}

// IsVnetManaged returns true if the vnet is managed.
func (s *ClusterScope) IsVnetManaged(ctx context.Context) (bool, error) {
if s.cache.IsVnetManaged == nil {
vnetSvc := virtualnetworks.New(s)
if managed, err := vnetSvc.IsVnetManaged(ctx); err != nil {
return false, err
} else {
s.cache.IsVnetManaged = &managed
}
func (s *ClusterScope) IsVnetManaged() (bool, error) {
if s.cache.IsVnetManaged != nil {
return to.Bool(s.cache.IsVnetManaged), nil
}
return to.Bool(s.cache.IsVnetManaged), nil
return false, errors.New("could not determine if vnet is managed")
}

// SetVnetManagedCache stores the value of VNet management in the cluster cache so it can be accessed later in the reconcile.
func (s *ClusterScope) SetVnetManagedCache(managed bool) {
s.cache.IsVnetManaged = &managed
}

// IsIPv6Enabled returns true if IPv6 is enabled.
Expand Down
16 changes: 13 additions & 3 deletions azure/scope/managedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"strings"

"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/to"
"github.com/go-logr/logr"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -102,6 +103,7 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
InfraMachinePool: params.InfraMachinePool,
PatchTarget: params.PatchTarget,
patchHelper: helper,
cache: &clusterCache{},
}, nil
}

Expand All @@ -118,6 +120,7 @@ type ManagedControlPlaneScope struct {
ControlPlane *infrav1exp.AzureManagedControlPlane
InfraMachinePool *infrav1exp.AzureManagedMachinePool
PatchTarget client.Object
cache *clusterCache

SystemNodePools []infrav1exp.AzureManagedMachinePool
}
Expand Down Expand Up @@ -299,9 +302,16 @@ func (s *ManagedControlPlaneScope) IsIPv6Enabled() bool {
}

// IsVnetManaged returns true if the vnet is managed.
func (s *ManagedControlPlaneScope) IsVnetManaged(ctx context.Context) (bool, error) {
vnetSvc := virtualnetworks.New(s)
return vnetSvc.IsVnetManaged(ctx)
func (s *ManagedControlPlaneScope) IsVnetManaged() (bool, error) {
if s.cache.IsVnetManaged != nil {
return to.Bool(s.cache.IsVnetManaged), nil
}
return false, errors.New("could not determine if vnet is managed")
}

// SetVnetManagedCache stores the value of VNet management in the cluster cache so it can be accessed later in the reconcile.
func (s *ManagedControlPlaneScope) SetVnetManagedCache(managed bool) {
s.cache.IsVnetManaged = &managed
}

// APIServerLBName returns the API Server LB name.
Expand Down
16 changes: 8 additions & 8 deletions azure/services/scalesets/scalesets.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (s *Service) Reconcile(ctx context.Context) (retErr error) {

// check if there is an ongoing long running operation
var (
future = s.Scope.GetLongRunningOperationState(s.Scope.ScaleSetSpec().Name, serviceName)
future = s.Scope.GetLongRunningOperationState(s.Scope.ScaleSetSpec().Name, ServiceName)
fetchedVMSS *azure.VMSS
)

Expand Down Expand Up @@ -144,7 +144,7 @@ func (s *Service) Reconcile(ctx context.Context) (retErr error) {
}

// if we get to here, we have completed any long running VMSS operations (creates / updates)
s.Scope.DeleteLongRunningOperationState(s.Scope.ScaleSetSpec().Name, serviceName)
s.Scope.DeleteLongRunningOperationState(s.Scope.ScaleSetSpec().Name, ServiceName)
return nil
}

Expand All @@ -171,7 +171,7 @@ func (s *Service) Delete(ctx context.Context) error {
}()

// check if there is an ongoing long running operation
future := s.Scope.GetLongRunningOperationState(vmssSpec.Name, serviceName)
future := s.Scope.GetLongRunningOperationState(vmssSpec.Name, ServiceName)
if future != nil {
// if the operation is not complete this will return an error
_, err := s.GetResultIfDone(ctx, future)
Expand All @@ -180,7 +180,7 @@ func (s *Service) Delete(ctx context.Context) error {
}

// ScaleSet has been deleted
s.Scope.DeleteLongRunningOperationState(vmssSpec.Name, serviceName)
s.Scope.DeleteLongRunningOperationState(vmssSpec.Name, ServiceName)
return nil
}

Expand All @@ -195,7 +195,7 @@ func (s *Service) Delete(ctx context.Context) error {
return errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", vmssSpec.Name, s.Scope.ResourceGroup())
}

future, err = converters.SDKToFuture(sdkFuture, infrav1.DeleteFuture, serviceName, vmssSpec.Name, s.Scope.ResourceGroup())
future, err = converters.SDKToFuture(sdkFuture, infrav1.DeleteFuture, ServiceName, vmssSpec.Name, s.Scope.ResourceGroup())
if err != nil {
return errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", vmssSpec.Name, s.Scope.ResourceGroup())
}
Expand All @@ -209,7 +209,7 @@ func (s *Service) Delete(ctx context.Context) error {
}

// future is either nil, or the result of the future is complete
s.Scope.DeleteLongRunningOperationState(vmssSpec.Name, serviceName)
s.Scope.DeleteLongRunningOperationState(vmssSpec.Name, ServiceName)
return nil
}

Expand All @@ -229,7 +229,7 @@ func (s *Service) createVMSS(ctx context.Context) (*infrav1.Future, error) {
return nil, errors.Wrap(err, "cannot create VMSS")
}

future, err := converters.SDKToFuture(sdkFuture, infrav1.PutFuture, serviceName, spec.Name, s.Scope.ResourceGroup())
future, err := converters.SDKToFuture(sdkFuture, infrav1.PutFuture, ServiceName, spec.Name, s.Scope.ResourceGroup())
if err != nil {
return future, errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", spec.Name, s.Scope.ResourceGroup())
}
Expand Down Expand Up @@ -284,7 +284,7 @@ func (s *Service) patchVMSSIfNeeded(ctx context.Context, infraVMSS *azure.VMSS)
return nil, errors.Wrap(err, "failed updating VMSS")
}

future, err := converters.SDKToFuture(sdkFuture, infrav1.PatchFuture, serviceName, spec.Name, s.Scope.ResourceGroup())
future, err := converters.SDKToFuture(sdkFuture, infrav1.PatchFuture, ServiceName, spec.Name, s.Scope.ResourceGroup())
if err != nil {
return nil, errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", spec.Name, s.Scope.ResourceGroup())
}
Expand Down
6 changes: 3 additions & 3 deletions azure/services/securitygroups/securitygroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type NSGScope interface {
azure.Authorizer
azure.AsyncStatusUpdater
NSGSpecs() []azure.ResourceSpecGetter
IsVnetManaged(ctx context.Context) (bool, error)
IsVnetManaged() (bool, error)
}

// Service provides operations on Azure resources.
Expand All @@ -64,7 +64,7 @@ func (s *Service) Reconcile(ctx context.Context) error {

// Only create the NSGs if their lifecycle is managed by this controller.
// NSGs are managed if and only if the vnet is managed.
managed, err := s.Scope.IsVnetManaged(ctx)
managed, err := s.Scope.IsVnetManaged()
if err != nil {
return errors.Wrap(err, "failed to determine if network security groups are managed")

Expand Down Expand Up @@ -100,7 +100,7 @@ func (s *Service) Delete(ctx context.Context) error {

// Only delete the NSG if its lifecycle is managed by this controller.
// NSGs are managed if and only if the vnet is managed.
managed, err := s.Scope.IsVnetManaged(ctx)
managed, err := s.Scope.IsVnetManaged()
if err != nil {
return errors.Wrap(err, "failed to determine if network security groups are managed")
} else if !managed {
Expand Down
2 changes: 1 addition & 1 deletion azure/services/subnets/subnets.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
s.Scope.SetSubnet(*existingSubnet)
continue
default:
managed, err := s.Scope.IsVnetManaged(ctx)
managed, err := s.Scope.IsVnetManaged()
if err != nil {
return errors.Wrap(err, "failed to check if vnet is managed")
} else if !managed {
Expand Down
15 changes: 11 additions & 4 deletions azure/services/virtualnetworks/virtualnetworks.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type VNetScope interface {
Vnet() *infrav1.VnetSpec
VNetSpec() azure.ResourceSpecGetter
ClusterName() string
IsVnetManaged(ctx context.Context) (bool, error)
SetVnetManagedCache(bool)
}

// Service provides operations on Azure resources.
Expand Down Expand Up @@ -69,6 +69,11 @@ func (s *Service) Reconcile(ctx context.Context) (err error) {

err = async.CreateResource(ctx, s.Scope, s.Client, vnetSpec, serviceName)
s.Scope.UpdatePutStatus(infrav1.VNetReadyCondition, serviceName, err)
if err != nil {
return err
}
// check and store whether the vnet is BYO so other services can use the cached information.
_, err = s.IsVnetManaged(ctx)
return err
}

Expand All @@ -81,7 +86,7 @@ func (s *Service) Delete(ctx context.Context) error {
defer cancel()

// Check that the vnet is not BYO.
managed, err := s.Scope.IsVnetManaged(ctx)
managed, err := s.IsVnetManaged(ctx)
if err != nil {
if azure.ResourceNotFound(err) {
// already deleted or doesn't exist
Expand All @@ -102,7 +107,7 @@ func (s *Service) Delete(ctx context.Context) error {
}

// IsVnetManaged returns true if the virtual network has an owned tag with the cluster name as value,
// meaning that the vnet's lifecycle is managed.
// meaning that the vnet's lifecycle is managed, and caches the result in scope so that other services that depend on the vnet can check if it is managed.
func (s *Service) IsVnetManaged(ctx context.Context) (bool, error) {
ctx, span := tele.Tracer().Start(ctx, "virtualnetworks.Service.IsVnetManaged")
defer span.End()
Expand All @@ -113,5 +118,7 @@ func (s *Service) IsVnetManaged(ctx context.Context) (bool, error) {
return false, err
}
tags := converters.MapToTags(vnet.Tags)
return tags.HasOwned(s.Scope.ClusterName()), nil
managed := tags.HasOwned(s.Scope.ClusterName())
s.Scope.SetVnetManagedCache(managed)
return managed, nil
}

0 comments on commit 590e79d

Please sign in to comment.