Skip to content

Commit a3384c1

Browse files
jackfrancisjackfrancis
authored
Merge pull request #4810 from nojnhuh/v2-reaper
ASOAPI: delete resources removed from spec
2 parents 0c64bea + 5503a41 commit a3384c1

File tree

5 files changed

+183
-49
lines changed

5 files changed

+183
-49
lines changed

‎exp/api/v1alpha1/azureasomanagedcluster_types.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,12 @@ type AzureASOManagedCluster struct {
7878
Status AzureASOManagedClusterStatus `json:"status,omitempty"`
7979
}
8080

81-
// SetResourceStatuses returns the status of resources.
81+
// GetResourceStatuses returns the status of resources.
82+
func (a *AzureASOManagedCluster) GetResourceStatuses() []ResourceStatus {
83+
return a.Status.Resources
84+
}
85+
86+
// SetResourceStatuses sets the status of resources.
8287
func (a *AzureASOManagedCluster) SetResourceStatuses(r []ResourceStatus) {
8388
a.Status.Resources = r
8489
}

‎exp/api/v1alpha1/azureasomanagedcontrolplane_types.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,12 @@ type AzureASOManagedControlPlane struct {
6666
Status AzureASOManagedControlPlaneStatus `json:"status,omitempty"`
6767
}
6868

69-
// SetResourceStatuses returns the status of resources.
69+
// GetResourceStatuses returns the status of resources.
70+
func (a *AzureASOManagedControlPlane) GetResourceStatuses() []ResourceStatus {
71+
return a.Status.Resources
72+
}
73+
74+
// SetResourceStatuses sets the status of resources.
7075
func (a *AzureASOManagedControlPlane) SetResourceStatuses(r []ResourceStatus) {
7176
a.Status.Resources = r
7277
}

‎exp/api/v1alpha1/azureasomanagedmachinepool_types.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,12 @@ type AzureASOManagedMachinePool struct {
6161
Status AzureASOManagedMachinePoolStatus `json:"status,omitempty"`
6262
}
6363

64-
// SetResourceStatuses returns the status of resources.
64+
// GetResourceStatuses returns the status of resources.
65+
func (a *AzureASOManagedMachinePool) GetResourceStatuses() []ResourceStatus {
66+
return a.Status.Resources
67+
}
68+
69+
// SetResourceStatuses sets the status of resources.
6570
func (a *AzureASOManagedMachinePool) SetResourceStatuses(r []ResourceStatus) {
6671
a.Status.Resources = r
6772
}

‎exp/controllers/resource_reconciler.go‎

Lines changed: 69 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2828
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2929
"k8s.io/apimachinery/pkg/runtime"
30+
"k8s.io/apimachinery/pkg/runtime/schema"
3031
"k8s.io/klog/v2"
3132
infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha1"
3233
"sigs.k8s.io/cluster-api-provider-azure/exp/mutators"
@@ -52,6 +53,7 @@ type watcher interface {
5253

5354
type resourceStatusObject interface {
5455
client.Object
56+
GetResourceStatuses() []infrav1exp.ResourceStatus
5557
SetResourceStatuses([]infrav1exp.ResourceStatus)
5658
}
5759

@@ -98,6 +100,28 @@ func (r *ResourceReconciler) Reconcile(ctx context.Context) error {
98100
})
99101
}
100102

103+
for _, oldStatus := range r.owner.GetResourceStatuses() {
104+
needsDelete := true
105+
for _, newStatus := range newResourceStatuses {
106+
if oldStatus.Resource.Group == newStatus.Resource.Group &&
107+
oldStatus.Resource.Kind == newStatus.Resource.Kind &&
108+
oldStatus.Resource.Name == newStatus.Resource.Name {
109+
needsDelete = false
110+
break
111+
}
112+
}
113+
114+
if needsDelete {
115+
updatedStatus, err := r.deleteResource(ctx, oldStatus.Resource)
116+
if err != nil {
117+
return err
118+
}
119+
if updatedStatus != nil {
120+
newResourceStatuses = append(newResourceStatuses, *updatedStatus)
121+
}
122+
}
123+
}
124+
101125
r.owner.SetResourceStatuses(newResourceStatuses)
102126

103127
return nil
@@ -141,50 +165,61 @@ func (r *ResourceReconciler) Delete(ctx context.Context) error {
141165

142166
var newResourceStatuses []infrav1exp.ResourceStatus
143167

144-
for _, spec := range r.resources {
145-
spec.SetNamespace(r.owner.GetNamespace())
146-
gvk := spec.GroupVersionKind()
147-
148-
log := log.WithValues("resource", klog.KObj(spec), "resourceVersion", gvk.GroupVersion(), "resourceKind", gvk.Kind)
149-
150-
log.V(4).Info("deleting resource")
151-
err := r.Client.Delete(ctx, spec)
152-
if apierrors.IsNotFound(err) {
153-
log.V(4).Info("resource has been deleted")
154-
continue
155-
}
168+
for _, spec := range r.owner.GetResourceStatuses() {
169+
newStatus, err := r.deleteResource(ctx, spec.Resource)
156170
if err != nil {
157-
return fmt.Errorf("failed to delete resource: %w", err)
158-
}
159-
160-
err = r.Client.Get(ctx, client.ObjectKeyFromObject(spec), spec)
161-
if apierrors.IsNotFound(err) {
162-
log.V(4).Info("resource has been deleted")
163-
continue
171+
return err
164172
}
165-
if err != nil {
166-
return fmt.Errorf("failed to get resource: %w", err)
173+
if newStatus != nil {
174+
newResourceStatuses = append(newResourceStatuses, *newStatus)
167175
}
168-
ready, err := readyStatus(ctx, spec)
169-
if err != nil {
170-
return fmt.Errorf("failed to get ready status: %w", err)
171-
}
172-
newResourceStatuses = append(newResourceStatuses, infrav1exp.ResourceStatus{
173-
Resource: infrav1exp.StatusResource{
174-
Group: gvk.Group,
175-
Version: gvk.Version,
176-
Kind: gvk.Kind,
177-
Name: spec.GetName(),
178-
},
179-
Ready: ready,
180-
})
181176
}
182177

183178
r.owner.SetResourceStatuses(newResourceStatuses)
184179

185180
return nil
186181
}
187182

183+
func (r *ResourceReconciler) deleteResource(ctx context.Context, resource infrav1exp.StatusResource) (*infrav1exp.ResourceStatus, error) {
184+
ctx, log, done := tele.StartSpanWithLogger(ctx, "controllers.ResourceReconciler.deleteResource")
185+
defer done()
186+
187+
spec := &unstructured.Unstructured{}
188+
spec.SetGroupVersionKind(schema.GroupVersionKind{Group: resource.Group, Version: resource.Version, Kind: resource.Kind})
189+
spec.SetNamespace(r.owner.GetNamespace())
190+
spec.SetName(resource.Name)
191+
192+
log = log.WithValues("resource", klog.KObj(spec), "resourceVersion", spec.GroupVersionKind().GroupVersion(), "resourceKind", spec.GetKind())
193+
194+
log.V(4).Info("deleting resource")
195+
err := r.Client.Delete(ctx, spec)
196+
if apierrors.IsNotFound(err) {
197+
log.V(4).Info("resource has been deleted")
198+
return nil, nil
199+
}
200+
if err != nil {
201+
return nil, fmt.Errorf("failed to delete resource: %w", err)
202+
}
203+
204+
err = r.Client.Get(ctx, client.ObjectKeyFromObject(spec), spec)
205+
if apierrors.IsNotFound(err) {
206+
log.V(4).Info("resource has been deleted")
207+
return nil, nil
208+
}
209+
if err != nil {
210+
return nil, fmt.Errorf("failed to get resource: %w", err)
211+
}
212+
ready, err := readyStatus(ctx, spec)
213+
if err != nil {
214+
return nil, fmt.Errorf("failed to get ready status: %w", err)
215+
}
216+
217+
return &infrav1exp.ResourceStatus{
218+
Resource: resource,
219+
Ready: ready,
220+
}, nil
221+
}
222+
188223
func readyStatus(ctx context.Context, u *unstructured.Unstructured) (bool, error) {
189224
_, log, done := tele.StartSpanWithLogger(ctx, "controllers.ResourceReconciler.readyStatus")
190225
defer done()

‎exp/controllers/resource_reconciler_test.go‎

Lines changed: 96 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,85 @@ func TestResourceReconcilerReconcile(t *testing.T) {
148148
g.Expect(resourcesStatuses[1].Resource.Name).To(Equal("rg2"))
149149
g.Expect(resourcesStatuses[1].Ready).To(BeFalse())
150150
})
151+
152+
t.Run("delete stale resources", func(t *testing.T) {
153+
g := NewGomegaWithT(t)
154+
155+
owner := &infrav1exp.AzureASOManagedCluster{
156+
Status: infrav1exp.AzureASOManagedClusterStatus{
157+
Resources: []infrav1exp.ResourceStatus{
158+
rgStatus("rg0"),
159+
rgStatus("rg1"),
160+
rgStatus("rg2"),
161+
rgStatus("rg3"),
162+
},
163+
},
164+
}
165+
166+
objs := []client.Object{
167+
&asoresourcesv1.ResourceGroup{
168+
ObjectMeta: metav1.ObjectMeta{
169+
Name: "rg0",
170+
Namespace: owner.Namespace,
171+
},
172+
},
173+
&asoresourcesv1.ResourceGroup{
174+
ObjectMeta: metav1.ObjectMeta{
175+
Name: "rg1",
176+
Namespace: owner.Namespace,
177+
},
178+
},
179+
&asoresourcesv1.ResourceGroup{
180+
ObjectMeta: metav1.ObjectMeta{
181+
Name: "rg2",
182+
Namespace: owner.Namespace,
183+
},
184+
},
185+
&asoresourcesv1.ResourceGroup{
186+
ObjectMeta: metav1.ObjectMeta{
187+
Name: "rg3",
188+
Namespace: owner.Namespace,
189+
Finalizers: []string{"still deleting"},
190+
},
191+
},
192+
}
193+
194+
c := fakeClientBuilder().
195+
WithObjects(objs...).
196+
Build()
197+
198+
r := &ResourceReconciler{
199+
Client: &FakeClient{
200+
Client: c,
201+
patchFunc: func(ctx context.Context, o client.Object, p client.Patch, po ...client.PatchOption) error {
202+
return nil
203+
},
204+
},
205+
resources: []*unstructured.Unstructured{
206+
rgJSON(g, s, &asoresourcesv1.ResourceGroup{
207+
ObjectMeta: metav1.ObjectMeta{
208+
Name: "rg1",
209+
},
210+
}),
211+
rgJSON(g, s, &asoresourcesv1.ResourceGroup{
212+
ObjectMeta: metav1.ObjectMeta{
213+
Name: "rg2",
214+
},
215+
}),
216+
},
217+
owner: owner,
218+
watcher: &FakeWatcher{},
219+
}
220+
221+
g.Expect(r.Reconcile(ctx)).To(Succeed())
222+
223+
resourcesStatuses := owner.Status.Resources
224+
g.Expect(resourcesStatuses).To(HaveLen(3))
225+
// rg0 should be deleted and gone
226+
g.Expect(resourcesStatuses[0].Resource.Name).To(Equal("rg1"))
227+
g.Expect(resourcesStatuses[1].Resource.Name).To(Equal("rg2"))
228+
g.Expect(resourcesStatuses[2].Resource.Name).To(Equal("rg3"))
229+
})
151230
}
152231

153232
func TestResourceReconcilerPause(t *testing.T) {
@@ -249,6 +328,12 @@ func TestResourceReconcilerDelete(t *testing.T) {
249328
ObjectMeta: metav1.ObjectMeta{
250329
Namespace: "ns",
251330
},
331+
Status: infrav1exp.AzureASOManagedClusterStatus{
332+
Resources: []infrav1exp.ResourceStatus{
333+
rgStatus("still-deleting"),
334+
rgStatus("already-gone"),
335+
},
336+
},
252337
}
253338

254339
objs := []client.Object{
@@ -271,18 +356,6 @@ func TestResourceReconcilerDelete(t *testing.T) {
271356
Client: &FakeClient{
272357
Client: c,
273358
},
274-
resources: []*unstructured.Unstructured{
275-
rgJSON(g, s, &asoresourcesv1.ResourceGroup{
276-
ObjectMeta: metav1.ObjectMeta{
277-
Name: "still-deleting",
278-
},
279-
}),
280-
rgJSON(g, s, &asoresourcesv1.ResourceGroup{
281-
ObjectMeta: metav1.ObjectMeta{
282-
Name: "already-gone",
283-
},
284-
}),
285-
},
286359
owner: owner,
287360
}
288361

@@ -507,3 +580,14 @@ func rgJSON(g Gomega, scheme *runtime.Scheme, rg *asoresourcesv1.ResourceGroup)
507580
g.Expect(scheme.Convert(rg, u, nil)).To(Succeed())
508581
return u
509582
}
583+
584+
func rgStatus(name string) infrav1exp.ResourceStatus {
585+
return infrav1exp.ResourceStatus{
586+
Resource: infrav1exp.StatusResource{
587+
Group: asoresourcesv1.GroupVersion.Group,
588+
Version: asoresourcesv1.GroupVersion.Version,
589+
Kind: "ResourceGroup",
590+
Name: name,
591+
},
592+
}
593+
}

0 commit comments

Comments
 (0)