Skip to content

Commit

Permalink
Add composition cleanup controller
Browse files Browse the repository at this point in the history
  • Loading branch information
Jordan Olshevski committed Jan 5, 2024
1 parent 4586079 commit d53a4fd
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 9 deletions.
64 changes: 64 additions & 0 deletions internal/controllers/cleanup/composition.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package cleanup

import (
"context"
"fmt"

apiv1 "github.com/Azure/eno/api/v1"
"github.com/Azure/eno/internal/manager"
"github.com/go-logr/logr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

// TODO: Should we limit the max number of resource slices per composition to protect etcd in the face of partial resource slice write failures?

// TODO: Add finalizer on composition to block until all resource slices owned by the composition are fully deleted (reuse builtin?)

type compositionController struct {
client client.Client
}

func NewCompositionController(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&apiv1.ResourceSlice{}).
Watches(&apiv1.Composition{}, manager.NewCompositionToResourceSliceHandler(mgr.GetClient())).
WithLogConstructor(manager.NewLogConstructor(mgr, "compositionCleanupController")).
Complete(&compositionController{
client: mgr.GetClient(),
})
}

func (c *compositionController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := logr.FromContextOrDiscard(ctx).WithValues("compositionName", req.Name, "compositionNamespace", req.Namespace)

comp := &apiv1.Composition{}
err := c.client.Get(ctx, req.NamespacedName, comp)
if err != nil {
return ctrl.Result{}, client.IgnoreNotFound(fmt.Errorf("getting composition: %w", err))
}
if !controllerutil.ContainsFinalizer(comp, "eno.azure.io/cleanup") {
return ctrl.Result{}, nil // nothing to do
}

list := &apiv1.ResourceSliceList{}
err = c.client.List(ctx, list, client.MatchingFields{
manager.IdxResourceSlicesByComposition: comp.Name,
})
if err != nil {
return ctrl.Result{}, fmt.Errorf("listing resource slices: %w", err)
}
if len(list.Items) > 0 {
return ctrl.Result{}, nil // slices still exist
}

controllerutil.RemoveFinalizer(comp, "eno.azure.io/cleanup")
err = c.client.Update(ctx, comp)
if err != nil {
return ctrl.Result{}, fmt.Errorf("removing finalizer: %w", err)
}

logger.Info("removed finalizer from composition because none of its resource slices remain")
return ctrl.Result{}, nil
}
2 changes: 1 addition & 1 deletion internal/controllers/cleanup/resourceslice.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func NewResourceSliceController(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&apiv1.ResourceSlice{}).
Watches(&apiv1.Composition{}, manager.NewCompositionToResourceSliceHandler(mgr.GetClient())).
WithLogConstructor(manager.NewLogConstructor(mgr, "sliceResourceLifecycleController")).
WithLogConstructor(manager.NewLogConstructor(mgr, "resourceSliceCleanupController")).
Complete(&resourceSliceController{
client: mgr.GetClient(),
})
Expand Down
15 changes: 7 additions & 8 deletions internal/controllers/reconciliation/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -499,15 +499,13 @@ func TestDeletionForeground(t *testing.T) {
testDeletion(t, client.PropagationPolicy(metav1.DeletePropagationForeground))
}

// TODO: Finish implementing composition cleanup

// func TestDeletionBackground(t *testing.T) {
// testDeletion(t, client.PropagationPolicy(metav1.DeletePropagationBackground))
// }
func TestDeletionBackground(t *testing.T) {
testDeletion(t, client.PropagationPolicy(metav1.DeletePropagationBackground))
}

// func TestDeletionOrphan(t *testing.T) {
// testDeletion(t, client.PropagationPolicy(metav1.DeletePropagationOrphan))
// }
func TestDeletionOrphan(t *testing.T) {
testDeletion(t, client.PropagationPolicy(metav1.DeletePropagationOrphan))
}

func testDeletion(t *testing.T, deleteOpts ...client.DeleteOption) {
scheme := runtime.NewScheme()
Expand All @@ -525,6 +523,7 @@ func testDeletion(t *testing.T, deleteOpts ...client.DeleteOption) {
require.NoError(t, synthesis.NewRolloutController(mgr.Manager, time.Millisecond))
require.NoError(t, synthesis.NewStatusController(mgr.Manager))
require.NoError(t, cleanup.NewResourceSliceController(mgr.Manager))
require.NoError(t, cleanup.NewCompositionController(mgr.Manager))
require.NoError(t, synthesis.NewPodLifecycleController(mgr.Manager, &synthesis.Config{
Timeout: time.Second * 5,
}))
Expand Down

0 comments on commit d53a4fd

Please sign in to comment.