reconcile: refactor freeing recreated resource finalizer removal #1772

AndrewChubatiuk · 2026-01-31T11:56:06Z

related to #1707

removed returning errRecreate inside reconcile functions, which was moved freeIfneeded function (which is now private and resides inside reconcile package), it now returns errRecreate if deleteTimestamp is not zero
removed complex merging metadata logic, which was needed before managedMetadata was introduced

Summary by cubic

Fixes finalizer cleanup by centralizing freeIfNeeded across reconcilers to remove stale finalizers and recreate deleted resources. Prevents resources from getting stuck and adds Ingress reconciliation plus simpler rule config updates.

Bug Fixes
- Unified freeIfNeeded usage across ConfigMap, Secret, Service, ServiceAccount, Role/RoleBinding/ClusterRole/ClusterRoleBinding, Deployment, DaemonSet, StatefulSet (+ PVC expand), HPA, PDB, HTTPRoute, Ingress, and VMServiceScrape to avoid hanging recreations.
- Refactored helpers and simplified signatures (dropped prev object plumbing), aligned variable names (newObj/existingObj), tightened equality checks; VMAlert rules now use the ConfigMap reconciler and reload only on changes; updated changelog.

^{Written for commit da40408. Summary will update on new commits.}

cubic-dev-ai

1 issue found across 16 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="docs/CHANGELOG.md">

<violation number="1" location="docs/CHANGELOG.md:29">
P1: Rule violated: **Changelog Review Agent**

Changelog entries must include reference links and a user‑centric before/after explanation. This entry is missing any issue/PR link and focuses on internal implementation details (function ordering/finalizer handling) instead of a user-visible impact description, violating the changelog structure requirements (References + User-centric explanation clauses).</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

docs/CHANGELOG.md

AndrewChubatiuk · 2026-02-01T06:17:38Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-02-01T06:17:43Z

@cubic-dev-ai review this PR

@AndrewChubatiuk I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

4 issues found across 24 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="internal/controller/operator/factory/reconcile/vmservicescrape.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/vmservicescrape.go:43">
P3: Spec diff is computed after overwriting existingObj.Spec, so the log always shows an empty diff and loses diagnostic value. Compute the diff before assigning to existingObj.Spec.</violation>
</file>

<file name="internal/controller/operator/factory/reconcile/configmap.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/configmap.go:24">
P1: Handle non-NotFound errors from the initial Get. Otherwise, API errors are swallowed and the reconciler proceeds with an empty object, which can lead to incorrect updates or misleading retries.</violation>
</file>

<file name="internal/controller/operator/factory/reconcile/httproute.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/httproute.go:40">
P2: Use DeepDerivative here to ignore controller-defaulted fields; DeepEqual will continually detect diffs once defaults are applied to the live object.</violation>
</file>

<file name="docs/CHANGELOG.md">

<violation number="1" location="docs/CHANGELOG.md:29">
P2: Rule violated: **Changelog Review Agent**

Changelog entry doesn’t follow the required user‑centric structure: it cites an internal implementation detail (“previous resource finalizer”) and doesn’t explicitly describe the before/after user-visible behavior. The changelog rule requires a before/after explanation without internal details.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

internal/controller/operator/factory/reconcile/configmap.go

internal/controller/operator/factory/reconcile/httproute.go

docs/CHANGELOG.md

internal/controller/operator/factory/reconcile/vmscrape.go

AndrewChubatiuk · 2026-02-01T06:34:26Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-02-01T06:34:31Z

@cubic-dev-ai review this PR

@AndrewChubatiuk I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

1 issue found across 24 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="internal/controller/operator/factory/reconcile/httproute.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/httproute.go:43">
P2: The strict DeepEqual on labels defeats the existing metadata-merge logic that preserves external labels, so any label added by another controller will force updates every reconcile. Rely on isObjectMetaEqual (which already handles label/annotation differences) to avoid update churn.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

internal/controller/operator/factory/reconcile/httproute.go

AndrewChubatiuk · 2026-02-01T08:34:05Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-02-01T08:34:09Z

@cubic-dev-ai review this PR

@AndrewChubatiuk I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

2 issues found across 38 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="internal/controller/operator/factory/reconcile/hpa.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/hpa.go:36">
P3: `spec_diff` is always empty because the diff is computed after `existingObj.Spec` is overwritten with `newObj.Spec`. Capture the diff before mutating `existingObj.Spec` so the log reflects the actual changes.</violation>
</file>

<file name="internal/controller/operator/factory/vmalert/rules.go">

<violation number="1" location="internal/controller/operator/factory/vmalert/rules.go:53">
P2: Reconcile now overwrites existing ConfigMap annotations with nil, so third‑party annotations on rule ConfigMaps are lost on update. The previous logic merged current annotations; this change causes annotation loss.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

internal/controller/operator/factory/vmalert/rules.go

internal/controller/operator/factory/reconcile/hpa.go

AndrewChubatiuk · 2026-02-01T12:47:07Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-02-01T12:47:11Z

@cubic-dev-ai review this PR

@AndrewChubatiuk I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

1 issue found across 49 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="internal/controller/operator/factory/reconcile/configmap.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/configmap.go:32">
P2: BinaryData changes are ignored. The reconcile compares/updates only Data/Labels/Annotations, so any BinaryData updates will never be applied. Include BinaryData in the equality check and copy it onto the existing object before update.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-02-01T13:01:25Z

internal/controller/operator/factory/reconcile/configmap.go

-	logger.WithContext(ctx).Info(fmt.Sprintf("updating ConfigMap %s configuration", newCM.Name))
-
-	return rclient.Update(ctx, newCM)
+		if equality.Semantic.DeepEqual(newObj.Data, existingObj.Data) &&


P2: BinaryData changes are ignored. The reconcile compares/updates only Data/Labels/Annotations, so any BinaryData updates will never be applied. Include BinaryData in the equality check and copy it onto the existing object before update.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/controller/operator/factory/reconcile/configmap.go, line 32: <comment>BinaryData changes are ignored. The reconcile compares/updates only Data/Labels/Annotations, so any BinaryData updates will never be applied. Include BinaryData in the equality check and copy it onto the existing object before update.</comment> <file context> @@ -7,41 +7,39 @@ import ( - logger.WithContext(ctx).Info(fmt.Sprintf("updating ConfigMap %s configuration", newCM.Name)) - - return rclient.Update(ctx, newCM) + if equality.Semantic.DeepEqual(newObj.Data, existingObj.Data) && + equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) && + equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) { </file context>

AndrewChubatiuk · 2026-02-01T15:03:01Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-02-01T15:03:09Z

@cubic-dev-ai review this PR

@AndrewChubatiuk I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

1 issue found across 49 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="internal/controller/operator/factory/reconcile/httproute.go">

<violation number="1" location="internal/controller/operator/factory/reconcile/httproute.go:38">
P2: Overwriting labels/annotations drops third‑party metadata. Use the existing merge logic to preserve unmanaged labels/annotations instead of replacing the maps outright.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-02-01T15:13:56Z

internal/controller/operator/factory/reconcile/httproute.go

-
-		return rclient.Update(ctx, newHTTPRoute)
+		specDiff := diffDeepDerivative(newObj.Spec, existingObj.Spec)
+		existingObj.Labels = newObj.Labels


P2: Overwriting labels/annotations drops third‑party metadata. Use the existing merge logic to preserve unmanaged labels/annotations instead of replacing the maps outright.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/controller/operator/factory/reconcile/httproute.go, line 38: <comment>Overwriting labels/annotations drops third‑party metadata. Use the existing merge logic to preserve unmanaged labels/annotations instead of replacing the maps outright.</comment> <file context> @@ -10,50 +10,35 @@ import ( - - return rclient.Update(ctx, newHTTPRoute) + specDiff := diffDeepDerivative(newObj.Spec, existingObj.Spec) + existingObj.Labels = newObj.Labels + existingObj.Annotations = newObj.Annotations + existingObj.Spec = newObj.Spec </file context>

AndrewChubatiuk requested a review from vrutkovs as a code owner January 31, 2026 11:56

cubic-dev-ai bot reviewed Jan 31, 2026

View reviewed changes

docs/CHANGELOG.md Outdated Show resolved Hide resolved

AndrewChubatiuk changed the title ~~reconcile: changed order of FreeIfNeeded and non empty deletionTimestamp check~~ reconcile: refactor freeing old resource finalizer removal Jan 31, 2026

AndrewChubatiuk force-pushed the fix-finalizer-cleanup branch 2 times, most recently from 389665c to 9f07ff5 Compare February 1, 2026 06:12

cubic-dev-ai bot reviewed Feb 1, 2026

View reviewed changes

AndrewChubatiuk force-pushed the fix-finalizer-cleanup branch from 9f07ff5 to fae5765 Compare February 1, 2026 06:33

AndrewChubatiuk changed the title ~~reconcile: refactor freeing old resource finalizer removal~~ reconcile: refactor freeing recreated resource finalizer removal Feb 1, 2026

cubic-dev-ai bot reviewed Feb 1, 2026

View reviewed changes

internal/controller/operator/factory/reconcile/httproute.go Outdated Show resolved Hide resolved

AndrewChubatiuk force-pushed the fix-finalizer-cleanup branch from fae5765 to 077cd83 Compare February 1, 2026 08:33

cubic-dev-ai bot reviewed Feb 1, 2026

View reviewed changes

internal/controller/operator/factory/vmalert/rules.go Outdated Show resolved Hide resolved

internal/controller/operator/factory/reconcile/hpa.go Outdated Show resolved Hide resolved

AndrewChubatiuk force-pushed the fix-finalizer-cleanup branch from 077cd83 to 67c0728 Compare February 1, 2026 12:46

cubic-dev-ai bot reviewed Feb 1, 2026

View reviewed changes

AndrewChubatiuk force-pushed the fix-finalizer-cleanup branch from 67c0728 to f7a6626 Compare February 1, 2026 14:55

reconcile: updated recreation logic

da40408

AndrewChubatiuk force-pushed the fix-finalizer-cleanup branch from f7a6626 to da40408 Compare February 1, 2026 15:02

cubic-dev-ai bot reviewed Feb 1, 2026

View reviewed changes

AndrewChubatiuk closed this Feb 1, 2026

reconcile: refactor freeing recreated resource finalizer removal #1772

reconcile: refactor freeing recreated resource finalizer removal #1772

Uh oh!

Conversation

AndrewChubatiuk commented Jan 31, 2026 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AndrewChubatiuk commented Feb 1, 2026

Uh oh!

cubic-dev-ai bot commented Feb 1, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AndrewChubatiuk commented Feb 1, 2026

Uh oh!

cubic-dev-ai bot commented Feb 1, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AndrewChubatiuk commented Feb 1, 2026

Uh oh!

cubic-dev-ai bot commented Feb 1, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AndrewChubatiuk commented Feb 1, 2026

Uh oh!

cubic-dev-ai bot commented Feb 1, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrewChubatiuk commented Feb 1, 2026

Uh oh!

cubic-dev-ai bot commented Feb 1, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

AndrewChubatiuk commented Jan 31, 2026 •

edited by cubic-dev-ai bot

Loading

cubic-dev-ai bot Feb 1, 2026 •

edited

Loading

cubic-dev-ai bot Feb 1, 2026 •

edited

Loading