-
Notifications
You must be signed in to change notification settings - Fork 20
feat: Change AppliedWork to used foregroundDeletion #60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
3e62cf4
c0c3243
584429c
3b69b3b
94c748e
4f03345
9d4979c
92b0e24
f142dc6
d8b4fc2
b111d7d
48e15e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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" | ||
|
|
@@ -417,7 +402,7 @@ | |
| Kind: fleetv1beta1.AppliedWorkKind, | ||
| Name: appliedWork.GetName(), | ||
| UID: appliedWork.GetUID(), | ||
| BlockOwnerDeletion: ptr.To(false), | ||
| BlockOwnerDeletion: ptr.To(true), | ||
britaniar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // Set the default values for the Work object to avoid additional validation logic in the | ||
|
|
@@ -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) | ||
britaniar marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| 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) | ||
| } | ||
britaniar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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) | ||
| } | ||
| } | ||
| klog.V(2).InfoS("AppliedWork deletion in progress", "appliedWork", work.Name) | ||
britaniar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return ctrl.Result{Requeue: true}, fmt.Errorf("AppliedWork %s is being deleted, waiting for the deletion to complete", work.Name) | ||
britaniar marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // 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 { | ||
|
||
| gvr := schema.GroupVersionResource{ | ||
| Group: res.Group, | ||
| Version: res.Version, | ||
| Resource: res.Resource, | ||
| } | ||
| var obj *unstructured.Unstructured | ||
| var err error | ||
| if res.Namespace != "" { | ||
britaniar marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 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 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 { | ||
ryanzhang-oss marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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 | ||
| } | ||
| 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 | ||
| } | ||
| 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) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.