From 4f9943e3235ca520bcaeacfbf6f10c9333dfe60b Mon Sep 17 00:00:00 2001 From: Jon Huhn Date: Tue, 6 Feb 2024 16:22:52 -0600 Subject: [PATCH] Add patches fields for AKS resources --- api/v1beta1/types_class.go | 14 ++ api/v1beta1/zz_generated.deepcopy.go | 10 + azure/scope/managedcontrolplane.go | 1 + azure/scope/managedmachinepool.go | 1 + azure/services/agentpools/spec.go | 11 + azure/services/aso/aso.go | 65 +++++- azure/services/aso/aso_test.go | 119 +++++++++++ azure/services/aso/interfaces.go | 5 + azure/services/aso/mock_aso/aso_mock.go | 37 ++++ azure/services/managedclusters/spec.go | 17 +- ...er.x-k8s.io_azuremanagedcontrolplanes.yaml | 10 + ....io_azuremanagedcontrolplanetemplates.yaml | 11 + ...ter.x-k8s.io_azuremanagedmachinepools.yaml | 10 + ...s.io_azuremanagedmachinepooltemplates.yaml | 11 + go.mod | 2 +- test/e2e/aks_patches.go | 192 ++++++++++++++++++ test/e2e/azure_test.go | 10 + 17 files changed, 521 insertions(+), 5 deletions(-) create mode 100644 test/e2e/aks_patches.go diff --git a/api/v1beta1/types_class.go b/api/v1beta1/types_class.go index ebb21773b141..64178da17cd7 100644 --- a/api/v1beta1/types_class.go +++ b/api/v1beta1/types_class.go @@ -235,6 +235,13 @@ type AzureManagedControlPlaneClassSpec struct { // SecurityProfile defines the security profile for cluster. // +optional SecurityProfile *ManagedClusterSecurityProfile `json:"securityProfile,omitempty"` + + // ASOManagedClusterPatches defines JSON merge patches to be applied to the generated ASO ManagedCluster resource. + // WARNING: This is meant to be used sparingly to enable features for development and testing that are not + // otherwise represented in the CAPZ API. Misconfiguration that conflicts with CAPZ's normal mode of + // operation is possible. + // +optional + ASOManagedClusterPatches []string `json:"asoManagedClusterPatches,omitempty"` } // ManagedClusterAutoUpgradeProfile defines the auto upgrade profile for a managed cluster. @@ -393,6 +400,13 @@ type AzureManagedMachinePoolClassSpec struct { // [AKS doc]: https://learn.microsoft.com/en-us/azure/aks/enable-host-encryption // +optional EnableEncryptionAtHost *bool `json:"enableEncryptionAtHost,omitempty"` + + // ASOManagedClustersAgentPoolPatches defines JSON merge patches to be applied to the generated ASO ManagedClustersAgentPool resource. + // WARNING: This is meant to be used sparingly to enable features for development and testing that are not + // otherwise represented in the CAPZ API. Misconfiguration that conflicts with CAPZ's normal mode of + // operation is possible. + // +optional + ASOManagedClustersAgentPoolPatches []string `json:"asoManagedClustersAgentPoolPatches,omitempty"` } // ManagedControlPlaneVirtualNetworkClassSpec defines the ManagedControlPlaneVirtualNetwork properties that may be shared across several managed control plane vnets. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index b9ccee6b4d24..ee778ef4b24c 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1450,6 +1450,11 @@ func (in *AzureManagedControlPlaneClassSpec) DeepCopyInto(out *AzureManagedContr *out = new(ManagedClusterSecurityProfile) (*in).DeepCopyInto(*out) } + if in.ASOManagedClusterPatches != nil { + in, out := &in.ASOManagedClusterPatches, &out.ASOManagedClusterPatches + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureManagedControlPlaneClassSpec. @@ -1823,6 +1828,11 @@ func (in *AzureManagedMachinePoolClassSpec) DeepCopyInto(out *AzureManagedMachin *out = new(bool) **out = **in } + if in.ASOManagedClustersAgentPoolPatches != nil { + in, out := &in.ASOManagedClustersAgentPoolPatches, &out.ASOManagedClustersAgentPoolPatches + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureManagedMachinePoolClassSpec. diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index c26d35347e28..6ae9192bf134 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -547,6 +547,7 @@ func (s *ManagedControlPlaneScope) ManagedClusterSpec() azure.ASOResourceSpecGet KubeletUserAssignedIdentity: s.ControlPlane.Spec.KubeletUserAssignedIdentity, NetworkPluginMode: s.ControlPlane.Spec.NetworkPluginMode, DNSPrefix: s.ControlPlane.Spec.DNSPrefix, + Patches: s.ControlPlane.Spec.ASOManagedClusterPatches, } if s.ControlPlane.Spec.SSHPublicKey != nil { diff --git a/azure/scope/managedmachinepool.go b/azure/scope/managedmachinepool.go index c4011ff3b0a5..548ebbd588f6 100644 --- a/azure/scope/managedmachinepool.go +++ b/azure/scope/managedmachinepool.go @@ -197,6 +197,7 @@ func buildAgentPoolSpec(managedControlPlane *infrav1.AzureManagedControlPlane, LinuxOSConfig: managedMachinePool.Spec.LinuxOSConfig, EnableFIPS: managedMachinePool.Spec.EnableFIPS, EnableEncryptionAtHost: managedMachinePool.Spec.EnableEncryptionAtHost, + Patches: managedMachinePool.Spec.ASOManagedClustersAgentPoolPatches, } if managedMachinePool.Spec.OSDiskSizeGB != nil { diff --git a/azure/services/agentpools/spec.go b/azure/services/agentpools/spec.go index 654730efa1ec..91977598447e 100644 --- a/azure/services/agentpools/spec.go +++ b/azure/services/agentpools/spec.go @@ -26,6 +26,7 @@ import ( "k8s.io/utils/ptr" 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/services/aso" "sigs.k8s.io/cluster-api-provider-azure/util/tele" "sigs.k8s.io/cluster-api-provider-azure/util/versions" ) @@ -150,6 +151,9 @@ type AgentPoolSpec struct { // EnableEncryptionAtHost indicates whether host encryption is enabled on the node pool EnableEncryptionAtHost *bool + + // Patches are extra patches to be applied to the ASO resource. + Patches []string } // ResourceRef implements azure.ASOResourceSpecGetter. @@ -306,3 +310,10 @@ func (s *AgentPoolSpec) WasManaged(resource *asocontainerservicev1.ManagedCluste // CAPZ has never supported BYO agent pools. return true } + +var _ aso.Patcher = (*AgentPoolSpec)(nil) + +// ExtraPatches implements aso.Patcher. +func (s *AgentPoolSpec) ExtraPatches() []string { + return s.Patches +} diff --git a/azure/services/aso/aso.go b/azure/services/aso/aso.go index e3192f929daf..96c0d7a391c1 100644 --- a/azure/services/aso/aso.go +++ b/azure/services/aso/aso.go @@ -18,17 +18,22 @@ package aso import ( "context" + "encoding/json" "fmt" "time" asoannotations "github.com/Azure/azure-service-operator/v2/pkg/common/annotations" "github.com/Azure/azure-service-operator/v2/pkg/genruntime" "github.com/Azure/azure-service-operator/v2/pkg/genruntime/conditions" + jsonpatch "github.com/evanphx/json-patch/v5" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/util/yaml" 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/util/aso" @@ -53,6 +58,7 @@ const ( type deepCopier[T any] interface { genruntime.MetaObject DeepCopy() T + SetGroupVersionKind(schema.GroupVersionKind) } // reconciler is an implementation of the Reconciler interface. It handles creation @@ -150,7 +156,7 @@ func (r *reconciler[T]) CreateOrUpdateResource(ctx context.Context, spec azure.A } // Construct parameters using the resource spec and information from the existing resource, if there is one. - parameters, err := spec.Parameters(ctx, existing.DeepCopy()) + parameters, err := PatchedParameters(ctx, r.Scheme(), spec, existing.DeepCopy()) if err != nil { return zero, errors.Wrapf(err, "failed to get desired parameters for resource %s/%s (service: %s)", resourceNamespace, resourceName, serviceName) } @@ -220,11 +226,64 @@ func (r *reconciler[T]) CreateOrUpdateResource(ctx context.Context, spec azure.A log.V(2).Info("resource up to date") return existing, nil } - log.V(2).Info("creating or updating resource", "diff", cmp.Diff(existing, parameters)) + log.V(2).Info("creating or updating resource", "diff", diff) return r.createOrUpdateResource(ctx, existing, parameters, resourceExists, serviceName) } -func (r *reconciler[T]) createOrUpdateResource(ctx context.Context, existing T, parameters T, resourceExists bool, serviceName string) (T, error) { +// PatchedParameters returns the Parameters of spec with patches applied. +func PatchedParameters[T deepCopier[T]](ctx context.Context, scheme *runtime.Scheme, spec azure.ASOResourceSpecGetter[T], existing T) (T, error) { + var zero T // to be returned with non-nil errors + parameters, err := spec.Parameters(ctx, existing) + if err != nil { + return zero, err + } + return applyPatches(scheme, spec, parameters) +} + +func applyPatches[T deepCopier[T]](scheme *runtime.Scheme, spec azure.ASOResourceSpecGetter[T], parameters T) (T, error) { + p, ok := spec.(Patcher) + if !ok { + return parameters, nil + } + + var zero T // to be returned with non-nil errors + + gvk, err := apiutil.GVKForObject(parameters, scheme) + if err != nil { + return zero, errors.Wrap(err, "failed to get GroupVersionKind for object") + } + parameters.SetGroupVersionKind(gvk) + paramData, err := json.Marshal(parameters) + if err != nil { + return zero, errors.Wrap(err, "failed to marshal JSON for patch") + } + + for i, extraPatch := range p.ExtraPatches() { + jsonPatch, err := yaml.ToJSON([]byte(extraPatch)) + if err != nil { + return zero, errors.Wrapf(err, "failed to convert patch at index %d to JSON", i) + } + paramData, err = jsonpatch.MergePatch(paramData, jsonPatch) + if err != nil { + return zero, errors.Wrapf(err, "failed to apply patch at index %d", i) + } + } + + decoder := serializer.NewCodecFactory(scheme).UniversalDeserializer() + obj, _, err := decoder.Decode(paramData, nil, nil) + if err != nil { + return zero, errors.Wrap(err, "failed to decode object") + } + + t, ok := obj.(T) + if !ok { + return zero, fmt.Errorf("decoded patched object is %T, not %T", obj, parameters) + } + + return t, nil +} + +func (r *reconciler[T]) createOrUpdateResource(ctx context.Context, existing T, parameters client.Object, resourceExists bool, serviceName string) (T, error) { var zero T var err error var logMessageVerbPrefix string diff --git a/azure/services/aso/aso_test.go b/azure/services/aso/aso_test.go index 44ca0281ed30..bec977159488 100644 --- a/azure/services/aso/aso_test.go +++ b/azure/services/aso/aso_test.go @@ -923,6 +923,125 @@ func TestCreateOrUpdateResource(t *testing.T) { g.Expect(updated.Annotations).NotTo(HaveKey(prePauseReconcilePolicyAnnotation)) g.Expect(updated.Annotations).To(HaveKeyWithValue(asoannotations.ReconcilePolicy, string(asoannotations.ReconcilePolicyManage))) }) + + t.Run("patches applied on create", func(t *testing.T) { + g := NewGomegaWithT(t) + + sch := runtime.NewScheme() + g.Expect(asoresourcesv1.AddToScheme(sch)).To(Succeed()) + c := fakeclient.NewClientBuilder(). + WithScheme(sch). + Build() + s := New[*asoresourcesv1.ResourceGroup](c, clusterName, newOwner()) + + mockCtrl := gomock.NewController(t) + specMock := struct { + *mock_azure.MockASOResourceSpecGetter[*asoresourcesv1.ResourceGroup] + *mock_aso.MockPatcher + }{ + mock_azure.NewMockASOResourceSpecGetter[*asoresourcesv1.ResourceGroup](mockCtrl), + mock_aso.NewMockPatcher(mockCtrl), + } + specMock.MockASOResourceSpecGetter.EXPECT().ResourceRef().Return(&asoresourcesv1.ResourceGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + }, + }) + specMock.MockASOResourceSpecGetter.EXPECT().Parameters(gomockinternal.AContext(), gomock.Any()).DoAndReturn(func(_ context.Context, group *asoresourcesv1.ResourceGroup) (*asoresourcesv1.ResourceGroup, error) { + return &asoresourcesv1.ResourceGroup{ + Spec: asoresourcesv1.ResourceGroup_Spec{ + Location: ptr.To("location-from-parameters"), + }, + }, nil + }) + + specMock.MockPatcher.EXPECT().ExtraPatches().Return([]string{ + `{"metadata": {"labels": {"extra-patch": "not-this-value"}}}`, + `{"metadata": {"labels": {"extra-patch": "this-value"}}}`, + `{"metadata": {"labels": {"another": "label"}}}`, + }) + + ctx := context.Background() + + result, err := s.CreateOrUpdateResource(ctx, specMock, "service") + g.Expect(result).To(BeNil()) + g.Expect(azure.IsOperationNotDoneError(err)).To(BeTrue(), "expected not done error, got %v", err) + + updated := &asoresourcesv1.ResourceGroup{} + g.Expect(c.Get(ctx, types.NamespacedName{Name: "name", Namespace: "namespace"}, updated)).To(Succeed()) + g.Expect(updated.Labels).To(HaveKeyWithValue("extra-patch", "this-value")) + g.Expect(updated.Labels).To(HaveKeyWithValue("another", "label")) + g.Expect(*updated.Spec.Location).To(Equal("location-from-parameters")) + }) + + t.Run("patches applied on update", func(t *testing.T) { + g := NewGomegaWithT(t) + + sch := runtime.NewScheme() + g.Expect(asoresourcesv1.AddToScheme(sch)).To(Succeed()) + c := fakeclient.NewClientBuilder(). + WithScheme(sch). + Build() + s := New[*asoresourcesv1.ResourceGroup](c, clusterName, newOwner()) + + mockCtrl := gomock.NewController(t) + specMock := struct { + *mock_azure.MockASOResourceSpecGetter[*asoresourcesv1.ResourceGroup] + *mock_aso.MockPatcher + }{ + mock_azure.NewMockASOResourceSpecGetter[*asoresourcesv1.ResourceGroup](mockCtrl), + mock_aso.NewMockPatcher(mockCtrl), + } + specMock.MockASOResourceSpecGetter.EXPECT().ResourceRef().Return(&asoresourcesv1.ResourceGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + }, + }) + specMock.MockASOResourceSpecGetter.EXPECT().Parameters(gomockinternal.AContext(), gomock.Any()).DoAndReturn(func(_ context.Context, group *asoresourcesv1.ResourceGroup) (*asoresourcesv1.ResourceGroup, error) { + group.Spec.Location = ptr.To("location-from-parameters") + return group, nil + }) + specMock.MockASOResourceSpecGetter.EXPECT().WasManaged(gomock.Any()).Return(false) + + specMock.MockPatcher.EXPECT().ExtraPatches().Return([]string{ + `{"metadata": {"labels": {"extra-patch": "not-this-value"}}}`, + `{"metadata": {"labels": {"extra-patch": "this-value"}}}`, + `{"metadata": {"labels": {"another": "label"}}}`, + }) + + ctx := context.Background() + g.Expect(c.Create(ctx, &asoresourcesv1.ResourceGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + Namespace: "namespace", + OwnerReferences: ownerRefs(), + Annotations: map[string]string{ + asoannotations.ReconcilePolicy: string(asoannotations.ReconcilePolicyManage), + }, + }, + Spec: asoresourcesv1.ResourceGroup_Spec{ + Location: ptr.To("location"), + }, + Status: asoresourcesv1.ResourceGroup_STATUS{ + Conditions: []conditions.Condition{ + { + Type: conditions.ConditionTypeReady, + Status: metav1.ConditionTrue, + }, + }, + }, + })).To(Succeed()) + + result, err := s.CreateOrUpdateResource(ctx, specMock, "service") + g.Expect(result).To(BeNil()) + g.Expect(azure.IsOperationNotDoneError(err)).To(BeTrue(), "expected not done error, got %v", err) + + updated := &asoresourcesv1.ResourceGroup{} + g.Expect(c.Get(ctx, types.NamespacedName{Name: "name", Namespace: "namespace"}, updated)).To(Succeed()) + g.Expect(updated.Labels).To(HaveKeyWithValue("extra-patch", "this-value")) + g.Expect(updated.Labels).To(HaveKeyWithValue("another", "label")) + g.Expect(*updated.Spec.Location).To(Equal("location-from-parameters")) + }) } // TestDeleteResource tests the DeleteResource function. diff --git a/azure/services/aso/interfaces.go b/azure/services/aso/interfaces.go index 171ba0fe328b..0acbafac75b7 100644 --- a/azure/services/aso/interfaces.go +++ b/azure/services/aso/interfaces.go @@ -39,6 +39,11 @@ type TagsGetterSetter[T genruntime.MetaObject] interface { SetTags(resource T, tags infrav1.Tags) } +// Patcher supplies extra patches to be applied to an ASO resource. +type Patcher interface { + ExtraPatches() []string +} + // Scope represents the common functionality related to all scopes needed for ASO services. type Scope interface { azure.AsyncStatusUpdater diff --git a/azure/services/aso/mock_aso/aso_mock.go b/azure/services/aso/mock_aso/aso_mock.go index 844e011f8f94..361371ca577b 100644 --- a/azure/services/aso/mock_aso/aso_mock.go +++ b/azure/services/aso/mock_aso/aso_mock.go @@ -167,6 +167,43 @@ func (mr *MockTagsGetterSetterMockRecorder[T]) SetTags(resource, tags any) *gomo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTags", reflect.TypeOf((*MockTagsGetterSetter[T])(nil).SetTags), resource, tags) } +// MockPatcher is a mock of Patcher interface. +type MockPatcher struct { + ctrl *gomock.Controller + recorder *MockPatcherMockRecorder +} + +// MockPatcherMockRecorder is the mock recorder for MockPatcher. +type MockPatcherMockRecorder struct { + mock *MockPatcher +} + +// NewMockPatcher creates a new mock instance. +func NewMockPatcher(ctrl *gomock.Controller) *MockPatcher { + mock := &MockPatcher{ctrl: ctrl} + mock.recorder = &MockPatcherMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockPatcher) EXPECT() *MockPatcherMockRecorder { + return m.recorder +} + +// ExtraPatches mocks base method. +func (m *MockPatcher) ExtraPatches() []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ExtraPatches") + ret0, _ := ret[0].([]string) + return ret0 +} + +// ExtraPatches indicates an expected call of ExtraPatches. +func (mr *MockPatcherMockRecorder) ExtraPatches() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ExtraPatches", reflect.TypeOf((*MockPatcher)(nil).ExtraPatches)) +} + // MockScope is a mock of Scope interface. type MockScope struct { ctrl *gomock.Controller diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go index 280126cc06cc..c1473df307c6 100644 --- a/azure/services/managedclusters/spec.go +++ b/azure/services/managedclusters/spec.go @@ -26,6 +26,7 @@ import ( "github.com/Azure/azure-service-operator/v2/pkg/genruntime" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" @@ -137,6 +138,9 @@ type ManagedClusterSpec struct { // SecurityProfile defines the security profile for the cluster. SecurityProfile *ManagedClusterSecurityProfile + + // Patches are extra patches to be applied to the ASO resource. + Patches []string } // ManagedClusterAutoUpgradeProfile auto upgrade profile for a managed cluster. @@ -646,8 +650,12 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing *asocontai return nil, errors.Wrapf(err, "failed to get agent pool specs for managed cluster %s", s.Name) } + scheme := runtime.NewScheme() + if err := asocontainerservicev1.AddToScheme(scheme); err != nil { + return nil, errors.Wrap(err, "error constructing scheme") + } for _, agentPoolSpec := range agentPoolSpecs { - agentPool, err := agentPoolSpec.Parameters(ctx, nil) + agentPool, err := aso.PatchedParameters(ctx, scheme, agentPoolSpec, nil) if err != nil { return nil, errors.Wrapf(err, "failed to get agent pool parameters for managed cluster %s", s.Name) } @@ -750,3 +758,10 @@ func (*ManagedClusterSpec) GetDesiredTags(resource *asocontainerservicev1.Manage func (*ManagedClusterSpec) SetTags(resource *asocontainerservicev1.ManagedCluster, tags infrav1.Tags) { resource.Spec.Tags = tags } + +var _ aso.Patcher = (*ManagedClusterSpec)(nil) + +// ExtraPatches implements aso.Patcher. +func (s *ManagedClusterSpec) ExtraPatches() []string { + return s.Patches +} diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml index 7f427bc2c96c..151f96c7c607 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml @@ -133,6 +133,16 @@ spec: - None type: string type: object + asoManagedClusterPatches: + description: 'ASOManagedClusterPatches defines JSON merge patches + to be applied to the generated ASO ManagedCluster resource. WARNING: + This is meant to be used sparingly to enable features for development + and testing that are not otherwise represented in the CAPZ API. + Misconfiguration that conflicts with CAPZ''s normal mode of operation + is possible.' + items: + type: string + type: array autoUpgradeProfile: description: AutoUpgradeProfile defines the auto upgrade configuration. properties: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanetemplates.yaml index bafd16a00985..91ac6f4f5355 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanetemplates.yaml @@ -124,6 +124,17 @@ spec: - None type: string type: object + asoManagedClusterPatches: + description: 'ASOManagedClusterPatches defines JSON merge + patches to be applied to the generated ASO ManagedCluster + resource. WARNING: This is meant to be used sparingly to + enable features for development and testing that are not + otherwise represented in the CAPZ API. Misconfiguration + that conflicts with CAPZ''s normal mode of operation is + possible.' + items: + type: string + type: array autoUpgradeProfile: description: AutoUpgradeProfile defines the auto upgrade configuration. properties: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml index 78de21112bcd..6b06f6933b7a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml @@ -72,6 +72,16 @@ spec: resources managed by the Azure provider, in addition to the ones added by default. type: object + asoManagedClustersAgentPoolPatches: + description: 'ASOManagedClustersAgentPoolPatches defines JSON merge + patches to be applied to the generated ASO ManagedClustersAgentPool + resource. WARNING: This is meant to be used sparingly to enable + features for development and testing that are not otherwise represented + in the CAPZ API. Misconfiguration that conflicts with CAPZ''s normal + mode of operation is possible.' + items: + type: string + type: array availabilityZones: description: AvailabilityZones - Availability zones for nodes. Must use VirtualMachineScaleSets AgentPoolType. Immutable. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepooltemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepooltemplates.yaml index 9fa9eaf819e8..ec550358d2a0 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepooltemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepooltemplates.yaml @@ -55,6 +55,17 @@ spec: add to Azure resources managed by the Azure provider, in addition to the ones added by default. type: object + asoManagedClustersAgentPoolPatches: + description: 'ASOManagedClustersAgentPoolPatches defines JSON + merge patches to be applied to the generated ASO ManagedClustersAgentPool + resource. WARNING: This is meant to be used sparingly to + enable features for development and testing that are not + otherwise represented in the CAPZ API. Misconfiguration + that conflicts with CAPZ''s normal mode of operation is + possible.' + items: + type: string + type: array availabilityZones: description: AvailabilityZones - Availability zones for nodes. Must use VirtualMachineScaleSets AgentPoolType. Immutable. diff --git a/go.mod b/go.mod index 3d0060f3f7eb..cb67a926faa5 100644 --- a/go.mod +++ b/go.mod @@ -103,7 +103,7 @@ require ( github.com/drone/envsubst/v2 v2.0.0-20210730161058-179042472c46 // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect github.com/evanphx/json-patch v5.7.0+incompatible // indirect - github.com/evanphx/json-patch/v5 v5.7.0 // indirect + github.com/evanphx/json-patch/v5 v5.7.0 github.com/fatih/camelcase v1.0.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect diff --git a/test/e2e/aks_patches.go b/test/e2e/aks_patches.go new file mode 100644 index 000000000000..ed2b00fd2c51 --- /dev/null +++ b/test/e2e/aks_patches.go @@ -0,0 +1,192 @@ +//go:build e2e +// +build e2e + +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "context" + "sync" + + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type AKSPatchSpecInput struct { + Cluster *clusterv1.Cluster + MachinePools []*expv1.MachinePool + WaitForUpdate []interface{} +} + +func AKSPatchSpec(ctx context.Context, inputGetter func() AKSPatchSpecInput) { + input := inputGetter() + + cred, err := azidentity.NewDefaultAzureCredential(nil) + Expect(err).NotTo(HaveOccurred()) + + managedclustersClient, err := armcontainerservice.NewManagedClustersClient(getSubscriptionID(Default), cred, nil) + Expect(err).NotTo(HaveOccurred()) + + agentpoolsClient, err := armcontainerservice.NewAgentPoolsClient(getSubscriptionID(Default), cred, nil) + Expect(err).NotTo(HaveOccurred()) + + mgmtClient := bootstrapClusterProxy.GetClient() + Expect(mgmtClient).NotTo(BeNil()) + + infraControlPlane := &infrav1.AzureManagedControlPlane{} + err = mgmtClient.Get(ctx, client.ObjectKey{ + Namespace: input.Cluster.Spec.ControlPlaneRef.Namespace, + Name: input.Cluster.Spec.ControlPlaneRef.Name, + }, infraControlPlane) + Expect(err).NotTo(HaveOccurred()) + + var wg sync.WaitGroup + + wg.Add(1) + go func() { + defer GinkgoRecover() + defer wg.Done() + + checkTags := func(exist map[string]string) func(Gomega) { + return func(g Gomega) { + resp, err := managedclustersClient.Get(ctx, infraControlPlane.Spec.ResourceGroupName, infraControlPlane.Name, nil) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(resp.Properties.ProvisioningState).To(Equal(ptr.To("Succeeded"))) + for k, v := range exist { + g.Expect(resp.ManagedCluster.Tags).To(HaveKeyWithValue(k, ptr.To(v))) + } + } + } + + var initialPatches []string + By("Deleting patches for control plane") + Eventually(func(g Gomega) { + g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(infraControlPlane), infraControlPlane)).To(Succeed()) + initialPatches = infraControlPlane.Spec.ASOManagedClusterPatches + infraControlPlane.Spec.ASOManagedClusterPatches = nil + g.Expect(mgmtClient.Update(ctx, infraControlPlane)).To(Succeed()) + }, inputGetter().WaitForUpdate...).Should(Succeed()) + Eventually(checkTags(nil), input.WaitForUpdate...).Should(Succeed()) + + By("Creating patches for control plane") + Eventually(func(g Gomega) { + g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(infraControlPlane), infraControlPlane)).To(Succeed()) + infraControlPlane.Spec.ASOManagedClusterPatches = []string{ + `{"spec": {"tags": {"capzpatchtest": "value"}}}`, + } + g.Expect(mgmtClient.Update(ctx, infraControlPlane)).To(Succeed()) + }, inputGetter().WaitForUpdate...).Should(Succeed()) + Eventually(checkTags(map[string]string{"capzpatchtest": "value"}), input.WaitForUpdate...).Should(Succeed()) + + By("Updating patches for control plane") + Eventually(func(g Gomega) { + g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(infraControlPlane), infraControlPlane)).To(Succeed()) + infraControlPlane.Spec.ASOManagedClusterPatches = append(infraControlPlane.Spec.ASOManagedClusterPatches, `{"spec": {"tags": {"capzpatchtest": "updated"}}}`) + g.Expect(mgmtClient.Update(ctx, infraControlPlane)).To(Succeed()) + }, inputGetter().WaitForUpdate...).Should(Succeed()) + Eventually(checkTags(map[string]string{"capzpatchtest": "updated"}), input.WaitForUpdate...).Should(Succeed()) + + By("Restoring initial patches for control plane") + Eventually(func(g Gomega) { + g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(infraControlPlane), infraControlPlane)).To(Succeed()) + infraControlPlane.Spec.ASOManagedClusterPatches = initialPatches + g.Expect(mgmtClient.Update(ctx, infraControlPlane)).To(Succeed()) + }, inputGetter().WaitForUpdate...).Should(Succeed()) + Eventually(checkTags(nil), input.WaitForUpdate...).Should(Succeed()) + }() + + for _, mp := range input.MachinePools { + wg.Add(1) + go func(mp *expv1.MachinePool) { + defer GinkgoRecover() + defer wg.Done() + + ammp := &infrav1.AzureManagedMachinePool{} + Expect(mgmtClient.Get(ctx, types.NamespacedName{ + Namespace: mp.Spec.Template.Spec.InfrastructureRef.Namespace, + Name: mp.Spec.Template.Spec.InfrastructureRef.Name, + }, ammp)).To(Succeed()) + + nonAdditionalTagKeys := map[string]struct{}{} + resp, err := agentpoolsClient.Get(ctx, infraControlPlane.Spec.ResourceGroupName, infraControlPlane.Name, *ammp.Spec.Name, nil) + Expect(err).NotTo(HaveOccurred()) + for k := range resp.AgentPool.Properties.Tags { + if _, exists := infraControlPlane.Spec.AdditionalTags[k]; !exists { + nonAdditionalTagKeys[k] = struct{}{} + } + } + + checkTags := func(exist map[string]string) func(Gomega) { + return func(g Gomega) { + resp, err := agentpoolsClient.Get(ctx, infraControlPlane.Spec.ResourceGroupName, infraControlPlane.Name, *ammp.Spec.Name, nil) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(resp.Properties.ProvisioningState).To(Equal(ptr.To("Succeeded"))) + for k, v := range exist { + g.Expect(resp.AgentPool.Properties.Tags).To(HaveKeyWithValue(k, ptr.To(v))) + } + } + } + + var initialPatches []string + Byf("Deleting all patches for machine pool %s", mp.Name) + Eventually(func(g Gomega) { + g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(ammp), ammp)).To(Succeed()) + initialPatches = ammp.Spec.ASOManagedClustersAgentPoolPatches + ammp.Spec.ASOManagedClustersAgentPoolPatches = nil + g.Expect(mgmtClient.Update(ctx, ammp)).To(Succeed()) + }, inputGetter().WaitForUpdate...).Should(Succeed()) + Eventually(checkTags(nil), input.WaitForUpdate...).Should(Succeed()) + + Byf("Creating patches for machine pool %s", mp.Name) + Eventually(func(g Gomega) { + g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(ammp), ammp)).To(Succeed()) + ammp.Spec.ASOManagedClustersAgentPoolPatches = []string{ + `{"spec": {"tags": {"capzpatchtest": "value"}}}`, + } + g.Expect(mgmtClient.Update(ctx, ammp)).To(Succeed()) + }, inputGetter().WaitForUpdate...).Should(Succeed()) + Eventually(checkTags(map[string]string{"capzpatchtest": "value"}), input.WaitForUpdate...).Should(Succeed()) + + Byf("Updating patches for machine pool %s", mp.Name) + Eventually(func(g Gomega) { + g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(ammp), ammp)).To(Succeed()) + ammp.Spec.ASOManagedClustersAgentPoolPatches = append(ammp.Spec.ASOManagedClustersAgentPoolPatches, `{"spec": {"tags": {"capzpatchtest": "updated"}}}`) + g.Expect(mgmtClient.Update(ctx, ammp)).To(Succeed()) + }, inputGetter().WaitForUpdate...).Should(Succeed()) + Eventually(checkTags(map[string]string{"capzpatchtest": "updated"}), input.WaitForUpdate...).Should(Succeed()) + + Byf("Restoring initial patches for machine pool %s", mp.Name) + Eventually(func(g Gomega) { + g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(ammp), ammp)).To(Succeed()) + ammp.Spec.ASOManagedClustersAgentPoolPatches = initialPatches + g.Expect(mgmtClient.Update(ctx, ammp)).To(Succeed()) + }, inputGetter().WaitForUpdate...).Should(Succeed()) + Eventually(checkTags(nil), input.WaitForUpdate...).Should(Succeed()) + }(mp) + } + + wg.Wait() +} diff --git a/test/e2e/azure_test.go b/test/e2e/azure_test.go index b0d6abce0a06..f7c2e472231b 100644 --- a/test/e2e/azure_test.go +++ b/test/e2e/azure_test.go @@ -825,6 +825,16 @@ var _ = Describe("Workload cluster creation", func() { } }) }) + + By("modifying custom patches", func() { + AKSPatchSpec(ctx, func() AKSPatchSpecInput { + return AKSPatchSpecInput{ + Cluster: result.Cluster, + MachinePools: result.MachinePools, + WaitForUpdate: e2eConfig.GetIntervals(specName, "wait-machine-pool-nodes"), + } + }) + }) }) })