Skip to content

Commit

Permalink
Merge pull request #3723 from nojnhuh/aso-wired
Browse files Browse the repository at this point in the history
Enable ASO
  • Loading branch information
k8s-ci-robot authored Aug 29, 2023
2 parents 145999b + 270f646 commit 34aa125
Show file tree
Hide file tree
Showing 20 changed files with 389 additions and 160 deletions.
1 change: 1 addition & 0 deletions Tiltfile
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ def observability():

k8s_resource(workload = "capz-controller-manager", labels = ["cluster-api"])
k8s_resource(workload = "capz-nmi", labels = ["cluster-api"])
k8s_resource(workload = "azureserviceoperator-controller-manager", labels = ["cluster-api"])

# Build CAPZ and add feature gates
def capz():
Expand Down
40 changes: 26 additions & 14 deletions azure/scope/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc
return nil, errors.New("failed to generate new scope from nil AzureCluster")
}

useLegacyGroups := false
if params.AzureCluster.Spec.IdentityRef == nil {
err := params.AzureClients.setCredentials(params.AzureCluster.Spec.SubscriptionID, params.AzureCluster.Spec.AzureEnvironment)
if err != nil {
Expand All @@ -89,6 +90,12 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc
if err != nil {
return nil, errors.Wrap(err, "failed to configure azure settings and credentials for Identity")
}

// ASO does not yet support per-resource credentials using UserAssignedMSI. Fallback to the legacy SDK
// groups service when it is used.
if credentialsProvider.Identity.Spec.Type == infrav1.UserAssignedMSI {
useLegacyGroups = true
}
}

if params.Cache == nil {
Expand All @@ -101,12 +108,13 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc
}

return &ClusterScope{
Client: params.Client,
AzureClients: params.AzureClients,
Cluster: params.Cluster,
AzureCluster: params.AzureCluster,
patchHelper: helper,
cache: params.Cache,
Client: params.Client,
AzureClients: params.AzureClients,
Cluster: params.Cluster,
AzureCluster: params.AzureCluster,
patchHelper: helper,
cache: params.Cache,
UseLegacyGroups: useLegacyGroups,
}, nil
}

Expand All @@ -117,8 +125,9 @@ type ClusterScope struct {
cache *ClusterCache

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

// ClusterCache stores ClusterCache data locally so we don't have to hit the API multiple times within the same reconcile loop.
Expand Down Expand Up @@ -1062,13 +1071,16 @@ func (s *ClusterScope) SetAnnotation(key, value string) {

// TagsSpecs returns the tag specs for the AzureCluster.
func (s *ClusterScope) TagsSpecs() []azure.TagsSpec {
return []azure.TagsSpec{
{
Scope: azure.ResourceGroupID(s.SubscriptionID(), s.ResourceGroup()),
Tags: s.AdditionalTags(),
Annotation: azure.RGTagsLastAppliedAnnotation,
},
if s.UseLegacyGroups {
return []azure.TagsSpec{
{
Scope: azure.ResourceGroupID(s.SubscriptionID(), s.ResourceGroup()),
Tags: s.AdditionalTags(),
Annotation: azure.RGTagsLastAppliedAnnotation,
},
}
}
return nil
}

// PrivateEndpointSpecs returns the private endpoint specs.
Expand Down
24 changes: 18 additions & 6 deletions azure/scope/managedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
return nil, errors.New("failed to generate new scope from nil ControlPlane")
}

useLegacyGroups := false
if params.ControlPlane.Spec.IdentityRef == nil {
if err := params.AzureClients.setCredentials(params.ControlPlane.Spec.SubscriptionID, params.ControlPlane.Spec.AzureEnvironment); err != nil {
return nil, errors.Wrap(err, "failed to create Azure session")
Expand All @@ -86,6 +87,12 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
if err := params.AzureClients.setCredentialsWithProvider(ctx, params.ControlPlane.Spec.SubscriptionID, params.ControlPlane.Spec.AzureEnvironment, credentialsProvider); err != nil {
return nil, errors.Wrap(err, "failed to configure azure settings and credentials for Identity")
}

// ASO does not yet support per-resource credentials using UserAssignedMSI. Fallback to the legacy SDK
// groups service when it is used.
if credentialsProvider.Identity.Spec.Type == infrav1.UserAssignedMSI {
useLegacyGroups = true
}
}

if params.Cache == nil {
Expand All @@ -105,6 +112,7 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
ManagedMachinePools: params.ManagedMachinePools,
patchHelper: helper,
cache: params.Cache,
UseLegacyGroups: useLegacyGroups,
}, nil
}

Expand All @@ -119,6 +127,7 @@ type ManagedControlPlaneScope struct {
Cluster *clusterv1.Cluster
ControlPlane *infrav1.AzureManagedControlPlane
ManagedMachinePools []ManagedMachinePool
UseLegacyGroups bool
}

// ManagedControlPlaneCache stores ManagedControlPlane data locally so we don't have to hit the API multiple times within the same reconcile loop.
Expand Down Expand Up @@ -731,18 +740,21 @@ func (s *ManagedControlPlaneScope) SetAnnotation(key, value string) {

// TagsSpecs returns the tag specs for the ManagedControlPlane.
func (s *ManagedControlPlaneScope) TagsSpecs() []azure.TagsSpec {
return []azure.TagsSpec{
{
Scope: azure.ResourceGroupID(s.SubscriptionID(), s.ResourceGroup()),
Tags: s.AdditionalTags(),
Annotation: azure.RGTagsLastAppliedAnnotation,
},
specs := []azure.TagsSpec{
{
Scope: azure.ManagedClusterID(s.SubscriptionID(), s.ResourceGroup(), s.ManagedClusterSpec().ResourceName()),
Tags: s.AdditionalTags(),
Annotation: azure.ManagedClusterTagsLastAppliedAnnotation,
},
}
if s.UseLegacyGroups {
specs = append(specs, azure.TagsSpec{
Scope: azure.ResourceGroupID(s.SubscriptionID(), s.ResourceGroup()),
Tags: s.AdditionalTags(),
Annotation: azure.RGTagsLastAppliedAnnotation,
})
}
return specs
}

// AvailabilityStatusResource refers to the AzureManagedControlPlane.
Expand Down
4 changes: 2 additions & 2 deletions azure/services/asogroups/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (s *Service) Name() string {

// Reconcile idempotently creates or updates a resource group.
func (s *Service) Reconcile(ctx context.Context) error {
ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.Service.Reconcile")
ctx, _, done := tele.StartSpanWithLogger(ctx, "asogroups.Service.Reconcile")
defer done()

ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout)
Expand All @@ -77,7 +77,7 @@ func (s *Service) Reconcile(ctx context.Context) error {

// Delete deletes the resource group if it is managed by capz.
func (s *Service) Delete(ctx context.Context) error {
ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.Service.Delete")
ctx, _, done := tele.StartSpanWithLogger(ctx, "asogroups.Service.Delete")
defer done()

ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout)
Expand Down
131 changes: 48 additions & 83 deletions azure/services/asogroups/groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,6 @@ func TestReconcileGroups(t *testing.T) {
}
}

type ErroringGetClient struct {
client.Client
err error
}

func (e *ErroringGetClient) Get(_ context.Context, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error {
return e.err
}

type ErroringDeleteClient struct {
client.Client
err error
Expand Down Expand Up @@ -223,48 +214,32 @@ func TestDeleteGroups(t *testing.T) {

func TestShouldDeleteIndividualResources(t *testing.T) {
tests := []struct {
name string
clientBuilder func(g Gomega) client.Client
expect func(s *mock_asogroups.MockGroupScopeMockRecorder)
expected bool
name string
objects []client.Object
expect func(s *mock_asogroups.MockGroupScopeMockRecorder)
expected bool
}{
{
name: "error checking if group is managed",
clientBuilder: func(g Gomega) client.Client {
scheme := runtime.NewScheme()
g.Expect(asoresourcesv1.AddToScheme(scheme))
c := fakeclient.NewClientBuilder().
WithScheme(scheme).
Build()
return &ErroringGetClient{
Client: c,
err: errors.New("an error"),
}
},
name: "error checking if group is managed",
objects: nil,
expect: func(s *mock_asogroups.MockGroupScopeMockRecorder) {
s.ASOGroupSpec().Return(&GroupSpec{}).AnyTimes()
s.ClusterName().Return("").AnyTimes()
},
expected: true,
},
{
name: "unmanaged",
clientBuilder: func(g Gomega) client.Client {
scheme := runtime.NewScheme()
g.Expect(asoresourcesv1.AddToScheme(scheme))
c := fakeclient.NewClientBuilder().
WithScheme(scheme).
WithObjects(&asoresourcesv1.ResourceGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
Labels: map[string]string{
infrav1.OwnedByClusterLabelKey: "not-cluster",
},
name: "group is unmanaged",
objects: []client.Object{
&asoresourcesv1.ResourceGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
Labels: map[string]string{
infrav1.OwnedByClusterLabelKey: "not-cluster",
},
}).
Build()
return c
},
},
},
expect: func(s *mock_asogroups.MockGroupScopeMockRecorder) {
s.ASOGroupSpec().Return(&GroupSpec{
Expand All @@ -276,26 +251,20 @@ func TestShouldDeleteIndividualResources(t *testing.T) {
expected: true,
},
{
name: "managed, RG has reconcile policy skip",
clientBuilder: func(g Gomega) client.Client {
scheme := runtime.NewScheme()
g.Expect(asoresourcesv1.AddToScheme(scheme))
c := fakeclient.NewClientBuilder().
WithScheme(scheme).
WithObjects(&asoresourcesv1.ResourceGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
Labels: map[string]string{
infrav1.OwnedByClusterLabelKey: "cluster",
},
Annotations: map[string]string{
aso.ReconcilePolicyAnnotation: aso.ReconcilePolicySkip,
},
name: "group is managed and has reconcile policy skip",
objects: []client.Object{
&asoresourcesv1.ResourceGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
Labels: map[string]string{
infrav1.OwnedByClusterLabelKey: "cluster",
},
}).
Build()
return c
Annotations: map[string]string{
aso.ReconcilePolicyAnnotation: aso.ReconcilePolicySkip,
},
},
},
},
expect: func(s *mock_asogroups.MockGroupScopeMockRecorder) {
s.ASOGroupSpec().Return(&GroupSpec{
Expand All @@ -307,26 +276,20 @@ func TestShouldDeleteIndividualResources(t *testing.T) {
expected: true,
},
{
name: "managed, RG has reconcile policy manage",
clientBuilder: func(g Gomega) client.Client {
scheme := runtime.NewScheme()
g.Expect(asoresourcesv1.AddToScheme(scheme))
c := fakeclient.NewClientBuilder().
WithScheme(scheme).
WithObjects(&asoresourcesv1.ResourceGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
Labels: map[string]string{
infrav1.OwnedByClusterLabelKey: "cluster",
},
Annotations: map[string]string{
aso.ReconcilePolicyAnnotation: aso.ReconcilePolicyManage,
},
name: "group is managed and has reconcile policy manage",
objects: []client.Object{
&asoresourcesv1.ResourceGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
Labels: map[string]string{
infrav1.OwnedByClusterLabelKey: "cluster",
},
}).
Build()
return c
Annotations: map[string]string{
aso.ReconcilePolicyAnnotation: aso.ReconcilePolicyManage,
},
},
},
},
expect: func(s *mock_asogroups.MockGroupScopeMockRecorder) {
s.ASOGroupSpec().Return(&GroupSpec{
Expand All @@ -347,10 +310,12 @@ func TestShouldDeleteIndividualResources(t *testing.T) {
defer mockCtrl.Finish()
scopeMock := mock_asogroups.NewMockGroupScope(mockCtrl)

var ctrlClient client.Client
if test.clientBuilder != nil {
ctrlClient = test.clientBuilder(g)
}
scheme := runtime.NewScheme()
g.Expect(asoresourcesv1.AddToScheme(scheme))
ctrlClient := fakeclient.NewClientBuilder().
WithScheme(scheme).
WithObjects(test.objects...).
Build()
scopeMock.EXPECT().GetClient().Return(ctrlClient).AnyTimes()
test.expect(scopeMock.EXPECT())

Expand Down
8 changes: 4 additions & 4 deletions config/aso/credentials.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ metadata:
name: aso-controller-settings
type: Opaque
data:
AZURE_SUBSCRIPTION_ID: ${AZURE_SUBSCRIPTION_ID_B64:=""}
AZURE_TENANT_ID: ${AZURE_TENANT_ID_B64:=""}
AZURE_CLIENT_ID: ${AZURE_CLIENT_ID_B64:=""}
AZURE_CLIENT_SECRET: ${AZURE_CLIENT_SECRET_B64:=""}
# Per-resource Secrets will be created based on a Cluster's AzureClusterIdentity.
AZURE_SUBSCRIPTION_ID: ""
AZURE_TENANT_ID: ""
AZURE_CLIENT_ID: ""
3 changes: 3 additions & 0 deletions config/default/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
resources:
- ../capz

components:
- ../aso
20 changes: 20 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -261,3 +261,23 @@ rules:
- get
- patch
- update
- apiGroups:
- resources.azure.com
resources:
- resourcegroups
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- resources.azure.com
resources:
- resourcegroups/status
verbs:
- get
- list
- watch
2 changes: 2 additions & 0 deletions controllers/azurecluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ func (acr *AzureClusterReconciler) SetupWithManager(ctx context.Context, mgr ctr
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azuremachinetemplates;azuremachinetemplates/status,verbs=get;list;watch
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=azureclusteridentities;azureclusteridentities/status,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups="",resources=namespaces,verbs=list;
// +kubebuilder:rbac:groups=resources.azure.com,resources=resourcegroups,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=resources.azure.com,resources=resourcegroups/status,verbs=get;list;watch

// Reconcile idempotently gets, creates, and updates a cluster.
func (acr *AzureClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
Expand Down
Loading

0 comments on commit 34aa125

Please sign in to comment.