reconcile: delete finalizer during reconcile only for resources that do not have any cleanup tasks by AndrewChubatiuk · Pull Request #1902 · VictoriaMetrics/operator

AndrewChubatiuk · 2026-02-28T05:28:26Z

currently reconcile functions for VMAgent, VMAuth and VMCluster remove finalizer on non-empty DeletionTimestamp, which may lead to dangling resources. This PR keeps finalizer for these resources when DeletionTimestamp is not empty to let respective controller handle resource removal.

additionally:

fixed API templates to remove extra newlines from generated markdown
remove finalizer for VMServiceScrape and VMPodscrape during reconcile as both resources have no cleanup actions needed
removed impact of metadata change on a need to recreate STS due to PVC changes
added RevisionHistoryLimit to a list of STS mutable fields to avoid its unneeded recreation

Summary by cubic

Keep finalizers only on resources that need cleanup (VMAgent, VMAuth, VMCluster), and remove them during reconcile for core K8s and scrape resources to prevent stuck deletions. Diff is now directional (desired vs current) and treats empty and nil equally; StatefulSet recreate checks use Spec-only changes.

Bug Fixes
- Finalizers: remove for ConfigMap/Secret/Service/ServiceAccount/RBAC/HPA/VPA/Ingress/HTTPRoute/PDB/DaemonSet/Deployment/StatefulSet/VMPodScrape/VMServiceScrape; keep for VMAgent/VMAuth/VMCluster. collectGarbage now takes a removeFinalizer flag and waits on deletion without dropping finalizers for VM resources. CHANGELOG updated.
- StatefulSet: ignore PVC metadata when deciding to recreate; compare Spec only and normalize immutable fields (including RevisionHistoryLimit); log spec_diff.
- Diff: fix argument order (desired, current); use EquateEmpty so empty slices/maps equal nil; derivative ignores zero values in the desired object; tests updated.
Refactors
- Diff logs: field-level paths with -old +new; reconcilers log as spec_diff/data_diff.
- Logs/Errors: use NamespacedName.String() across reconcile and vmdistributed; deletion-wait logs show finalizers only when present; API markdown templates trimmed extra newlines.

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

cubic-dev-ai

1 issue found across 21 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


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

<violation number="1" location="docs/CHANGELOG.md:23">
P1: Custom agent: **Changelog Review Agent**

This changelog entry violates the mandatory structure: it lacks required issue/PR references and describes internal implementation details instead of a clear user-visible before/after outcome.</violation>
</file>

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

docs/CHANGELOG.md

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

vrutkovs · 2026-03-03T15:21:06Z

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

-		specD := diffDeep(actualPVC.Spec, newPVC.Spec)
-		logMsg := fmt.Sprintf("changes detected for PVC=%s metaDiff=%v, specDiff=%v", newPVC.Name, metaD, specD)
-		logger.WithContext(ctx).Info(logMsg)
+	specDiff := diffDeepDerivative(newObj.Spec, existingObj.Spec)


No metadata diff?

metadata is now updated in updatePVC function, before this function only changed PVC size.

AndrewChubatiuk · 2026-03-03T15:27:36Z

@cubic-dev-ai

cubic-dev-ai · 2026-03-03T15:27:43Z

@cubic-dev-ai

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

cubic-dev-ai

2 issues found across 31 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


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

<violation number="1" location="docs/CHANGELOG.md:27">
P1: Custom agent: **Changelog Review Agent**

This changelog entry doesn’t follow the required structure: it lacks a before/after user‑visible explanation and includes no issue/PR references. It also reads as an internal implementation detail, which is rejected by the changelog rule.</violation>

<violation number="2" location="docs/CHANGELOG.md:27">
P3: Rewrite this changelog entry for clarity and correct grammar so users can clearly understand which resources lose finalizers and which keep them.

(Based on your team's feedback about clarifying behavior and roles in documentation text.) [FEEDBACK_USED]</violation>
</file>

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

docs/CHANGELOG.md

cubic-dev-ai

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


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

<violation number="1" location="docs/CHANGELOG.md:27">
P1: Custom agent: **Changelog Review Agent**

This changelog entry violates the required structure because it omits issue/PR references (mandatory "References" section). Add relevant links to comply with the changelog format gate.</violation>
</file>

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

cubic-dev-ai · 2026-03-03T15:35:17Z

docs/CHANGELOG.md

 * BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): fix PVC being owned by StatefulSet and top-level object simultaenously. See [#1845](https://github.com/VictoriaMetrics/operator/issues/1845).
-
 * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): remove unneeded finalizer from core K8s resources. See [#835](https://github.com/VictoriaMetrics/operator/issues/835).
+* BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): remove finalizers from VMServiceScrape and VMPodScrape objects, and keep finalizers on VMAgent, VMCluster, and VMAuth when DeletionTimestamp is not empty.


P1: Custom agent: Changelog Review Agent

This changelog entry violates the required structure because it omits issue/PR references (mandatory "References" section). Add relevant links to comply with the changelog format gate.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At docs/CHANGELOG.md, line 27: <comment>This changelog entry violates the required structure because it omits issue/PR references (mandatory "References" section). Add relevant links to comply with the changelog format gate.</comment> <file context> @@ -24,7 +24,7 @@ aliases: * BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): fix PVC being owned by StatefulSet and top-level object simultaenously. See [#1845](https://github.com/VictoriaMetrics/operator/issues/1845). * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): remove unneeded finalizer from core K8s resources. See [#835](https://github.com/VictoriaMetrics/operator/issues/835). -* BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): remove finalizers from VMServiceScrape and VMPodScrape objects and keep them at VMAgent, VMCluster, VMAuth finalizers when DeletionTimestamp is not empty +* BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): remove finalizers from VMServiceScrape and VMPodScrape objects, and keep finalizers on VMAgent, VMCluster, and VMAuth when DeletionTimestamp is not empty. ## [v0.68.1](https://github.com/VictoriaMetrics/operator/releases/tag/v0.68.1) </file context>

…do not have any cleanup tasks

vrutkovs · 2026-03-03T15:40:45Z

Perhaps this fixes #1921? I wonder if we could add test for that?

- convert NamespacedName to string in logs - update diffDeepDerivative, now slices and maps and considered equal if length of both is 0 (not only of the first maps as it was before), before labels and annotations diff was ignored - do not take PVC metadata diff into account during checking if STS should be recreated since metadata update is covered by a separate function - fixed diffDeep and diffDeepDerivative signature, arguments were swapped before

…do not have any cleanup tasks (#1902) * reconcile: delete finalizer during reconcile only for resources that do not have any cleanup tasks * reconcile updates - convert NamespacedName to string in logs - update diffDeepDerivative, now slices and maps and considered equal if length of both is 0 (not only of the first maps as it was before), before labels and annotations diff was ignored - do not take PVC metadata diff into account during checking if STS should be recreated since metadata update is covered by a separate function - fixed diffDeep and diffDeepDerivative signature, arguments were swapped before

AndrewChubatiuk requested a review from vrutkovs as a code owner February 28, 2026 05:28

cubic-dev-ai bot reviewed Feb 28, 2026

View reviewed changes

docs/CHANGELOG.md Outdated Show resolved Hide resolved

AndrewChubatiuk force-pushed the fix-do-not-delete-finalizer-on-vm-resources branch 8 times, most recently from 47418d4 to ed152c7 Compare March 1, 2026 05:44

AndrewChubatiuk assigned vrutkovs Mar 1, 2026

AndrewChubatiuk force-pushed the fix-do-not-delete-finalizer-on-vm-resources branch from ed152c7 to 299a15a Compare March 1, 2026 07:57

AndrewChubatiuk mentioned this pull request Mar 3, 2026

Release 0.68.2 #1918

Merged

vrutkovs requested changes Mar 3, 2026

View reviewed changes

docs/CHANGELOG.md Outdated Show resolved Hide resolved

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

AndrewChubatiuk force-pushed the fix-do-not-delete-finalizer-on-vm-resources branch 3 times, most recently from 56916f3 to 8e418f6 Compare March 3, 2026 15:15

vrutkovs approved these changes Mar 3, 2026

View reviewed changes

AndrewChubatiuk force-pushed the fix-do-not-delete-finalizer-on-vm-resources branch from 5c56c1c to a263dfc Compare March 3, 2026 15:20

vrutkovs reviewed Mar 3, 2026

View reviewed changes

AndrewChubatiuk force-pushed the fix-do-not-delete-finalizer-on-vm-resources branch from a263dfc to e486f15 Compare March 3, 2026 15:27

cubic-dev-ai bot reviewed Mar 3, 2026

View reviewed changes

docs/CHANGELOG.md Outdated Show resolved Hide resolved

docs/CHANGELOG.md Outdated Show resolved Hide resolved

cubic-dev-ai bot reviewed Mar 3, 2026

View reviewed changes

reconcile: delete finalizer during reconcile only for resources that …

24e67f5

…do not have any cleanup tasks

AndrewChubatiuk force-pushed the fix-do-not-delete-finalizer-on-vm-resources branch 2 times, most recently from 0a14bcc to afdb35f Compare March 3, 2026 15:40

vrutkovs approved these changes Mar 3, 2026

View reviewed changes

AndrewChubatiuk force-pushed the fix-do-not-delete-finalizer-on-vm-resources branch from afdb35f to fe8a38a Compare March 3, 2026 15:52

AndrewChubatiuk merged commit 3e5730f into master Mar 3, 2026
2 checks passed

AndrewChubatiuk deleted the fix-do-not-delete-finalizer-on-vm-resources branch March 3, 2026 15:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reconcile: delete finalizer during reconcile only for resources that do not have any cleanup tasks#1902

reconcile: delete finalizer during reconcile only for resources that do not have any cleanup tasks#1902
AndrewChubatiuk merged 2 commits intomasterfrom
fix-do-not-delete-finalizer-on-vm-resources

AndrewChubatiuk commented Feb 28, 2026 •

edited by cubic-dev-ai bot

Loading

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vrutkovs Mar 3, 2026

Uh oh!

AndrewChubatiuk Mar 3, 2026

Uh oh!

AndrewChubatiuk commented Mar 3, 2026

Uh oh!

cubic-dev-ai bot commented Mar 3, 2026

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

cubic-dev-ai bot Mar 3, 2026 •

edited

Loading

Uh oh!

vrutkovs commented Mar 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

AndrewChubatiuk commented Feb 28, 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!

Uh oh!

Uh oh!

vrutkovs Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

AndrewChubatiuk Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

AndrewChubatiuk commented Mar 3, 2026

Uh oh!

cubic-dev-ai bot commented Mar 3, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Mar 3, 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!

vrutkovs commented Mar 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AndrewChubatiuk commented Feb 28, 2026 •

edited by cubic-dev-ai bot

Loading

cubic-dev-ai bot Mar 3, 2026 •

edited

Loading