Skip to content

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

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.

Sign up for GitHub

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

Merged
AndrewChubatiuk merged 2 commits into from
Mar 3, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
24e67f5
reconcile: delete finalizer during reconcile only for resources that …
AndrewChubatiuk Feb 28, 2026
fe8a38a
reconcile updates
AndrewChubatiuk Feb 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/crds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ jobs:
run: |
cp ../__vm-operator-repo/config/crd/overlay/crd.descriptionless.yaml charts/victoria-metrics-operator/charts/crds/crds/crd.yaml
cp ../__vm-operator-repo/config/crd/overlay/crd.specless.yaml charts/victoria-metrics-operator/crd.yaml
bzip2 -k charts/victoria-metrics-operator/charts/crds/crds/crd.descriptionless.yaml
mv charts/victoria-metrics-operator/charts/crds/crds/crd.descriptionless.yaml.bz2 charts/victoria-metrics-operator/charts/crds/files/crd.yaml.bz2
bzip2 -k charts/victoria-metrics-operator/charts/crds/crds/crd.yaml
mv charts/victoria-metrics-operator/charts/crds/crds/crd.yaml.bz2 charts/victoria-metrics-operator/charts/crds/files/crd.yaml.bz2
echo "BUILDTIME=$(date +%s)" >> $GITHUB_OUTPUT
echo "SHORT_SHA=$(git rev-parse --short $GITHUB_SHA)" >> $GITHUB_OUTPUT
working-directory: __vm-charts-repo
Expand Down
3 changes: 1 addition & 2 deletions docs/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ aliases:
* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): VMPodScrape for VLAgent and VMAgent now uses the correct port; previously it used the wrong port and could cause scrape failures. See [#1887](https://github.com/VictoriaMetrics/operator/issues/1887).
* BUGFIX: [vmdistributed](https://docs.victoriametrics.com/operator/resources/vmdistributed/): updated VMAuth config consolidating all VMSelects into a single read and all VMClusters into a single write backend
* 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.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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>
Fix with Cubic


## [v0.68.1](https://github.com/VictoriaMetrics/operator/releases/tag/v0.68.1)
**Release date:** 23 February 2026
Expand All @@ -33,7 +33,6 @@ aliases:

* BUGFIX: [vmanomaly](https://docs.victoriametrics.com/operator/resources/vmanomaly/): fix configuration marshalling for [Prophet model](https://docs.victoriametrics.com/anomaly-detection/components/models/#prophet). Previously, using Prophet model would lead to panic during configuration marshalling.


## [v0.68.0](https://github.com/VictoriaMetrics/operator/releases/tag/v0.68.0)
**Release date:** 23 February 2026

Expand Down
744 changes: 2 additions & 742 deletions docs/api.md
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 5 additions & 7 deletions docs/templates/api/details.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{{- define "gvDetails" -}}
{{- $gv := . -}}
{{- $gv := . }}

## {{ $gv.GroupVersionString }}

Expand All @@ -10,10 +10,8 @@
{{- range $gv.SortedKinds }}
- {{ $gv.TypeForKind . | markdownRenderTypeLink }}
{{- end }}
{{ end }}

{{ range $gv.SortedTypes }}
{{ template "type" . }}
{{ end }}

{{- end }}
{{- range $gv.SortedTypes }}
{{- template "type" . }}
{{- end }}
{{- end -}}
8 changes: 3 additions & 5 deletions docs/templates/api/list.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ tags:
{{- range $groupVersions }}
- {{ markdownRenderGVLink . }}
{{- end }}

{{ range $groupVersions }}
{{ template "gvDetails" . }}
{{ end }}

{{- range $groupVersions }}
{{- template "gvDetails" . }}
{{- end }}
{{- end -}}
41 changes: 22 additions & 19 deletions docs/templates/api/type.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,43 +28,46 @@

{{- define "type" -}}
{{- $type := . -}}
{{- if markdownShouldRenderType $type -}}
{{- if markdownShouldRenderType $type }}

#### {{ $type.Name }}
{{- if $type.IsAlias }}

{{ if $type.IsAlias }}_Underlying type:_ _{{ markdownRenderTypeLink $type.UnderlyingType }}_{{ end }}
_Underlying type:_ _{{ markdownRenderTypeLink $type.UnderlyingType }}_
{{- end }}
{{- if $type.Doc }}

{{ $type.Doc }}
{{- end }}
{{- if $type.Validation }}

{{ if $type.Validation -}}
_Validation:_
{{- range $type.Validation }}
- {{ . }}
{{- end }}
{{- end }}
{{- if $type.References }}

{{- if $type.References -}}
Appears in: {{ range $i, $ref := $type.SortedReferences }}{{ if $i }}, {{ end }}{{ markdownRenderTypeLink $ref }}{{- end }}
{{- end }}
{{- if $type.Members }}

{{ if $type.Members -}}
| Field | Description |
| --- | --- |
{{ if $type.GVK -}}
{{- if $type.GVK }}
| apiVersion<br/>_string_ | (Required)<br/>`{{ $type.GVK.Group }}/{{ $type.GVK.Version }}` |
| kind<br/>_string_ | (Required)<br/>`{{ $type.GVK.Kind }}` |
{{ end -}}
{{- $members := default dict -}}
{{- range $member := $type.Members -}}
{{- end }}
{{- $members := default dict }}
{{- range $member := $type.Members }}
{{- $_ := set $members $member.Name $member }}
{{- end -}}
{{- $memberKeys := (keys $members | sortAlpha) -}}
{{ range $memberKeys -}}
{{- $member := index $members . -}}
{{- $id := lower (printf "%s-%s" $type.Name $member.Name) -}}
{{- end }}
{{- $memberKeys := (keys $members | sortAlpha) }}
{{- range $memberKeys }}
{{- $member := index $members . }}
{{- $id := lower (printf "%s-%s" $type.Name $member.Name) }}
| {{ $member.Name }}<a href="#{{ $id }}" id="{{ $id }}">#</a><br/>_{{ markdownRenderType $member.Type }}_ | {{ if $member.Markers.optional }}_(Optional)_<br/>{{else}}_(Required)_<br/>{{ end }}{{ template "type_members" $member }}{{ template "deprecated" (dict "member" $member "type" $type.Name) }} |
{{ end -}}

{{- end -}}
{{- end -}}
{{- end -}}
{{- end }}
{{- end }}
{{- end }}
{{- end }}
5 changes: 3 additions & 2 deletions internal/controller/operator/factory/reconcile/configmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
func ConfigMap(ctx context.Context, rclient client.Client, newObj *corev1.ConfigMap, prevMeta *metav1.ObjectMeta, owner *metav1.OwnerReference) (bool, error) {
nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace}
updated := true
removeFinalizer := true
err := retryOnConflict(func() error {
var existingObj corev1.ConfigMap
if err := rclient.Get(ctx, nsn, &existingObj); err != nil {
Expand All @@ -27,10 +28,10 @@ func ConfigMap(ctx context.Context, rclient client.Client, newObj *corev1.Config
}
return fmt.Errorf("cannot get existing ConfigMap=%s: %w", nsn.String(), err)
}
if err := collectGarbage(ctx, rclient, &existingObj); err != nil {
if err := collectGarbage(ctx, rclient, &existingObj, removeFinalizer); err != nil {
return err
}
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, true)
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, removeFinalizer)
if err != nil {
return err
}
Expand Down
5 changes: 3 additions & 2 deletions internal/controller/operator/factory/reconcile/daemonset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func DaemonSet(ctx context.Context, rclient client.Client, newObj, prevObj *apps
}
rclient.Scheme().Default(newObj)
nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace}
removeFinalizer := true
err := retryOnConflict(func() error {
var existingObj appsv1.DaemonSet
if err := rclient.Get(ctx, nsn, &existingObj); err != nil {
Expand All @@ -39,11 +40,11 @@ func DaemonSet(ctx context.Context, rclient client.Client, newObj, prevObj *apps
}
return fmt.Errorf("cannot get DaemonSet=%s: %w", nsn.String(), err)
}
if err := collectGarbage(ctx, rclient, &existingObj); err != nil {
if err := collectGarbage(ctx, rclient, &existingObj, removeFinalizer); err != nil {
return err
}
spec := &newObj.Spec
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, true)
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, removeFinalizer)
if err != nil {
return err
}
Expand Down
5 changes: 3 additions & 2 deletions internal/controller/operator/factory/reconcile/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func Deployment(ctx context.Context, rclient client.Client, newObj, prevObj *app
}
rclient.Scheme().Default(newObj)
nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace}
removeFinalizer := true
err := retryOnConflict(func() error {
var existingObj appsv1.Deployment
if err := rclient.Get(ctx, nsn, &existingObj); err != nil {
Expand All @@ -40,14 +41,14 @@ func Deployment(ctx context.Context, rclient client.Client, newObj, prevObj *app
}
return fmt.Errorf("cannot get Deployment=%s: %w", nsn.String(), err)
}
if err := collectGarbage(ctx, rclient, &existingObj); err != nil {
if err := collectGarbage(ctx, rclient, &existingObj, removeFinalizer); err != nil {
return err
}
spec := &newObj.Spec
if hasHPA {
spec.Replicas = existingObj.Spec.Replicas
}
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, true)
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, removeFinalizer)
if err != nil {
return err
}
Expand Down
3 changes: 0 additions & 3 deletions internal/controller/operator/factory/reconcile/deploy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,6 @@ func TestDeployReconcile(t *testing.T) {
getDeploy(func(d *appsv1.Deployment) {
d.Status.ReadyReplicas = 1
d.Status.Conditions[0].Reason = "ReplicaSetUpdated"
d.Spec.Template.Annotations = map[string]string{
"new-annotation": "value",
}
}),
},
actions: []k8stools.ClientAction{
Expand Down
51 changes: 22 additions & 29 deletions internal/controller/operator/factory/reconcile/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
)

type fieldDiffRecorder struct {
Expand All @@ -26,58 +27,50 @@ func (r *fieldDiffRecorder) PopStep() {

// Report implements cmp.Reporter interface
func (r *fieldDiffRecorder) Report(rs cmp.Result) {
if !rs.Equal() {
a1, a2 := r.path.Last().Values()
if r.useDerivativeDiff {
switch a1.Kind() {
case reflect.String:
if a1.Len() == 0 {
return
}
case reflect.Slice:
if a1.IsNil() || a1.Len() == 0 {
return
}
case reflect.Pointer:
if a1.IsNil() {
return
}
case reflect.Map:
if a1.IsNil() || a1.Len() == 0 {
return
}
if rs.Equal() {
return
}
desired, current := r.path.Last().Values()
if r.useDerivativeDiff {
switch desired.Kind() {
case reflect.String, reflect.Slice:
if desired.Len() == 0 {
return
}
case reflect.Pointer:
if desired.IsNil() {
return
}
}
r.diffs = append(r.diffs, fmt.Sprintf("%#v:-%q +%q", r.path, formatDiffValue(a2), formatDiffValue(a1)))
}
r.diffs = append(r.diffs, fmt.Sprintf("%#v:-%q +%q", r.path, formatDiffValue(current), formatDiffValue(desired)))
}

// String implements Stringer interface
func (r *fieldDiffRecorder) String() string {
return strings.Join(r.diffs, ",")
}

func diffDeep(a1, a2 any) string {
return diffDeepInternal(a1, a2, false)

func diffDeep(desired, current any) string {
return diffDeepInternal(desired, current, false)
}

func diffDeepDerivative(a1, a2 any) string {
return diffDeepInternal(a1, a2, true)
func diffDeepDerivative(desired, current any) string {
return diffDeepInternal(desired, current, true)
}

// diffDeepInternal is similar to diffDeep except that unset fields in a1 are
// diffDeepInternal is similar to diffDeep except that unset fields in desired are
// ignored (not compared). This allows us to focus on the fields that matter to
// the semantic comparison.
//
// The unset fields include a nil pointer and an empty string.
//
// Helper function for equality.Semantic.DeepDerivative
func diffDeepInternal(a1, a2 any, useDerivative bool) string {
func diffDeepInternal(desired, current any, useDerivative bool) string {
r := fieldDiffRecorder{
useDerivativeDiff: useDerivative,
}
cmp.Diff(a1, a2, cmp.Reporter(&r))
cmp.Diff(desired, current, cmp.Reporter(&r), cmpopts.EquateEmpty())
return r.String()

}
Expand Down
10 changes: 5 additions & 5 deletions internal/controller/operator/factory/reconcile/diff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ func TestDiffDeepOk(t *testing.T) {
}
var a1Slice []string
a2SliceNonNil := make([]string, 0)
f(a1Slice, a2SliceNonNil, `{[]string}:-"[]" +"nil"`)
f(a1Slice, a2SliceNonNil, ``)
a1MapNonNil := make(map[int]int, 0)
var a2MapNil map[int]int
f(a1MapNonNil, a2MapNil, `{map[int]int}:-"nil" +"map[]"`)
f(5, 5, "")
f(a1MapNonNil, a2MapNil, ``)
f(5, 5, ``)
f(5, 6, `{int}:-"6" +"5"`)
f("new", "line", `{string}:-"line" +"new"`)

Expand All @@ -32,7 +32,7 @@ func TestDiffDeepOk(t *testing.T) {
Field1: make([]string, 0),
Field3: 5,
}
f(a1StructEmpty, a2StructFilled, `{reconcile.cmpStruct}.Field1:-"[]" +"nil",{reconcile.cmpStruct}.Field3:-"5" +"0"`)
f(a1StructEmpty, a2StructFilled, `{reconcile.cmpStruct}.Field3:-"5" +"0"`)

var a2StructEmptyPtr *cmpStruct
a1StructFilledPtr := &cmpStruct{
Expand All @@ -54,7 +54,7 @@ func TestDiffDeepDerivativeOk(t *testing.T) {
var newM map[int]int
oldM := make(map[int]int, 0)
f(oldM, newM, ``)
f(5, 5, "")
f(5, 5, ``)
f(5, 6, `{int}:-"6" +"5"`)
f("new", "line", `{string}:-"line" +"new"`)

Expand Down
5 changes: 3 additions & 2 deletions internal/controller/operator/factory/reconcile/hpa.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func HPA(ctx context.Context, rclient client.Client, newObj, prevObj *v2.Horizon
if prevObj != nil {
prevMeta = &prevObj.ObjectMeta
}
removeFinalizer := true
return retryOnConflict(func() error {
var existingObj v2.HorizontalPodAutoscaler
if err := rclient.Get(ctx, nsn, &existingObj); err != nil {
Expand All @@ -30,10 +31,10 @@ func HPA(ctx context.Context, rclient client.Client, newObj, prevObj *v2.Horizon
}
return fmt.Errorf("cannot get HPA=%s: %w", nsn.String(), err)
}
if err := collectGarbage(ctx, rclient, &existingObj); err != nil {
if err := collectGarbage(ctx, rclient, &existingObj, removeFinalizer); err != nil {
return err
}
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, true)
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, removeFinalizer)
if err != nil {
return err
}
Expand Down
5 changes: 3 additions & 2 deletions internal/controller/operator/factory/reconcile/httproute.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func HTTPRoute(ctx context.Context, rclient client.Client, newObj, prevObj *gwap
if prevObj != nil {
prevMeta = &prevObj.ObjectMeta
}
removeFinalizer := true
return retryOnConflict(func() error {
var existingObj gwapiv1.HTTPRoute
if err := rclient.Get(ctx, nsn, &existingObj); err != nil {
Expand All @@ -30,10 +31,10 @@ func HTTPRoute(ctx context.Context, rclient client.Client, newObj, prevObj *gwap
}
return fmt.Errorf("cannot get existing HTTPRoute=%s: %w", nsn.String(), err)
}
if err := collectGarbage(ctx, rclient, &existingObj); err != nil {
if err := collectGarbage(ctx, rclient, &existingObj, removeFinalizer); err != nil {
return err
}
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, true)
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, removeFinalizer)
if err != nil {
return err
}
Expand Down
5 changes: 3 additions & 2 deletions internal/controller/operator/factory/reconcile/ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func Ingress(ctx context.Context, rclient client.Client, newObj, prevObj *networ
if prevObj != nil {
prevMeta = &prevObj.ObjectMeta
}
removeFinalizer := true
return retryOnConflict(func() error {
var existingObj networkingv1.Ingress
if err := rclient.Get(ctx, nsn, &existingObj); err != nil {
Expand All @@ -30,10 +31,10 @@ func Ingress(ctx context.Context, rclient client.Client, newObj, prevObj *networ
}
return fmt.Errorf("cannot get existing Ingress=%s: %w", nsn.String(), err)
}
if err := collectGarbage(ctx, rclient, &existingObj); err != nil {
if err := collectGarbage(ctx, rclient, &existingObj, removeFinalizer); err != nil {
return err
}
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, true)
metaChanged, err := mergeMeta(&existingObj, newObj, prevMeta, owner, removeFinalizer)
if err != nil {
return err
}
Expand Down
Loading