Skip to content

Commit

Permalink
use a regional base uri for making VMSS requests
Browse files Browse the repository at this point in the history
  • Loading branch information
devigned committed Nov 11, 2021
1 parent 0ac1bef commit c87eec2
Show file tree
Hide file tree
Showing 7 changed files with 124 additions and 27 deletions.
37 changes: 37 additions & 0 deletions azure/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package azure
import (
"fmt"
"net/http"
"net/url"
"path"

"github.com/Azure/go-autorest/autorest/azure"

Expand Down Expand Up @@ -406,3 +408,38 @@ func msCorrelationIDSendDecorator(snd autorest.Sender) autorest.Sender {
return snd.Do(r)
})
}

type aliasAuth = Authorizer

// baseURIAddapter wraps an azure.Authorizer and adds a region to the BaseURI. This is useful if you need to make direct
// calls to a specific Azure region. One possible case is to avoid replication delay when listing resources within a
// resource group. For example, listing the VMSSes within a resource group.
type baseURIAddapter struct {
aliasAuth
Region string
parsedURL *url.URL
}

// WithRegionalBaseURI returns an authorizer that has a regional base URI, like `https://{region}.management.azure.com`.
func WithRegionalBaseURI(authorizer Authorizer, region string) (Authorizer, error) {
parsedURI, err := url.Parse(authorizer.BaseURI())
if err != nil {
return nil, errors.Wrap(err, "failed to parse the base URI of client")
}

return &baseURIAddapter{
aliasAuth: authorizer,
Region: region,
parsedURL: parsedURI,
}, nil
}

// BaseURI return a regional base URI, like `https://{region}.management.azure.com`.
func (a *baseURIAddapter) BaseURI() string {
if a == nil || a.parsedURL == nil || a.Region == "" {
return a.aliasAuth.BaseURI()
}

sansScheme := path.Join(fmt.Sprintf("%s.%s", a.Region, a.parsedURL.Host), a.parsedURL.Path)
return fmt.Sprintf("%s://%s", a.parsedURL.Scheme, sansScheme)
}
52 changes: 52 additions & 0 deletions azure/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ import (
"testing"

"github.com/Azure/go-autorest/autorest"
"github.com/golang/mock/gomock"
. "github.com/onsi/gomega"

"sigs.k8s.io/cluster-api-provider-azure/azure/mock_azure"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
)

Expand Down Expand Up @@ -288,3 +291,52 @@ func TestMSCorrelationIDSendDecorator(t *testing.T) {
receivedReq.Header.Get(string(tele.CorrIDKeyVal)),
).To(Equal(string(corrID)))
}

func TestWithRegionalBaseURI(t *testing.T) {
cases := []struct {
Name string
AuthorizerFactory func(authMock *mock_azure.MockAuthorizer) Authorizer
Region string
Result string
}{
{
Name: "with a region",
AuthorizerFactory: func(authMock *mock_azure.MockAuthorizer) Authorizer {
authMock.EXPECT().BaseURI().Return("http://foo.bar").AnyTimes()
return authMock
},
Region: "bazz",
Result: "http://bazz.foo.bar",
},
{
Name: "with no region",
AuthorizerFactory: func(authMock *mock_azure.MockAuthorizer) Authorizer {
authMock.EXPECT().BaseURI().Return("http://foo.bar").AnyTimes()
return authMock
},
Result: "http://foo.bar",
},
{
Name: "with a region and path",
AuthorizerFactory: func(authMock *mock_azure.MockAuthorizer) Authorizer {
authMock.EXPECT().BaseURI().Return("http://foo.bar/something/id").AnyTimes()
return authMock
},
Region: "bazz",
Result: "http://bazz.foo.bar/something/id",
},
}

for _, c := range cases {
c := c
t.Run(c.Name, func(t *testing.T) {
g := NewWithT(t)
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
authMock := mock_azure.NewMockAuthorizer(mockCtrl)
regionalAuth, err := WithRegionalBaseURI(c.AuthorizerFactory(authMock), c.Region)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(regionalAuth.BaseURI()).To(Equal(c.Result))
})
}
}
33 changes: 22 additions & 11 deletions exp/controllers/azuremanagedmachinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,6 @@ import (
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/tools/record"
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/scope"
infracontroller "sigs.k8s.io/cluster-api-provider-azure/controllers"
infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/pkg/coalescing"
"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
clusterv1exp "sigs.k8s.io/cluster-api/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/util"
Expand All @@ -44,6 +36,15 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/scope"
infracontroller "sigs.k8s.io/cluster-api-provider-azure/controllers"
infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/pkg/coalescing"
"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
)

// AzureManagedMachinePoolReconciler reconciles an AzureManagedMachinePool object.
Expand All @@ -56,7 +57,7 @@ type AzureManagedMachinePoolReconciler struct {
createAzureManagedMachinePoolService azureManagedMachinePoolServiceCreator
}

type azureManagedMachinePoolServiceCreator func(managedControlPlaneScope *scope.ManagedControlPlaneScope) *azureManagedMachinePoolService
type azureManagedMachinePoolServiceCreator func(managedControlPlaneScope *scope.ManagedControlPlaneScope) (*azureManagedMachinePoolService, error)

// NewAzureManagedMachinePoolReconciler returns a new AzureManagedMachinePoolReconciler instance.
func NewAzureManagedMachinePoolReconciler(client client.Client, log logr.Logger, recorder record.EventRecorder, reconcileTimeout time.Duration, watchFilterValue string) *AzureManagedMachinePoolReconciler {
Expand Down Expand Up @@ -240,7 +241,12 @@ func (ammpr *AzureManagedMachinePoolReconciler) reconcileNormal(ctx context.Cont
return reconcile.Result{}, err
}

if err := ammpr.createAzureManagedMachinePoolService(scope).Reconcile(ctx); err != nil {
svc, err := ammpr.createAzureManagedMachinePoolService(scope)
if err != nil {
return reconcile.Result{}, errors.Wrap(err, "failed to create an AzureManageMachinePoolService")
}

if err := svc.Reconcile(ctx); err != nil {
// Handle transient and terminal errors
log := scope.WithValues("name", scope.InfraMachinePool.Name, "namespace", scope.InfraMachinePool.Namespace)
var reconcileError azure.ReconcileError
Expand Down Expand Up @@ -278,7 +284,12 @@ func (ammpr *AzureManagedMachinePoolReconciler) reconcileDelete(ctx context.Cont
// So, remove the finalizer.
controllerutil.RemoveFinalizer(scope.InfraMachinePool, infrav1.ClusterFinalizer)
} else {
if err := ammpr.createAzureManagedMachinePoolService(scope).Delete(ctx); err != nil {
svc, err := ammpr.createAzureManagedMachinePoolService(scope)
if err != nil {
return reconcile.Result{}, errors.Wrap(err, "failed to create an AzureManageMachinePoolService")
}

if err := svc.Delete(ctx); err != nil {
return reconcile.Result{}, errors.Wrapf(err, "error deleting AzureManagedMachinePool %s/%s", scope.InfraMachinePool.Namespace, scope.InfraMachinePool.Name)
}
// Machine pool successfully deleted, remove the finalizer.
Expand Down
15 changes: 12 additions & 3 deletions exp/controllers/azuremanagedmachinepool_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,21 @@ func (a *AgentPoolVMSSNotFoundError) Is(target error) bool {
}

// newAzureManagedMachinePoolService populates all the services based on input scope.
func newAzureManagedMachinePoolService(scope *scope.ManagedControlPlaneScope) *azureManagedMachinePoolService {
func newAzureManagedMachinePoolService(scope *scope.ManagedControlPlaneScope) (*azureManagedMachinePoolService, error) {
var authorizer azure.Authorizer = scope
if scope.ControlPlane != nil {
regionalAuthorizer, err := azure.WithRegionalBaseURI(scope, scope.ControlPlane.Spec.Location)
if err != nil {
return nil, errors.Wrap(err, "failed to create a regional authorizer")
}
authorizer = regionalAuthorizer
}

return &azureManagedMachinePoolService{
scope: scope,
agentPoolsSvc: agentpools.New(scope),
scaleSetsSvc: scalesets.NewClient(scope),
}
scaleSetsSvc: scalesets.NewClient(authorizer),
}, nil
}

// Reconcile reconciles all the services in a predetermined order.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ spec:
kind: AzureClusterIdentity
name: ${CLUSTER_IDENTITY_NAME}
namespace: ${CLUSTER_IDENTITY_NAMESPACE}
location: northcentralus
location: ${AZURE_LOCATION}
resourceGroupName: ${AZURE_RESOURCE_GROUP:=${CLUSTER_NAME}}
sshPublicKey: ${AZURE_SSH_PUBLIC_KEY_B64:=""}
subscriptionID: ${AZURE_SUBSCRIPTION_ID}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,3 @@ resources:
- ../../../flavors/aks-multi-tenancy
patchesStrategicMerge:
- ../patches/tags-aks.yaml
- ./patch_location.yaml
11 changes: 0 additions & 11 deletions templates/test/ci/prow-aks-multi-tenancy/patch_location.yaml

This file was deleted.

0 comments on commit c87eec2

Please sign in to comment.