Skip to content

Commit caefcc8

Browse files
committed
target reconciler: never update dependents which are in deletion
this is to avoid clearing finalizers; other changes would be anyway rejected by the api server; in addition, when updating dependents, merge existing finalizers
1 parent b38f3a1 commit caefcc8

File tree

1 file changed

+11
-2
lines changed

1 file changed

+11
-2
lines changed

pkg/component/target.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ func (t *reconcileTarget[T]) Reconcile(ctx context.Context, component T) (bool,
522522
item.Phase = PhaseCreating
523523
item.Status = kstatus.InProgressStatus.String()
524524
numUnready++
525-
} else if existingObject.GetAnnotations()[t.annotationKeyDigest] != item.Digest {
525+
} else if existingObject.GetDeletionTimestamp().IsZero() && existingObject.GetAnnotations()[t.annotationKeyDigest] != item.Digest {
526526
switch updatePolicy {
527527
case types.UpdatePolicyDefault:
528528
if err := t.updateObject(ctx, object, existingObject); err != nil {
@@ -544,7 +544,7 @@ func (t *reconcileTarget[T]) Reconcile(ctx context.Context, component T) (bool,
544544
if err != nil {
545545
return false, errors.Wrapf(err, "error checking status of object %s", item)
546546
}
547-
if res.Status == kstatus.CurrentStatus {
547+
if existingObject.GetDeletionTimestamp().IsZero() && res.Status == kstatus.CurrentStatus {
548548
item.Phase = PhaseReady
549549
} else {
550550
numUnready++
@@ -714,6 +714,10 @@ func (t *reconcileTarget[T]) updateObject(ctx context.Context, object client.Obj
714714
t.client.EventRecorder().Eventf(existingObject, corev1.EventTypeWarning, objectReasonUpdateError, "Error updating object: %s", err)
715715
}
716716
}()
717+
if !existingObject.GetDeletionTimestamp().IsZero() {
718+
// note: we must not update objects which are in deletion (e.g. to avoid unintentionally clearing finalizers), so we want to see the panic in that case
719+
panic("this cannot happen")
720+
}
717721
data, err := runtime.DefaultUnstructuredConverter.ToUnstructured(object)
718722
if err != nil {
719723
return err
@@ -722,6 +726,9 @@ func (t *reconcileTarget[T]) updateObject(ctx context.Context, object client.Obj
722726
if isCrd(object) || isApiService(object) {
723727
controllerutil.AddFinalizer(object, t.reconcilerName)
724728
}
729+
for _, finalizer := range existingObject.GetFinalizers() {
730+
controllerutil.AddFinalizer(object, finalizer)
731+
}
725732
if object.GetResourceVersion() == "" {
726733
object.SetResourceVersion((existingObject.GetResourceVersion()))
727734
}
@@ -818,6 +825,7 @@ func (t *reconcileTarget[T]) isCrdUsed(ctx context.Context, crd *apiextensionsv1
818825
Version: crd.Spec.Versions[0].Name,
819826
Kind: crd.Spec.Names.Kind,
820827
}
828+
// TODO: better use metav1.PartialObjectMetadataList?
821829
list := &unstructured.UnstructuredList{}
822830
list.SetGroupVersionKind(gvk)
823831
labelSelector := labels.Everything()
@@ -874,6 +882,7 @@ func (t *reconcileTarget[T]) isApiServiceUsed(ctx context.Context, apiService *a
874882
Version: apiService.Spec.Version,
875883
Kind: kind,
876884
}
885+
// TODO: better use metav1.PartialObjectMetadataList?
877886
list := &unstructured.UnstructuredList{}
878887
list.SetGroupVersionKind(gvk)
879888
if err := t.client.List(ctx, list, &client.ListOptions{LabelSelector: labelSelector, Limit: 1}); err != nil {

0 commit comments

Comments
 (0)