Skip to content

reconcile: refactor freeing recreated resource finalizer removal #1772

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

Closed
AndrewChubatiuk wants to merge 1 commit into from
Closed

reconcile: refactor freeing recreated resource finalizer removal #1772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ aliases:
* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): fixed conflicts for `VMAlert`, `VMAlertmanager` and `VMAuth` reconcilers, which are updating same objects concurrently with reconcilers for their child objects.
* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): previously PVC downscaling always emitted a warning, which is not expected, while using PVC autoresizer; now warning during attempt to downsize PVC is only emitted if `operator.victoriametrics.com/pvc-allow-volume-expansion: false` is not set. See [#1747](https://github.com/VictoriaMetrics/operator/issues/1747).
* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): skip self scrape objects management if respective controller is disabled. See [#1718](https://github.com/VictoriaMetrics/operator/issues/1718).
* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): previously, recreating a resource after deletion could hang and block updates; now resource recreation completes normally. See [#1707](https://github.com/VictoriaMetrics/operator/issues/1707).

## [v0.67.0](https://github.com/VictoriaMetrics/operator/releases/tag/v0.67.0)
**Release date:** 23 January 2026
Expand Down
12 changes: 0 additions & 12 deletions internal/controller/operator/factory/finalize/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,3 @@ func removeConfigReloaderRole(ctx context.Context, rclient client.Client, cr crO
}
return nil
}

// FreeIfNeeded checks if resource must be freed from finalizer and garbage collected by kubernetes
func FreeIfNeeded(ctx context.Context, rclient client.Client, object client.Object) error {
if object.GetDeletionTimestamp().IsZero() {
// fast path
return nil
}
if err := RemoveFinalizer(ctx, rclient, object); err != nil {
return fmt.Errorf("cannot remove finalizer from object=%s/%s, kind=%q: %w", object.GetNamespace(), object.GetName(), object.GetObjectKind().GroupVersionKind(), err)
}
return fmt.Errorf("deletionTimestamp is not zero=%q for object=%s/%s kind=%s, recreating it at next reconcile loop. Warning never delete object manually", object.GetDeletionTimestamp(), object.GetNamespace(), object.GetName(), object.GetObjectKind().GroupVersionKind())
}
54 changes: 26 additions & 28 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 @@ -7,41 +7,39 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1"
"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger"
)

// ConfigMap reconciles configmap object
func ConfigMap(ctx context.Context, rclient client.Client, newCM *corev1.ConfigMap, prevCMMEta *metav1.ObjectMeta) error {
var currentCM corev1.ConfigMap
if err := rclient.Get(ctx, types.NamespacedName{Namespace: newCM.Namespace, Name: newCM.Name}, &currentCM); err != nil {
if k8serrors.IsNotFound(err) {
logger.WithContext(ctx).Info(fmt.Sprintf("creating new ConfigMap %s", newCM.Name))
return rclient.Create(ctx, newCM)
func ConfigMap(ctx context.Context, rclient client.Client, newObj *corev1.ConfigMap) (bool, error) {
nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace}
updated := true
err := retryOnConflict(func() error {
var existingObj corev1.ConfigMap
if err := rclient.Get(ctx, nsn, &existingObj); err != nil {
if k8serrors.IsNotFound(err) {
logger.WithContext(ctx).Info(fmt.Sprintf("creating new ConfigMap=%s", nsn))
return rclient.Create(ctx, newObj)
}
return err
}
}
if !currentCM.DeletionTimestamp.IsZero() {
return newErrRecreate(ctx, &currentCM)
}
var prevCM *corev1.ConfigMap
if prevCMMEta != nil {
prevCM = &corev1.ConfigMap{
ObjectMeta: *prevCMMEta,
if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil {
return err
}
}
if equality.Semantic.DeepEqual(newCM.Data, currentCM.Data) &&
isObjectMetaEqual(&currentCM, newCM, prevCM) {
return nil
}

vmv1beta1.AddFinalizer(newCM, &currentCM)
mergeObjectMetadataIntoNew(newCM, &currentCM, prevCM)

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) &&
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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

equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) &&
equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) {
updated = false
return nil
}
existingObj.Labels = newObj.Labels
existingObj.Annotations = newObj.Annotations
existingObj.Data = newObj.Data
logger.WithContext(ctx).Info(fmt.Sprintf("updating ConfigMap=%s", nsn))
return rclient.Update(ctx, &existingObj)
})
return updated, err
}
81 changes: 23 additions & 58 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 @@ -13,78 +13,43 @@ import (
"k8s.io/apimachinery/pkg/util/wait"
"sigs.k8s.io/controller-runtime/pkg/client"

vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1"
"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/finalize"
"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger"
)

// DaemonSet performs an update or create operator for daemonset and waits until it finishes update rollout
func DaemonSet(ctx context.Context, rclient client.Client, newDs, prevDs *appsv1.DaemonSet) error {
var isPrevEqual bool
var prevSpecDiff string
if prevDs != nil {
isPrevEqual = equality.Semantic.DeepDerivative(prevDs.Spec, newDs.Spec)
if !isPrevEqual {
prevSpecDiff = diffDeepDerivative(prevDs.Spec, newDs.Spec)
}
}
rclient.Scheme().Default(newDs)

func DaemonSet(ctx context.Context, rclient client.Client, newObj *appsv1.DaemonSet) error {
rclient.Scheme().Default(newObj)
nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace}
return retryOnConflict(func() error {
var currentDs appsv1.DaemonSet
err := rclient.Get(ctx, types.NamespacedName{Name: newDs.Name, Namespace: newDs.Namespace}, &currentDs)
if err != nil {
var existingObj appsv1.DaemonSet
if err := rclient.Get(ctx, nsn, &existingObj); err != nil {
if k8serrors.IsNotFound(err) {
logger.WithContext(ctx).Info(fmt.Sprintf("creating new DaemonSet %s", newDs.Name))
if err := rclient.Create(ctx, newDs); err != nil {
return fmt.Errorf("cannot create new DaemonSet for app: %s, err: %w", newDs.Name, err)
logger.WithContext(ctx).Info(fmt.Sprintf("creating new DaemonSet=%s", nsn))
if err := rclient.Create(ctx, newObj); err != nil {
return fmt.Errorf("cannot create new DaemonSet=%s: %w", nsn, err)
}
return waitDaemonSetReady(ctx, rclient, newDs, appWaitReadyDeadline)
return waitDaemonSetReady(ctx, rclient, newObj, appWaitReadyDeadline)
}
return fmt.Errorf("cannot get DaemonSet for app: %s err: %w", newDs.Name, err)
}
if !currentDs.DeletionTimestamp.IsZero() {
return newErrRecreate(ctx, &currentDs)
return fmt.Errorf("cannot get DaemonSet=%s: %w", nsn, err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &currentDs); err != nil {
if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil {
return err
}
newDs.Status = currentDs.Status

isEqual := equality.Semantic.DeepDerivative(newDs.Spec, currentDs.Spec)
isEqual := equality.Semantic.DeepDerivative(newObj.Spec, existingObj.Spec)
if isEqual &&
isPrevEqual &&
equality.Semantic.DeepEqual(newDs.Labels, currentDs.Labels) &&
isObjectMetaEqual(&currentDs, newDs, prevDs) {
return waitDaemonSetReady(ctx, rclient, newDs, appWaitReadyDeadline)
}
var prevTemplateAnnotations map[string]string
if prevDs != nil {
prevTemplateAnnotations = prevDs.Annotations
}

vmv1beta1.AddFinalizer(newDs, &currentDs)
newDs.Spec.Template.Annotations = mergeAnnotations(currentDs.Spec.Template.Annotations, newDs.Spec.Template.Annotations, prevTemplateAnnotations)
mergeObjectMetadataIntoNew(&currentDs, newDs, prevDs)

logMsg := fmt.Sprintf("updating DaemonSet %s configuration"+
"is_prev_equal=%v,is_current_equal=%v,is_prev_nil=%v",
newDs.Name, isPrevEqual, isEqual, prevDs == nil)

if len(prevSpecDiff) > 0 {
logMsg += fmt.Sprintf(", prev_spec_diff=%s", prevSpecDiff)
equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) &&
equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) {
return waitDaemonSetReady(ctx, rclient, &existingObj, appWaitReadyDeadline)
}
if !isEqual {
logMsg += fmt.Sprintf(", curr_spec_diff=%s", diffDeepDerivative(newDs.Spec, currentDs.Spec))
specDiff := diffDeepDerivative(newObj.Spec, existingObj.Spec)
existingObj.Spec = newObj.Spec
existingObj.Labels = newObj.Labels
existingObj.Annotations = newObj.Annotations
logger.WithContext(ctx).Info(fmt.Sprintf("updating DaemonSet=%s, spec_diff=%s", nsn, specDiff))
if err := rclient.Update(ctx, &existingObj); err != nil {
return fmt.Errorf("cannot update DaemonSet=%s: %w", nsn, err)
}

logger.WithContext(ctx).Info(logMsg)

if err := rclient.Update(ctx, newDs); err != nil {
return fmt.Errorf("cannot update DaemonSet for app: %s, err: %w", newDs.Name, err)
}

return waitDaemonSetReady(ctx, rclient, newDs, appWaitReadyDeadline)
return waitDaemonSetReady(ctx, rclient, &existingObj, appWaitReadyDeadline)
})
}

Expand Down
71 changes: 35 additions & 36 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 @@ -9,86 +9,85 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"sigs.k8s.io/controller-runtime/pkg/client"

vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1"
"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/finalize"
"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger"
)

// Deployment performs an update or create operator for deployment and waits until it's replicas is ready
func Deployment(ctx context.Context, rclient client.Client, newDeploy, prevDeploy *appsv1.Deployment, hasHPA bool) error {
func Deployment(ctx context.Context, rclient client.Client, newObj, prevObj *appsv1.Deployment, hasHPA bool) error {

var isPrevEqual bool
var prevSpecDiff string
if prevDeploy != nil {
isPrevEqual = equality.Semantic.DeepDerivative(prevDeploy.Spec, newDeploy.Spec)
var prevMeta *metav1.ObjectMeta
if prevObj != nil {
prevMeta = &prevObj.ObjectMeta
isPrevEqual = equality.Semantic.DeepDerivative(prevObj.Spec, newObj.Spec)
if !isPrevEqual {
prevSpecDiff = diffDeepDerivative(prevDeploy.Spec, newDeploy.Spec)
prevSpecDiff = diffDeepDerivative(prevObj.Spec, newObj.Spec)
}
}
rclient.Scheme().Default(newDeploy)
rclient.Scheme().Default(newObj)

nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace}
return retryOnConflict(func() error {
var currentDeploy appsv1.Deployment
err := rclient.Get(ctx, types.NamespacedName{Name: newDeploy.Name, Namespace: newDeploy.Namespace}, &currentDeploy)
if err != nil {
var existingObj appsv1.Deployment
if err := rclient.Get(ctx, nsn, &existingObj); err != nil {
if k8serrors.IsNotFound(err) {
logger.WithContext(ctx).Info(fmt.Sprintf("creating new Deployment %s", newDeploy.Name))
if err := rclient.Create(ctx, newDeploy); err != nil {
return fmt.Errorf("cannot create new deployment for app: %s, err: %w", newDeploy.Name, err)
logger.WithContext(ctx).Info(fmt.Sprintf("creating new Deployment=%s", nsn))
if err := rclient.Create(ctx, newObj); err != nil {
return fmt.Errorf("cannot create new Deployment=%s: %w", nsn, err)
}
return waitDeploymentReady(ctx, rclient, newDeploy, appWaitReadyDeadline)
return waitDeploymentReady(ctx, rclient, newObj, appWaitReadyDeadline)
}
return fmt.Errorf("cannot get deployment for app: %s err: %w", newDeploy.Name, err)
return fmt.Errorf("cannot get Deployment=%s: %w", nsn, err)
}
if !currentDeploy.DeletionTimestamp.IsZero() {
return newErrRecreate(ctx, &currentDeploy)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &currentDeploy); err != nil {
if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil {
return err
}
if hasHPA {
newDeploy.Spec.Replicas = currentDeploy.Spec.Replicas
newObj.Spec.Replicas = existingObj.Spec.Replicas
}
newDeploy.Status = currentDeploy.Status
newObj.Status = existingObj.Status
var prevTemplateAnnotations map[string]string
if prevDeploy != nil {
prevTemplateAnnotations = prevDeploy.Spec.Template.Annotations
if prevObj != nil {
prevTemplateAnnotations = prevObj.Spec.Template.Annotations
}
isEqual := equality.Semantic.DeepDerivative(newDeploy.Spec, currentDeploy.Spec)
isEqual := equality.Semantic.DeepDerivative(newObj.Spec, existingObj.Spec)
if isEqual &&
isPrevEqual &&
equality.Semantic.DeepEqual(newDeploy.Labels, currentDeploy.Labels) &&
isObjectMetaEqual(&currentDeploy, newDeploy, prevDeploy) {
return waitDeploymentReady(ctx, rclient, newDeploy, appWaitReadyDeadline)
equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) &&
isObjectMetaEqual(&existingObj, newObj, prevMeta) {
return waitDeploymentReady(ctx, rclient, newObj, appWaitReadyDeadline)
}

vmv1beta1.AddFinalizer(newDeploy, &currentDeploy)
newDeploy.Spec.Template.Annotations = mergeAnnotations(currentDeploy.Spec.Template.Annotations, newDeploy.Spec.Template.Annotations, prevTemplateAnnotations)
mergeObjectMetadataIntoNew(&currentDeploy, newDeploy, prevDeploy)
vmv1beta1.AddFinalizer(newObj, &existingObj)
newObj.Spec.Template.Annotations = mergeAnnotations(existingObj.Spec.Template.Annotations, newObj.Spec.Template.Annotations, prevTemplateAnnotations)
mergeObjectMetadataIntoNew(&existingObj, newObj, prevMeta)

logMsg := fmt.Sprintf("updating Deployment %s configuration"+
logMsg := fmt.Sprintf("updating Deployment=%s configuration"+
"is_prev_equal=%v,is_current_equal=%v,is_prev_nil=%v",
newDeploy.Name, isPrevEqual, isEqual, prevDeploy == nil)
nsn, isPrevEqual, isEqual, prevObj == nil)

if len(prevSpecDiff) > 0 {
logMsg += fmt.Sprintf(", prev_spec_diff=%s", prevSpecDiff)
}
if !isEqual {
logMsg += fmt.Sprintf(", curr_spec_diff=%s", diffDeepDerivative(newDeploy.Spec, currentDeploy.Spec))
logMsg += fmt.Sprintf(", curr_spec_diff=%s", diffDeepDerivative(newObj.Spec, existingObj.Spec))
}

logger.WithContext(ctx).Info(logMsg)

if err := rclient.Update(ctx, newDeploy); err != nil {
return fmt.Errorf("cannot update deployment for app: %s, err: %w", newDeploy.Name, err)
if err := rclient.Update(ctx, newObj); err != nil {
return fmt.Errorf("cannot update Deployment=%s: %w", nsn, err)
}

return waitDeploymentReady(ctx, rclient, newDeploy, appWaitReadyDeadline)
return waitDeploymentReady(ctx, rclient, newObj, appWaitReadyDeadline)
})
}

Expand Down Expand Up @@ -166,6 +165,6 @@ func reportFirstNotReadyPodOnError(ctx context.Context, rclient client.Client, o
return podStatusesToError(origin, &dp)
}
return &errWaitReady{
origin: fmt.Errorf("cannot find any pod for selector=%q, check kubernetes events, origin err: %w", selector.String(), origin),
origin: fmt.Errorf("cannot find any pod for selector=%q, check kubernetes events: %w", selector.String(), origin),
}
}
40 changes: 17 additions & 23 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 @@ -10,40 +10,34 @@ import (
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/finalize"
"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger"
)

// HPA creates or update horizontalPodAutoscaler object
func HPA(ctx context.Context, rclient client.Client, newHPA, prevHPA *v2.HorizontalPodAutoscaler) error {
func HPA(ctx context.Context, rclient client.Client, newObj *v2.HorizontalPodAutoscaler) error {
nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace}
return retryOnConflict(func() error {
var currentHPA v2.HorizontalPodAutoscaler
if err := rclient.Get(ctx, types.NamespacedName{Name: newHPA.GetName(), Namespace: newHPA.GetNamespace()}, &currentHPA); err != nil {
var existingObj v2.HorizontalPodAutoscaler
if err := rclient.Get(ctx, nsn, &existingObj); err != nil {
if k8serrors.IsNotFound(err) {
logger.WithContext(ctx).Info(fmt.Sprintf("creating HPA %s configuration", newHPA.Name))
return rclient.Create(ctx, newHPA)
logger.WithContext(ctx).Info(fmt.Sprintf("creating HPA=%s configuration", nsn))
return rclient.Create(ctx, newObj)
}
return fmt.Errorf("cannot get exist hpa object: %w", err)
return fmt.Errorf("cannot get existing HPA=%s: %w", nsn, err)
}
if !currentHPA.DeletionTimestamp.IsZero() {
return newErrRecreate(ctx, &currentHPA)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &currentHPA); err != nil {
if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil {
return err
}

if equality.Semantic.DeepEqual(newHPA.Spec, currentHPA.Spec) &&
equality.Semantic.DeepEqual(newHPA.Labels, currentHPA.Labels) &&
isObjectMetaEqual(&currentHPA, newHPA, prevHPA) {
if equality.Semantic.DeepEqual(newObj.Spec, existingObj.Spec) &&
equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) &&
equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) {
return nil
}

mergeObjectMetadataIntoNew(&currentHPA, newHPA, prevHPA)
newHPA.Status = currentHPA.Status

logMsg := fmt.Sprintf("updating HPA %s configuration spec_diff: %s", newHPA.Name, diffDeepDerivative(newHPA.Spec, currentHPA.Spec))
logger.WithContext(ctx).Info(logMsg)

return rclient.Update(ctx, newHPA)
specDiff := diffDeepDerivative(newObj.Spec, existingObj.Spec)
existingObj.Labels = newObj.Labels
existingObj.Annotations = newObj.Annotations
existingObj.Spec = newObj.Spec
logger.WithContext(ctx).Info(fmt.Sprintf("updating HPA=%s, spec_diff: %s", nsn, specDiff))
return rclient.Update(ctx, &existingObj)
})
}
Loading