Skip to content

Commit 911e68a

Browse files
committed
update to workapplier to use foreground deletion and to update owner reference when co-ownership is allowed
Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
1 parent 3b17212 commit 911e68a

File tree

9 files changed

+1543
-208
lines changed

9 files changed

+1543
-208
lines changed

pkg/controllers/workapplier/controller.go

Lines changed: 113 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.
66
You may obtain a copy of the License at
77
8-
http://www.apache.org/licenses/LICENSE-2.0
9-
10-
Unless required by applicable law or agreed to in writing, software
11-
distributed under the License is distributed on an "AS IS" BASIS,
12-
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13-
See the License for the specific language governing permissions and
14-
limitations under the License.
15-
*/
16-
17-
/*
18-
Copyright 2025 The KubeFleet Authors.
19-
20-
Licensed under the Apache License, Version 2.0 (the "License");
21-
you may not use this file except in compliance with the License.
22-
You may obtain a copy of the License at
23-
24-
http://www.apache.org/licenses/LICENSE-2.0
8+
http://www.apache.org/licenses/LICENSE-2.0
259
2610
Unless required by applicable law or agreed to in writing, software
2711
distributed under the License is distributed on an "AS IS" BASIS,
@@ -34,6 +18,7 @@ package workapplier
3418

3519
import (
3620
"context"
21+
"fmt"
3722
"time"
3823

3924
"go.uber.org/atomic"
@@ -417,7 +402,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
417402
Kind: fleetv1beta1.AppliedWorkKind,
418403
Name: appliedWork.GetName(),
419404
UID: appliedWork.GetUID(),
420-
BlockOwnerDeletion: ptr.To(false),
405+
BlockOwnerDeletion: ptr.To(true),
421406
}
422407

423408
// Set the default values for the Work object to avoid additional validation logic in the
@@ -487,27 +472,121 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
487472

488473
// garbageCollectAppliedWork deletes the appliedWork and all the manifests associated with it from the cluster.
489474
func (r *Reconciler) garbageCollectAppliedWork(ctx context.Context, work *fleetv1beta1.Work) (ctrl.Result, error) {
490-
deletePolicy := metav1.DeletePropagationBackground
475+
deletePolicy := metav1.DeletePropagationForeground
491476
if !controllerutil.ContainsFinalizer(work, fleetv1beta1.WorkFinalizer) {
492477
return ctrl.Result{}, nil
493478
}
494-
// delete the appliedWork which will remove all the manifests associated with it
495-
// TODO: allow orphaned manifest
479+
// Delete the appliedWork which will remove all the manifests associated with it, unless they have
480+
// other owner references.
496481
appliedWork := fleetv1beta1.AppliedWork{
497482
ObjectMeta: metav1.ObjectMeta{Name: work.Name},
498483
}
499-
err := r.spokeClient.Delete(ctx, &appliedWork, &client.DeleteOptions{PropagationPolicy: &deletePolicy})
500-
switch {
501-
case apierrors.IsNotFound(err):
502-
klog.V(2).InfoS("The appliedWork is already deleted", "appliedWork", work.Name)
503-
case err != nil:
504-
klog.ErrorS(err, "Failed to delete the appliedWork", "appliedWork", work.Name)
484+
// Get the AppliedWork object
485+
if err := r.spokeClient.Get(ctx, types.NamespacedName{Name: work.Name}, &appliedWork); err != nil {
486+
if apierrors.IsNotFound(err) {
487+
klog.V(2).InfoS("The appliedWork is already deleted, removing the finalizer from the work", "appliedWork", work.Name)
488+
return r.removeWorkFinalizer(ctx, work)
489+
}
490+
klog.ErrorS(err, "Failed to get AppliedWork", "appliedWork", work.Name)
505491
return ctrl.Result{}, controller.NewAPIServerError(false, err)
506-
default:
507-
klog.InfoS("Successfully deleted the appliedWork", "appliedWork", work.Name)
508492
}
509-
controllerutil.RemoveFinalizer(work, fleetv1beta1.WorkFinalizer)
510493

494+
// Delete the appliedWork object.
495+
if appliedWork.DeletionTimestamp.IsZero() {
496+
// Update the owner reference in the AppliedWork object.
497+
if err := r.updateOwnerReference(ctx, &appliedWork); err != nil {
498+
klog.ErrorS(err, "Failed to update owner reference in the appliedWork", "appliedWork", work.Name)
499+
return ctrl.Result{}, controller.NewAPIServerError(false, err)
500+
}
501+
klog.V(2).InfoS("Deleting the appliedWork", "appliedWork", work.Name)
502+
if err := r.spokeClient.Delete(ctx, &appliedWork, &client.DeleteOptions{PropagationPolicy: &deletePolicy}); err != nil {
503+
if apierrors.IsNotFound(err) {
504+
klog.V(2).InfoS("The appliedWork is already deleted", "appliedWork", work.Name)
505+
return r.removeWorkFinalizer(ctx, work)
506+
}
507+
klog.V(2).ErrorS(err, "Failed to delete the appliedWork", "appliedWork", work.Name)
508+
return ctrl.Result{}, controller.NewAPIServerError(false, err)
509+
}
510+
}
511+
klog.V(2).InfoS("AppliedWork deletion in progress", "appliedWork", work.Name)
512+
return ctrl.Result{Requeue: true}, fmt.Errorf("AppliedWork %s is being deleted, waiting for the deletion to complete", work.Name)
513+
}
514+
515+
// updateOwnerReference updates the AppliedWork owner reference in the manifest objects.
516+
// It changes the `blockOwnerDeletion` field to true, so that the AppliedWork can be deleted in cases where
517+
// the other owner references do not exist or are invalid.
518+
// https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/#owner-references-in-object-specifications
519+
func (r *Reconciler) updateOwnerReference(ctx context.Context, appliedWork *fleetv1beta1.AppliedWork) error {
520+
appliedWorkOwnerRef := &metav1.OwnerReference{
521+
APIVersion: fleetv1beta1.GroupVersion.String(),
522+
Kind: "AppliedWork",
523+
Name: appliedWork.Name,
524+
UID: appliedWork.UID,
525+
}
526+
527+
for _, res := range appliedWork.Status.AppliedResources {
528+
gvr := schema.GroupVersionResource{
529+
Group: res.Group,
530+
Version: res.Version,
531+
Resource: res.Resource,
532+
}
533+
var obj *unstructured.Unstructured
534+
var err error
535+
if res.Namespace != "" {
536+
obj, err = r.spokeDynamicClient.Resource(gvr).Namespace(res.Namespace).Get(ctx, res.Name, metav1.GetOptions{})
537+
} else {
538+
obj, err = r.spokeDynamicClient.Resource(gvr).Get(ctx, res.Name, metav1.GetOptions{})
539+
}
540+
if err != nil {
541+
if apierrors.IsNotFound(err) {
542+
continue
543+
}
544+
klog.ErrorS(err, "Failed to get manifest", "gvr", gvr, "name", res.Name, "namespace", res.Namespace)
545+
return err
546+
}
547+
// Check if there is more than one owner reference. If there is only one owner reference, it is the appliedWork itself.
548+
// Otherwise, at least one other owner reference exists, and we need to leave resource alone.
549+
if len(obj.GetOwnerReferences()) > 1 {
550+
ownerRefs := obj.GetOwnerReferences()
551+
552+
// Re-build the owner reference; update the blockOwnerDeletion field to false.
553+
updated := false
554+
for idx := range ownerRefs {
555+
if ownerRefs[idx].UID == appliedWorkOwnerRef.UID &&
556+
ownerRefs[idx].Name == appliedWorkOwnerRef.Name &&
557+
ownerRefs[idx].Kind == appliedWorkOwnerRef.Kind &&
558+
ownerRefs[idx].APIVersion == appliedWorkOwnerRef.APIVersion {
559+
ownerRefs[idx].BlockOwnerDeletion = ptr.To(false)
560+
updated = true
561+
}
562+
}
563+
if updated {
564+
obj.SetOwnerReferences(ownerRefs)
565+
566+
if res.Namespace != "" {
567+
_, err = r.spokeDynamicClient.Resource(gvr).Namespace(res.Namespace).Update(ctx, obj, metav1.UpdateOptions{})
568+
if err != nil {
569+
klog.ErrorS(err, "Failed to update manifest owner references", "gvr", gvr, "name", res.Name, "namespace", res.Namespace)
570+
return err
571+
}
572+
klog.V(4).InfoS("Updated manifest owner references", "gvr", gvr, "name", res.Name, "namespace", res.Namespace)
573+
} else {
574+
_, err = r.spokeDynamicClient.Resource(gvr).Update(ctx, obj, metav1.UpdateOptions{})
575+
if err != nil {
576+
klog.ErrorS(err, "Failed to update manifest owner references", "gvr", gvr, "name", res.Name)
577+
return err
578+
}
579+
klog.V(4).InfoS("Updated manifest owner references", "gvr", gvr, "name", res.Name)
580+
}
581+
}
582+
}
583+
}
584+
return nil
585+
}
586+
587+
// removeWorkFinalizer removes the finalizer from the work and updates it in the hub.
588+
func (r *Reconciler) removeWorkFinalizer(ctx context.Context, work *fleetv1beta1.Work) (ctrl.Result, error) {
589+
controllerutil.RemoveFinalizer(work, fleetv1beta1.WorkFinalizer)
511590
if err := r.hubClient.Update(ctx, work, &client.UpdateOptions{}); err != nil {
512591
klog.ErrorS(err, "Failed to remove the finalizer from the work", "work", klog.KObj(work))
513592
return ctrl.Result{}, controller.NewAPIServerError(false, err)
@@ -557,6 +636,10 @@ func (r *Reconciler) ensureAppliedWork(ctx context.Context, work *fleetv1beta1.W
557636
return nil, controller.NewAPIServerError(false, err)
558637
}
559638
}
639+
if err := r.spokeClient.Get(ctx, types.NamespacedName{Name: workRef.Name}, appliedWork); err != nil {
640+
klog.ErrorS(err, "Failed to get the appliedWork", "appliedWork", appliedWork.Name)
641+
return nil, controller.NewAPIServerError(false, err)
642+
}
560643
klog.InfoS("Recreated the appliedWork resource", "appliedWork", workRef.Name)
561644
return appliedWork, nil
562645
}

0 commit comments

Comments
 (0)