Skip to content
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.

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

controllers/finalize: Remove finalizer for child objects for non zero DeletionTimestamp #956

Merged
merged 3 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions controllers/factory/alertmanager/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,9 @@ func createDefaultAMConfig(ctx context.Context, cr *victoriametricsv1beta1.VMAle
}
return err
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existAMSecretConfig); err != nil {
return err
}

newAMSecretConfig.Annotations = labels.Merge(existAMSecretConfig.Annotations, newAMSecretConfig.Annotations)
newAMSecretConfig.Finalizers = victoriametricsv1beta1.MergeFinalizers(&existAMSecretConfig, victoriametricsv1beta1.FinalizerName)
Expand Down
12 changes: 12 additions & 0 deletions controllers/factory/finalize/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,15 @@ func removeConfigReloaderRole(ctx context.Context, rclient client.Client, crd cr
}
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())
}
4 changes: 4 additions & 0 deletions controllers/factory/reconcile/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"time"

victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -30,6 +31,9 @@ func Deployment(ctx context.Context, rclient client.Client, newDeploy *appsv1.De
}
return fmt.Errorf("cannot get deployment for app: %s err: %w", newDeploy.Name, err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &currentDeploy); err != nil {
return err
}
if hasHPA {
newDeploy.Spec.Replicas = currentDeploy.Spec.Replicas
}
Expand Down
4 changes: 4 additions & 0 deletions controllers/factory/reconcile/hpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
"github.com/VictoriaMetrics/operator/controllers/factory/k8stools"

victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
Expand All @@ -24,6 +25,9 @@ func HPA(ctx context.Context, rclient client.Client, targetHPA client.Object) er
}
return fmt.Errorf("cannot get exist hpa object: %w", err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, existHPA); err != nil {
return err
}
targetHPA.SetResourceVersion(existHPA.GetResourceVersion())
victoriametricsv1beta1.MergeFinalizers(targetHPA, victoriametricsv1beta1.FinalizerName)
targetHPA.SetAnnotations(labels.Merge(existHPA.GetAnnotations(), targetHPA.GetAnnotations()))
Expand Down
4 changes: 4 additions & 0 deletions controllers/factory/reconcile/pdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
"github.com/VictoriaMetrics/operator/controllers/factory/logger"
policyv1 "k8s.io/api/policy/v1"
"k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -26,6 +27,9 @@ func PDB(ctx context.Context, rclient client.Client, pdb *policyv1.PodDisruption
}
return fmt.Errorf("cannot get existing pdb: %s, err: %w", pdb.Name, err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, currentPdb); err != nil {
return err
}
pdb.Annotations = labels.Merge(currentPdb.Annotations, pdb.Annotations)
if currentPdb.ResourceVersion != "" {
pdb.ResourceVersion = currentPdb.ResourceVersion
Expand Down
3 changes: 3 additions & 0 deletions controllers/factory/reconcile/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ func reconcileService(ctx context.Context, rclient client.Client, newService *v1
}
return fmt.Errorf("cannot get service for existing service: %w", err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, existingService); err != nil {
return err
}
// lets save annotations and labels even after recreation.
if newService.Spec.Type != existingService.Spec.Type {
// type mismatch.
Expand Down
4 changes: 4 additions & 0 deletions controllers/factory/reconcile/service_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/labels"
Expand All @@ -23,6 +24,9 @@ func ServiceAccount(ctx context.Context, rclient client.Client, sa *v1.ServiceAc
}
return fmt.Errorf("cannot get ServiceAccount for given CRD Object=%q, err=%w", sa.Name, err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existSA); err != nil {
return err
}

existSA.OwnerReferences = sa.OwnerReferences
existSA.Finalizers = victoriametricsv1beta1.MergeFinalizers(&existSA, victoriametricsv1beta1.FinalizerName)
Expand Down
4 changes: 4 additions & 0 deletions controllers/factory/reconcile/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
"github.com/VictoriaMetrics/operator/controllers/factory/logger"
"github.com/VictoriaMetrics/operator/internal/config"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -69,6 +70,9 @@ func HandleSTSUpdate(ctx context.Context, rclient client.Client, cr STSOptions,
}
return fmt.Errorf("cannot get sts %s under namespace %s: %w", newSts.Name, newSts.Namespace, err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &currentSts); err != nil {
return err
}
// will update the original cr replicaCount to propagate right num,
// for now, it's only used in vmselect
if cr.UpdateReplicaCount != nil {
Expand Down
4 changes: 4 additions & 0 deletions controllers/factory/reconcile/vmservicescrape.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -22,6 +23,9 @@ func VMServiceScrapeForCRD(ctx context.Context, rclient client.Client, vss *vict
}
return err
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existVSS); err != nil {
return err
}
updateIsNeeded := !equality.Semantic.DeepEqual(vss.Spec, existVSS.Spec) || !equality.Semantic.DeepEqual(vss.Labels, existVSS.Labels) || !equality.Semantic.DeepEqual(vss.Annotations, existVSS.Annotations)
existVSS.Spec = vss.Spec
existVSS.Labels = vss.Labels
Expand Down
13 changes: 13 additions & 0 deletions controllers/factory/vmagent/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

v1beta12 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
rbacV1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -143,6 +144,9 @@ func ensureVMAgentCRExist(ctx context.Context, cr *v1beta12.VMAgent, rclient cli
}
return fmt.Errorf("cannot get exist cluster role for vmagent: %w", err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existsClusterRole); err != nil {
return err
}

existsClusterRole.OwnerReferences = clusterRole.OwnerReferences
existsClusterRole.Labels = clusterRole.Labels
Expand All @@ -162,6 +166,9 @@ func ensureVMAgentCRBExist(ctx context.Context, cr *v1beta12.VMAgent, rclient cl
}
return fmt.Errorf("cannot get clusterRoleBinding for vmagent: %w", err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existsClusterRoleBinding); err != nil {
return err
}

existsClusterRoleBinding.OwnerReferences = clusterRoleBinding.OwnerReferences
existsClusterRoleBinding.Labels = clusterRoleBinding.Labels
Expand Down Expand Up @@ -221,6 +228,9 @@ func ensureVMAgentRExist(ctx context.Context, cr *v1beta12.VMAgent, rclient clie
}
return fmt.Errorf("cannot get exist cluster role for vmagent: %w", err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existsClusterRole); err != nil {
return err
}

existsClusterRole.OwnerReferences = nr.OwnerReferences
existsClusterRole.Labels = nr.Labels
Expand All @@ -240,6 +250,9 @@ func ensureVMAgentRBExist(ctx context.Context, cr *v1beta12.VMAgent, rclient cli
}
return fmt.Errorf("cannot get rb for vmagent: %w", err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existsRB); err != nil {
return err
}

existsRB.OwnerReferences = rb.OwnerReferences
existsRB.Labels = rb.Labels
Expand Down
3 changes: 3 additions & 0 deletions controllers/factory/vmagent/vmagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,9 @@ func createOrUpdateTLSAssets(ctx context.Context, cr *victoriametricsv1beta1.VMA
}
return fmt.Errorf("cannot get existing tls secret: %s, for vmagent: %s, err: %w", tlsAssetsSecret.Name, cr.Name, err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, currentAssetSecret); err != nil {
return err
}
tlsAssetsSecret.Annotations = labels.Merge(currentAssetSecret.Annotations, tlsAssetsSecret.Annotations)
victoriametricsv1beta1.MergeFinalizers(tlsAssetsSecret, victoriametricsv1beta1.FinalizerName)
return rclient.Update(ctx, tlsAssetsSecret)
Expand Down
5 changes: 5 additions & 0 deletions controllers/factory/vmagent/vmagent_scrapeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/utils"
"github.com/VictoriaMetrics/metricsql"
victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
"github.com/VictoriaMetrics/operator/controllers/factory/k8stools"
"github.com/VictoriaMetrics/operator/controllers/factory/logger"
"github.com/VictoriaMetrics/operator/internal/config"
Expand Down Expand Up @@ -154,6 +155,10 @@ func createOrUpdateConfigurationSecret(ctx context.Context, cr *victoriametricsv
return nil, fmt.Errorf("cannot get secret for vmagent: %q : %w", cr.Name, err)
}

if err := finalize.FreeIfNeeded(ctx, rclient, curSecret); err != nil {
return nil, err
}

s.Annotations = labels.Merge(curSecret.Annotations, s.Annotations)
s.Finalizers = victoriametricsv1beta1.MergeFinalizers(curSecret, victoriametricsv1beta1.FinalizerName)
return ssCache, rclient.Update(ctx, s)
Expand Down
15 changes: 8 additions & 7 deletions controllers/factory/vmalert/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"

victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
"github.com/VictoriaMetrics/operator/controllers/factory/k8stools"
"github.com/VictoriaMetrics/operator/controllers/factory/logger"
"github.com/ghodss/yaml"
Expand Down Expand Up @@ -130,20 +131,20 @@ func CreateOrUpdateRuleConfigMaps(ctx context.Context, cr *victoriametricsv1beta
}
}
for _, cm := range toUpdate {
if err := finalize.FreeIfNeeded(ctx, rclient, &cm); err != nil {
return nil, err
}
err = rclient.Update(ctx, &cm)
if err != nil {
return nil, fmt.Errorf("failed to update rules Configmap: %s, err: %w", cm.Name, err)
}
}
for _, cm := range toDelete {
if victoriametricsv1beta1.IsContainsFinalizer(cm.Finalizers, victoriametricsv1beta1.FinalizerName) {
cm.Finalizers = victoriametricsv1beta1.RemoveFinalizer(cm.Finalizers, victoriametricsv1beta1.FinalizerName)
if err := rclient.Update(ctx, &cm); err != nil {
return nil, fmt.Errorf("cannot remove finalizer from configmap: %s, err: %w", cm.Name, err)
}
if err := finalize.RemoveFinalizer(ctx, rclient, &cm); err != nil {
return nil, fmt.Errorf("cannot remove finalizer for vmalert cm: %w", err)
}
err = rclient.Delete(ctx, &cm)
if err != nil {

if err := finalize.SafeDelete(ctx, rclient, &cm); err != nil {
return nil, fmt.Errorf("failed to delete rules Configmap: %s, err: %w", cm.Name, err)
}
}
Expand Down
6 changes: 6 additions & 0 deletions controllers/factory/vmalert/vmalert.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ func createOrUpdateVMAlertSecret(ctx context.Context, rclient client.Client, cr
}
return nil
}
if err := finalize.FreeIfNeeded(ctx, rclient, curSecret); err != nil {
return err
}
s.Annotations = labels.Merge(curSecret.Annotations, s.Annotations)
return rclient.Update(ctx, s)
}
Expand Down Expand Up @@ -739,6 +742,9 @@ func createOrUpdateTLSAssetsForVMAlert(ctx context.Context, cr *victoriametricsv
}
return fmt.Errorf("cannot get existing tls secret: %s, for vmalert: %s, err: %w", tlsAssetsSecret.Name, cr.Name, err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, currentAssetSecret); err != nil {
return err
}
for annotation, value := range currentAssetSecret.Annotations {
tlsAssetsSecret.Annotations[annotation] = value
}
Expand Down
7 changes: 7 additions & 0 deletions controllers/factory/vmauth/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

v1beta12 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/finalize"
v1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -33,6 +34,9 @@ func ensureVMAuthRoleExist(ctx context.Context, cr *v1beta12.VMAuth, rclient cli
}
return fmt.Errorf("cannot get role for vmauth: %w", err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existRole); err != nil {
return err
}

existRole.OwnerReferences = role.OwnerReferences
existRole.Labels = role.Labels
Expand All @@ -51,6 +55,9 @@ func ensureVMAgentRBExist(ctx context.Context, cr *v1beta12.VMAuth, rclient clie
}
return fmt.Errorf("cannot get rolebinding for vmauth: %w", err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existRoleBinding); err != nil {
return err
}

existRoleBinding.OwnerReferences = roleBinding.OwnerReferences
existRoleBinding.Labels = roleBinding.Labels
Expand Down
6 changes: 6 additions & 0 deletions controllers/factory/vmauth/vmauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,9 @@ func CreateOrUpdateVMAuthConfig(ctx context.Context, rclient client.Client, cr *
}
return err
}
if err := finalize.FreeIfNeeded(ctx, rclient, &curSecret); err != nil {
return err
}
var (
generatedConf = s.Data[vmAuthConfigNameGz]
curConfig, curConfigFound = curSecret.Data[vmAuthConfigNameGz]
Expand Down Expand Up @@ -414,6 +417,9 @@ func CreateOrUpdateVMAuthIngress(ctx context.Context, rclient client.Client, cr
}
return err
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existIngress); err != nil {
return err
}
newIngress.Annotations = labels.Merge(existIngress.Annotations, newIngress.Annotations)
newIngress.Finalizers = victoriametricsv1beta1.MergeFinalizers(&existIngress, victoriametricsv1beta1.FinalizerName)
return rclient.Update(ctx, newIngress)
Expand Down
8 changes: 7 additions & 1 deletion controllers/factory/vmsingle/vmsingle.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ func CreateVMSingleStorage(ctx context.Context, cr *victoriametricsv1beta1.VMSin
if err := rclient.Create(ctx, newPvc); err != nil {
return fmt.Errorf("cannot create new pvc for vmsingle: %w", err)
}

if !existPvc.DeletionTimestamp.IsZero() {
f41gh7 marked this conversation as resolved.
Show resolved Hide resolved
l.Info("exist pvc for vmsingle has non zero DeletionTimestamp. Ignore it or make backup, delete VMSingle object and restore it from backup.")
}
return nil
}
return fmt.Errorf("cannot get existing pvc for vmsingle: %w", err)
Expand Down Expand Up @@ -447,6 +449,10 @@ func CreateOrUpdateVMSingleStreamAggrConfig(ctx context.Context, cr *victoriamet
if errors.IsNotFound(err) {
return rclient.Create(ctx, streamAggrCM)
}
return fmt.Errorf("cannot fetch exist configmap for vmsingle streamAggr: %w", err)
}
if err := finalize.FreeIfNeeded(ctx, rclient, &existCM); err != nil {
return err
}
streamAggrCM.Annotations = labels.Merge(existCM.Annotations, streamAggrCM.Annotations)
victoriametricsv1beta1.MergeFinalizers(streamAggrCM, victoriametricsv1beta1.FinalizerName)
Expand Down
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ aliases:
## Next release

- [vmalertmanager](./api.md#vmalertmanager): ignores content of `cr.spec.configSecret` if it's name clashes with secret used by operator for storing alertmanager config. See this [issue](https://github.com/VictoriaMetrics/operator/issues/954) for details.
- [operator](./README.md): remove finalizer for child objects with non-empty `DeletetionTimestamp`. See this [issue](https://github.com/VictoriaMetrics/operator/issues/953) for details.
- [operator](./README.md): skip storageClass check if there is no PVC size change. See this [issue](https://github.com/VictoriaMetrics/operator/issues/957) for details.

## [v0.44.0](https://github.com/VictoriaMetrics/operator/releases/tag/v0.44.0) - 9 May 2024
Expand Down
Loading