Provisioning finalisers fix 2 (#111679)

* adding some logs to better understand what might be happening

* only focus this PR on improve logging in finalizer handling

* debug log before calling finalizers

* working on finalizers

* removing last todos, adding unit tests

* better use SupportedFinalizers name

* addressing comments

* wip: fix tests and add delete error in status

* chore: codegen

* chore: codegen openapi

* Merge remote-tracking branch 'origin/main' into provisioning-finalisers-fix-2

* update frontend client

* fix: errors in testing

* fix: breaking test

---------

Co-authored-by: Daniele Ferru <daniele.ferru@grafana.com>
Co-authored-by: Ryan McKinley <ryantxu@gmail.com>
This commit is contained in:
Costa Alexoglou
2025-09-29 15:21:12 +02:00
committed by GitHub
co-authored by Daniele Ferru Ryan McKinley
parent 893523dd7c
commit 1b766b9c9f
22 changed files with 1296 additions and 484 deletions
@@ -3,6 +3,8 @@ package controller
import (
"context"
"encoding/json"
"fmt"
"slices"
"sort"
"strings"
"time"
@@ -32,8 +34,18 @@ func (f *finalizer) process(ctx context.Context,
finalizers []string,
) error {
logger := logging.FromContext(ctx)
logger.Info("process finalizers", "finalizers", finalizers)
for _, finalizer := range finalizers {
orderedFinalizers := [3]string{
repository.CleanFinalizer,
repository.ReleaseOrphanResourcesFinalizer,
repository.RemoveOrphanResourcesFinalizer}
for _, finalizer := range orderedFinalizers {
if !slices.Contains(finalizers, finalizer) {
continue
}
logger.Info("running finalizer", "finalizer", finalizer)
var err error
var count int
start := time.Now()
@@ -42,44 +54,33 @@ func (f *finalizer) process(ctx context.Context,
switch finalizer {
case repository.CleanFinalizer:
// NOTE: the controller loop will never get run unless a finalizer is set
logger.Info("running cleanup finalizer")
hooks, ok := repo.(repository.Hooks)
if ok {
if err = hooks.OnDelete(ctx); err != nil {
logger.Warn("Error running deletion hooks", "err", err)
err = fmt.Errorf("execute deletion hooks: %w", err)
outcome = metricutils.ErrorOutcome
}
}
case repository.ReleaseOrphanResourcesFinalizer:
count, err = f.processExistingItems(ctx, repo.Config(),
func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error {
patchAnnotations, err := getPatchedAnnotations(item)
if err != nil {
return err
}
_, err = client.Patch(
ctx, item.Name, types.JSONPatchType, patchAnnotations, v1.PatchOptions{},
)
return err
})
logger.Info("releasing orphan resources")
count, err = f.processExistingItems(ctx, repo.Config(), f.releaseResources(ctx, logger))
if err != nil {
err = fmt.Errorf("release resources: %w", err)
outcome = metricutils.ErrorOutcome
logger.Warn("Error processing release orphan resources finalizer", "err", err)
}
case repository.RemoveOrphanResourcesFinalizer:
count, err = f.processExistingItems(ctx, repo.Config(),
func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error {
return client.Delete(ctx, item.Name, v1.DeleteOptions{})
})
logger.Info("removing orphan resources")
count, err = f.processExistingItems(ctx, repo.Config(), f.removeResources(ctx, logger))
if err != nil {
err = fmt.Errorf("remove resources: %w", err)
outcome = metricutils.ErrorOutcome
logger.Warn("Error processing remove orphan resources finalizer", "err", err)
}
default:
logger.Warn("skipping unknown finalizer", "finalizer", finalizer)
logger.Error("skipping unknown finalizer", "finalizer", finalizer)
continue
}
@@ -106,36 +107,74 @@ func (f *finalizer) processExistingItems(
items, err := f.lister.List(ctx, repo.Namespace, repo.Name)
if err != nil {
logger.Warn("error listing resources", "error", err)
logger.Error("error listing resources", "error", err)
return 0, err
}
// Safe deletion order
sortResourceListForDeletion(items)
count := 0
errors := 0
for _, item := range items.Items {
res, _, err := clients.ForResource(ctx, schema.GroupVersionResource{
Group: item.Group,
Resource: item.Resource,
})
logger.Error("error getting client for resource", "resource", item.Resource, "error", err)
if err != nil {
return count, err
}
err = cb(res, &item)
if err != nil {
logger.Warn("error processing item", "name", item.Name, "error", err)
errors++
logger.Error("error processing item", "name", item.Name, "error", err)
return count, fmt.Errorf("processing item: %w", err)
} else {
count++
}
}
logger.Info("processed orphan items", "items", count, "errors", errors)
logger.Info("processed orphan items", "items", count)
return count, nil
}
func (f *finalizer) releaseResources(
ctx context.Context, logger logging.Logger,
) func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error {
return func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error {
logger.Info("release resource",
"name", item.Name,
"group", item.Group,
"resource", item.Resource,
)
patchAnnotations, err := getPatchedAnnotations(item)
if err != nil {
return fmt.Errorf("get patched annotations: %w", err)
}
_, err = client.Patch(
ctx, item.Name, types.JSONPatchType, patchAnnotations, v1.PatchOptions{},
)
if err != nil {
return fmt.Errorf("patch resource to release ownership: %w", err)
}
return nil
}
}
func (f *finalizer) removeResources(
ctx context.Context, logger logging.Logger,
) func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error {
return func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error {
logger.Info("remove resource",
"name", item.Name,
"group", item.Group,
"resource", item.Resource,
)
return client.Delete(ctx, item.Name, v1.DeleteOptions{})
}
}
type jsonPatchOperation struct {
Op string `json:"op"`
Path string `json:"path"`