Skip to content
6 changes: 4 additions & 2 deletions pkg/controllers/workapplier/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package workapplier
import (
"context"
"fmt"
"reflect"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/validation"
Expand Down Expand Up @@ -518,7 +517,10 @@ func validateOwnerReferences(
// expected AppliedWork object. For safety reasons, Fleet will still do a sanity check.
found := false
for _, ownerRef := range inMemberClusterObjOwnerRefs {
if reflect.DeepEqual(ownerRef, *expectedAppliedWorkOwnerRef) {
if ownerRef.UID == expectedAppliedWorkOwnerRef.UID &&
ownerRef.Name == expectedAppliedWorkOwnerRef.Name &&
ownerRef.Kind == expectedAppliedWorkOwnerRef.Kind &&
ownerRef.APIVersion == expectedAppliedWorkOwnerRef.APIVersion {
found = true
break
}
Expand Down
140 changes: 110 additions & 30 deletions pkg/controllers/workapplier/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,11 @@
limitations under the License.
*/

/*
Copyright 2025 The KubeFleet Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package workapplier

import (
"context"
"fmt"
"time"

"go.uber.org/atomic"
Expand Down Expand Up @@ -417,7 +402,7 @@
Kind: fleetv1beta1.AppliedWorkKind,
Name: appliedWork.GetName(),
UID: appliedWork.GetUID(),
BlockOwnerDeletion: ptr.To(false),
BlockOwnerDeletion: ptr.To(true),
}

// Set the default values for the Work object to avoid additional validation logic in the
Expand Down Expand Up @@ -487,27 +472,122 @@

// garbageCollectAppliedWork deletes the appliedWork and all the manifests associated with it from the cluster.
func (r *Reconciler) garbageCollectAppliedWork(ctx context.Context, work *fleetv1beta1.Work) (ctrl.Result, error) {
deletePolicy := metav1.DeletePropagationBackground
deletePolicy := metav1.DeletePropagationForeground
if !controllerutil.ContainsFinalizer(work, fleetv1beta1.WorkFinalizer) {
return ctrl.Result{}, nil
}
// delete the appliedWork which will remove all the manifests associated with it
// TODO: allow orphaned manifest
// Delete the appliedWork which will remove all the manifests associated with it, unless they have
// other owner references.
appliedWork := fleetv1beta1.AppliedWork{
ObjectMeta: metav1.ObjectMeta{Name: work.Name},
}
err := r.spokeClient.Delete(ctx, &appliedWork, &client.DeleteOptions{PropagationPolicy: &deletePolicy})
switch {
case apierrors.IsNotFound(err):
klog.V(2).InfoS("The appliedWork is already deleted", "appliedWork", work.Name)
case err != nil:
klog.ErrorS(err, "Failed to delete the appliedWork", "appliedWork", work.Name)
return ctrl.Result{}, controller.NewAPIServerError(false, err)
default:
klog.InfoS("Successfully deleted the appliedWork", "appliedWork", work.Name)
// Get the AppliedWork object
if err := r.spokeClient.Get(ctx, types.NamespacedName{Name: work.Name}, &appliedWork); err != nil {
if apierrors.IsNotFound(err) {
klog.V(2).InfoS("The appliedWork is already deleted, removing the finalizer from the work", "appliedWork", work.Name)
return r.removeWorkFinalizer(ctx, work)
}
klog.ErrorS(err, "Failed to get AppliedWork", "appliedWork", work.Name)
return ctrl.Result{Requeue: true}, controller.NewAPIServerError(false, err)

Check warning on line 491 in pkg/controllers/workapplier/controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/workapplier/controller.go#L490-L491

Added lines #L490 - L491 were not covered by tests
}
controllerutil.RemoveFinalizer(work, fleetv1beta1.WorkFinalizer)

// Delete the appliedWork object.
if appliedWork.DeletionTimestamp.IsZero() {
// Update the owner reference in the AppliedWork object.
if err := r.updateOwnerReference(ctx, &appliedWork); err != nil {
klog.ErrorS(err, "Failed to update owner reference in the appliedWork", "appliedWork", work.Name)
return ctrl.Result{}, controller.NewAPIServerError(false, err)
}

Check warning on line 500 in pkg/controllers/workapplier/controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/workapplier/controller.go#L498-L500

Added lines #L498 - L500 were not covered by tests
klog.V(2).InfoS("Deleting the appliedWork", "appliedWork", work.Name)
if err := r.spokeClient.Delete(ctx, &appliedWork, &client.DeleteOptions{PropagationPolicy: &deletePolicy}); err != nil {
if apierrors.IsNotFound(err) {
klog.V(2).InfoS("The appliedWork is already deleted", "appliedWork", work.Name)
return r.removeWorkFinalizer(ctx, work)
}
klog.V(2).ErrorS(err, "Failed to delete the appliedWork", "appliedWork", work.Name)
return ctrl.Result{}, controller.NewAPIServerError(false, err)

Check warning on line 508 in pkg/controllers/workapplier/controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/workapplier/controller.go#L503-L508

Added lines #L503 - L508 were not covered by tests
}
}
klog.V(2).InfoS("AppliedWork deletion in progress", "appliedWork", work.Name)
return ctrl.Result{Requeue: true}, fmt.Errorf("AppliedWork %s is being deleted, waiting for the deletion to complete", work.Name)
}

// updateOwnerReference updates the AppliedWork owner reference in the manifest objects.
// It changes the `blockOwnerDeletion` field to true, so that the AppliedWork can be deleted in cases where
// the other owner references do not exist or are invalid.
// https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/#owner-references-in-object-specifications
func (r *Reconciler) updateOwnerReference(ctx context.Context, appliedWork *fleetv1beta1.AppliedWork) error {
appliedWorkOwnerRef := &metav1.OwnerReference{
APIVersion: fleetv1beta1.GroupVersion.String(),
Kind: "AppliedWork",
Name: appliedWork.Name,
UID: appliedWork.UID,
}

for _, res := range appliedWork.Status.AppliedResources {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Britania! This part might have some potential consistency issues; the AppliedResources on AppliedWork objects are updated only at the end of the reconciliation loop on a best effort basis; it might not incude all the applied manifests if, say, the status update is skipped/interruptted. The work object status is, instead, designed to be always consistent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make appliedResources accurate? I thought we rely on this to remove resources from the member cluster

gvr := schema.GroupVersionResource{
Group: res.Group,
Version: res.Version,
Resource: res.Resource,
}
var obj *unstructured.Unstructured
var err error
if res.Namespace != "" {
obj, err = r.spokeDynamicClient.Resource(gvr).Namespace(res.Namespace).Get(ctx, res.Name, metav1.GetOptions{})
} else {
obj, err = r.spokeDynamicClient.Resource(gvr).Get(ctx, res.Name, metav1.GetOptions{})
}
if err != nil {
if apierrors.IsNotFound(err) {
continue
}
klog.ErrorS(err, "Failed to get manifest", "gvr", gvr, "name", res.Name, "namespace", res.Namespace)
return err

Check warning on line 545 in pkg/controllers/workapplier/controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/workapplier/controller.go#L544-L545

Added lines #L544 - L545 were not covered by tests
}
// Check if there is more than one owner reference. If there is only one owner reference, it is the appliedWork itself.
// Otherwise, at least one other owner reference exists, and we need to leave resource alone.
if len(obj.GetOwnerReferences()) > 1 {
ownerRefs := obj.GetOwnerReferences()

// Re-build the owner reference; update the blockOwnerDeletion field to false.
updated := false
for idx := range ownerRefs {
if ownerRefs[idx].UID == appliedWorkOwnerRef.UID &&
ownerRefs[idx].Name == appliedWorkOwnerRef.Name &&
ownerRefs[idx].Kind == appliedWorkOwnerRef.Kind &&
ownerRefs[idx].APIVersion == appliedWorkOwnerRef.APIVersion &&
*ownerRefs[idx].BlockOwnerDeletion {
ownerRefs[idx].BlockOwnerDeletion = ptr.To(false)
updated = true
}
}
if updated {
obj.SetOwnerReferences(ownerRefs)

if res.Namespace != "" {
_, err = r.spokeDynamicClient.Resource(gvr).Namespace(res.Namespace).Update(ctx, obj, metav1.UpdateOptions{})
if err != nil {
klog.ErrorS(err, "Failed to update manifest owner references", "gvr", gvr, "name", res.Name, "namespace", res.Namespace)
return err
}

Check warning on line 572 in pkg/controllers/workapplier/controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/workapplier/controller.go#L570-L572

Added lines #L570 - L572 were not covered by tests
klog.V(4).InfoS("Updated manifest owner references", "gvr", gvr, "name", res.Name, "namespace", res.Namespace)
} else {
_, err = r.spokeDynamicClient.Resource(gvr).Update(ctx, obj, metav1.UpdateOptions{})
if err != nil {
klog.ErrorS(err, "Failed to update manifest owner references", "gvr", gvr, "name", res.Name)
return err
}

Check warning on line 579 in pkg/controllers/workapplier/controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/workapplier/controller.go#L577-L579

Added lines #L577 - L579 were not covered by tests
klog.V(4).InfoS("Updated manifest owner references", "gvr", gvr, "name", res.Name)
}
}
}
}
return nil
}

// removeWorkFinalizer removes the finalizer from the work and updates it in the hub.
func (r *Reconciler) removeWorkFinalizer(ctx context.Context, work *fleetv1beta1.Work) (ctrl.Result, error) {
controllerutil.RemoveFinalizer(work, fleetv1beta1.WorkFinalizer)
if err := r.hubClient.Update(ctx, work, &client.UpdateOptions{}); err != nil {
klog.ErrorS(err, "Failed to remove the finalizer from the work", "work", klog.KObj(work))
return ctrl.Result{}, controller.NewAPIServerError(false, err)
Expand Down
Loading
Loading