ASOAPI: delete resources removed from spec

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
 }

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
 }

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
 }

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,50 +165,61 @@ 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)
 
 	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()

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,
+		},
+	}
+}