Skip to content

Commit 9afe456

Browse files
committed
Fix stale conditions on delete
Signed-off-by: Mangirdas Judeikis <mangirdas@judeikis.lt> On-behalf-of: @SAP mangirdas.judeikis@sap.com
1 parent ff0b5f2 commit 9afe456

2 files changed

Lines changed: 37 additions & 3 deletions

File tree

pkg/reconciler/apis/apibindingdeletion/apibinding_deletion_controller.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,6 @@ func (c *Controller) process(ctx context.Context, key string) error {
261261
return remainingErr
262262
}
263263

264-
apibindingCopy = apibinding.DeepCopy()
265264
filtered := make([]string, 0, len(apibindingCopy.Finalizers))
266265
for i := range apibindingCopy.Finalizers {
267266
if apibindingCopy.Finalizers[i] == APIBindingFinalizer {

pkg/reconciler/core/logicalclusterdeletion/deletion/logicalcluster_resource_deletor.go

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import (
5050

5151
kcpmetadata "github.com/kcp-dev/client-go/metadata"
5252
"github.com/kcp-dev/logicalcluster/v3"
53+
"github.com/kcp-dev/sdk/apis/apis"
5354
"github.com/kcp-dev/sdk/apis/core"
5455
corev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1"
5556
tenancyv1alpha1 "github.com/kcp-dev/sdk/apis/tenancy/v1alpha1"
@@ -380,19 +381,31 @@ func (d *logicalClusterResourcesDeleter) deleteAllContent(ctx context.Context, w
380381
return d.isBoundResource(logicalcluster.From(ws), group, resource)
381382
}},
382383
}, resources)
383-
groupVersionResources, err := groupVersionResources(deletableResources)
384+
allGVRs, err := groupVersionResources(deletableResources)
384385
if err != nil {
385386
// discovery errors are not fatal. We often have some set of resources we can operate against even if we don't have a complete list
386387
errs = append(errs, err)
387388
deletionContentSuccessReason = "GroupVersionParsingFailed"
388389
}
389390

391+
// Split GVRs: delete APIBindings last so their schemas remain available
392+
// while namespaces cascade-delete bound resources.
393+
apiBindingGVRs := map[schema.GroupVersionResource]sets.Set[string]{}
394+
nonAPIBindingGVRs := map[schema.GroupVersionResource]sets.Set[string]{}
395+
for gvr, verbs := range allGVRs {
396+
if gvr.Group == apis.GroupName && gvr.Resource == "apibindings" {
397+
apiBindingGVRs[gvr] = verbs
398+
} else {
399+
nonAPIBindingGVRs[gvr] = verbs
400+
}
401+
}
402+
390403
numRemainingTotals := allGVRDeletionMetadata{
391404
gvrToNumRemaining: map[schema.GroupVersionResource]int{},
392405
finalizersToNumRemaining: map[string]int{},
393406
}
394407
deleteContentErrs := []error{}
395-
for gvr, verbs := range groupVersionResources {
408+
for gvr, verbs := range nonAPIBindingGVRs {
396409
gvrDeletionMetadata, err := d.deleteAllContentForGroupVersionResource(ctx, logicalcluster.From(ws), gvr, verbs, clusterDeletedAt)
397410
if err != nil {
398411
// If there is an error, hold on to it but proceed with all the remaining
@@ -413,6 +426,28 @@ func (d *logicalClusterResourcesDeleter) deleteAllContent(ctx context.Context, w
413426
}
414427
}
415428

429+
// Only delete APIBindings after all other resources have been cleaned up.
430+
if len(numRemainingTotals.gvrToNumRemaining) == 0 && len(numRemainingTotals.finalizersToNumRemaining) == 0 && len(deleteContentErrs) == 0 {
431+
for gvr, verbs := range apiBindingGVRs {
432+
gvrDeletionMetadata, err := d.deleteAllContentForGroupVersionResource(ctx, logicalcluster.From(ws), gvr, verbs, clusterDeletedAt)
433+
if err != nil {
434+
deleteContentErrs = append(deleteContentErrs, err)
435+
}
436+
if gvrDeletionMetadata.finalizerEstimateSeconds > estimate {
437+
estimate = gvrDeletionMetadata.finalizerEstimateSeconds
438+
}
439+
if gvrDeletionMetadata.numRemaining > 0 {
440+
numRemainingTotals.gvrToNumRemaining[gvr] = gvrDeletionMetadata.numRemaining
441+
for finalizer, numRemaining := range gvrDeletionMetadata.finalizersToNumRemaining {
442+
if numRemaining == 0 {
443+
continue
444+
}
445+
numRemainingTotals.finalizersToNumRemaining[finalizer] += numRemaining
446+
}
447+
}
448+
}
449+
}
450+
416451
if len(deleteContentErrs) > 0 {
417452
errs = append(errs, deleteContentErrs...)
418453
deletionContentSuccessReason = "ContentDeletionFailed"

0 commit comments

Comments
 (0)