diff --git a/exp/api/v1alpha1/azureasomanagedcluster_types.go b/exp/api/v1alpha1/azureasomanagedcluster_types.go index 77cec7f9620..026f4237a85 100644 --- a/exp/api/v1alpha1/azureasomanagedcluster_types.go +++ b/exp/api/v1alpha1/azureasomanagedcluster_types.go @@ -78,7 +78,12 @@ type AzureASOManagedCluster struct { Status AzureASOManagedClusterStatus `json:"status,omitempty"` } -// SetResourceStatuses returns the status of resources. +// GetResourceStatuses returns the status of resources. +func (a *AzureASOManagedCluster) GetResourceStatuses() []ResourceStatus { + return a.Status.Resources +} + +// SetResourceStatuses sets the status of resources. func (a *AzureASOManagedCluster) SetResourceStatuses(r []ResourceStatus) { a.Status.Resources = r } diff --git a/exp/api/v1alpha1/azureasomanagedcontrolplane_types.go b/exp/api/v1alpha1/azureasomanagedcontrolplane_types.go index 4122a924c88..7ea30afa1e9 100644 --- a/exp/api/v1alpha1/azureasomanagedcontrolplane_types.go +++ b/exp/api/v1alpha1/azureasomanagedcontrolplane_types.go @@ -66,7 +66,12 @@ type AzureASOManagedControlPlane struct { Status AzureASOManagedControlPlaneStatus `json:"status,omitempty"` } -// SetResourceStatuses returns the status of resources. +// GetResourceStatuses returns the status of resources. +func (a *AzureASOManagedControlPlane) GetResourceStatuses() []ResourceStatus { + return a.Status.Resources +} + +// SetResourceStatuses sets the status of resources. func (a *AzureASOManagedControlPlane) SetResourceStatuses(r []ResourceStatus) { a.Status.Resources = r } diff --git a/exp/api/v1alpha1/azureasomanagedmachinepool_types.go b/exp/api/v1alpha1/azureasomanagedmachinepool_types.go index 240a66eec34..f082e12da07 100644 --- a/exp/api/v1alpha1/azureasomanagedmachinepool_types.go +++ b/exp/api/v1alpha1/azureasomanagedmachinepool_types.go @@ -61,7 +61,12 @@ type AzureASOManagedMachinePool struct { Status AzureASOManagedMachinePoolStatus `json:"status,omitempty"` } -// SetResourceStatuses returns the status of resources. +// GetResourceStatuses returns the status of resources. +func (a *AzureASOManagedMachinePool) GetResourceStatuses() []ResourceStatus { + return a.Status.Resources +} + +// SetResourceStatuses sets the status of resources. func (a *AzureASOManagedMachinePool) SetResourceStatuses(r []ResourceStatus) { a.Status.Resources = r } diff --git a/exp/controllers/resource_reconciler.go b/exp/controllers/resource_reconciler.go index bc4e4b46038..3788723e2c3 100644 --- a/exp/controllers/resource_reconciler.go +++ b/exp/controllers/resource_reconciler.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/klog/v2" infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha1" "sigs.k8s.io/cluster-api-provider-azure/exp/mutators" @@ -52,6 +53,7 @@ type watcher interface { type resourceStatusObject interface { client.Object + GetResourceStatuses() []infrav1exp.ResourceStatus SetResourceStatuses([]infrav1exp.ResourceStatus) } @@ -98,6 +100,28 @@ func (r *ResourceReconciler) Reconcile(ctx context.Context) error { }) } + for _, oldStatus := range r.owner.GetResourceStatuses() { + needsDelete := true + for _, newStatus := range newResourceStatuses { + if oldStatus.Resource.Group == newStatus.Resource.Group && + oldStatus.Resource.Kind == newStatus.Resource.Kind && + oldStatus.Resource.Name == newStatus.Resource.Name { + needsDelete = false + break + } + } + + if needsDelete { + updatedStatus, err := r.deleteResource(ctx, oldStatus.Resource) + if err != nil { + return err + } + if updatedStatus != nil { + newResourceStatuses = append(newResourceStatuses, *updatedStatus) + } + } + } + r.owner.SetResourceStatuses(newResourceStatuses) return nil @@ -141,43 +165,14 @@ func (r *ResourceReconciler) Delete(ctx context.Context) error { var newResourceStatuses []infrav1exp.ResourceStatus - for _, spec := range r.resources { - spec.SetNamespace(r.owner.GetNamespace()) - gvk := spec.GroupVersionKind() - - log := log.WithValues("resource", klog.KObj(spec), "resourceVersion", gvk.GroupVersion(), "resourceKind", gvk.Kind) - - log.V(4).Info("deleting resource") - err := r.Client.Delete(ctx, spec) - if apierrors.IsNotFound(err) { - log.V(4).Info("resource has been deleted") - continue - } + for _, spec := range r.owner.GetResourceStatuses() { + newStatus, err := r.deleteResource(ctx, spec.Resource) if err != nil { - return fmt.Errorf("failed to delete resource: %w", err) - } - - err = r.Client.Get(ctx, client.ObjectKeyFromObject(spec), spec) - if apierrors.IsNotFound(err) { - log.V(4).Info("resource has been deleted") - continue + return err } - if err != nil { - return fmt.Errorf("failed to get resource: %w", err) + if newStatus != nil { + newResourceStatuses = append(newResourceStatuses, *newStatus) } - ready, err := readyStatus(ctx, spec) - if err != nil { - return fmt.Errorf("failed to get ready status: %w", err) - } - newResourceStatuses = append(newResourceStatuses, infrav1exp.ResourceStatus{ - Resource: infrav1exp.StatusResource{ - Group: gvk.Group, - Version: gvk.Version, - Kind: gvk.Kind, - Name: spec.GetName(), - }, - Ready: ready, - }) } r.owner.SetResourceStatuses(newResourceStatuses) @@ -185,6 +180,46 @@ func (r *ResourceReconciler) Delete(ctx context.Context) error { return nil } +func (r *ResourceReconciler) deleteResource(ctx context.Context, resource infrav1exp.StatusResource) (*infrav1exp.ResourceStatus, error) { + ctx, log, done := tele.StartSpanWithLogger(ctx, "controllers.ResourceReconciler.deleteResource") + defer done() + + spec := &unstructured.Unstructured{} + spec.SetGroupVersionKind(schema.GroupVersionKind{Group: resource.Group, Version: resource.Version, Kind: resource.Kind}) + spec.SetNamespace(r.owner.GetNamespace()) + spec.SetName(resource.Name) + + log = log.WithValues("resource", klog.KObj(spec), "resourceVersion", spec.GroupVersionKind().GroupVersion(), "resourceKind", spec.GetKind()) + + log.V(4).Info("deleting resource") + err := r.Client.Delete(ctx, spec) + if apierrors.IsNotFound(err) { + log.V(4).Info("resource has been deleted") + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("failed to delete resource: %w", err) + } + + err = r.Client.Get(ctx, client.ObjectKeyFromObject(spec), spec) + if apierrors.IsNotFound(err) { + log.V(4).Info("resource has been deleted") + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("failed to get resource: %w", err) + } + ready, err := readyStatus(ctx, spec) + if err != nil { + return nil, fmt.Errorf("failed to get ready status: %w", err) + } + + return &infrav1exp.ResourceStatus{ + Resource: resource, + Ready: ready, + }, nil +} + func readyStatus(ctx context.Context, u *unstructured.Unstructured) (bool, error) { _, log, done := tele.StartSpanWithLogger(ctx, "controllers.ResourceReconciler.readyStatus") defer done() diff --git a/exp/controllers/resource_reconciler_test.go b/exp/controllers/resource_reconciler_test.go index 3ed310fd3ca..41aabf4187f 100644 --- a/exp/controllers/resource_reconciler_test.go +++ b/exp/controllers/resource_reconciler_test.go @@ -148,6 +148,85 @@ func TestResourceReconcilerReconcile(t *testing.T) { g.Expect(resourcesStatuses[1].Resource.Name).To(Equal("rg2")) g.Expect(resourcesStatuses[1].Ready).To(BeFalse()) }) + + t.Run("delete stale resources", func(t *testing.T) { + g := NewGomegaWithT(t) + + owner := &infrav1exp.AzureASOManagedCluster{ + Status: infrav1exp.AzureASOManagedClusterStatus{ + Resources: []infrav1exp.ResourceStatus{ + rgStatus("rg0"), + rgStatus("rg1"), + rgStatus("rg2"), + rgStatus("rg3"), + }, + }, + } + + objs := []client.Object{ + &asoresourcesv1.ResourceGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rg0", + Namespace: owner.Namespace, + }, + }, + &asoresourcesv1.ResourceGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rg1", + Namespace: owner.Namespace, + }, + }, + &asoresourcesv1.ResourceGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rg2", + Namespace: owner.Namespace, + }, + }, + &asoresourcesv1.ResourceGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rg3", + Namespace: owner.Namespace, + Finalizers: []string{"still deleting"}, + }, + }, + } + + c := fakeClientBuilder(). + WithObjects(objs...). + Build() + + r := &ResourceReconciler{ + Client: &FakeClient{ + Client: c, + patchFunc: func(ctx context.Context, o client.Object, p client.Patch, po ...client.PatchOption) error { + return nil + }, + }, + resources: []*unstructured.Unstructured{ + rgJSON(g, s, &asoresourcesv1.ResourceGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rg1", + }, + }), + rgJSON(g, s, &asoresourcesv1.ResourceGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rg2", + }, + }), + }, + owner: owner, + watcher: &FakeWatcher{}, + } + + g.Expect(r.Reconcile(ctx)).To(Succeed()) + + resourcesStatuses := owner.Status.Resources + g.Expect(resourcesStatuses).To(HaveLen(3)) + // rg0 should be deleted and gone + g.Expect(resourcesStatuses[0].Resource.Name).To(Equal("rg1")) + g.Expect(resourcesStatuses[1].Resource.Name).To(Equal("rg2")) + g.Expect(resourcesStatuses[2].Resource.Name).To(Equal("rg3")) + }) } func TestResourceReconcilerPause(t *testing.T) { @@ -249,6 +328,12 @@ func TestResourceReconcilerDelete(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Namespace: "ns", }, + Status: infrav1exp.AzureASOManagedClusterStatus{ + Resources: []infrav1exp.ResourceStatus{ + rgStatus("still-deleting"), + rgStatus("already-gone"), + }, + }, } objs := []client.Object{ @@ -271,18 +356,6 @@ func TestResourceReconcilerDelete(t *testing.T) { Client: &FakeClient{ Client: c, }, - resources: []*unstructured.Unstructured{ - rgJSON(g, s, &asoresourcesv1.ResourceGroup{ - ObjectMeta: metav1.ObjectMeta{ - Name: "still-deleting", - }, - }), - rgJSON(g, s, &asoresourcesv1.ResourceGroup{ - ObjectMeta: metav1.ObjectMeta{ - Name: "already-gone", - }, - }), - }, owner: owner, } @@ -507,3 +580,14 @@ func rgJSON(g Gomega, scheme *runtime.Scheme, rg *asoresourcesv1.ResourceGroup) g.Expect(scheme.Convert(rg, u, nil)).To(Succeed()) return u } + +func rgStatus(name string) infrav1exp.ResourceStatus { + return infrav1exp.ResourceStatus{ + Resource: infrav1exp.StatusResource{ + Group: asoresourcesv1.GroupVersion.Group, + Version: asoresourcesv1.GroupVersion.Version, + Kind: "ResourceGroup", + Name: name, + }, + } +}