diff --git a/controllers/factory/alertmanager/statefulset.go b/controllers/factory/alertmanager/statefulset.go index 37cc0ccb..7954aa3f 100644 --- a/controllers/factory/alertmanager/statefulset.go +++ b/controllers/factory/alertmanager/statefulset.go @@ -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) diff --git a/controllers/factory/finalize/common.go b/controllers/factory/finalize/common.go index 4bb9ec50..bd4ed121 100644 --- a/controllers/factory/finalize/common.go +++ b/controllers/factory/finalize/common.go @@ -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()) +} diff --git a/controllers/factory/reconcile/deploy.go b/controllers/factory/reconcile/deploy.go index 2c47182e..c340fcb9 100644 --- a/controllers/factory/reconcile/deploy.go +++ b/controllers/factory/reconcile/deploy.go @@ -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" @@ -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, ¤tDeploy); err != nil { + return err + } if hasHPA { newDeploy.Spec.Replicas = currentDeploy.Spec.Replicas } diff --git a/controllers/factory/reconcile/hpa.go b/controllers/factory/reconcile/hpa.go index 3f654bcb..059e6fde 100644 --- a/controllers/factory/reconcile/hpa.go +++ b/controllers/factory/reconcile/hpa.go @@ -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" @@ -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())) diff --git a/controllers/factory/reconcile/pdb.go b/controllers/factory/reconcile/pdb.go index 48327d90..09a2257c 100644 --- a/controllers/factory/reconcile/pdb.go +++ b/controllers/factory/reconcile/pdb.go @@ -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" @@ -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 diff --git a/controllers/factory/reconcile/service.go b/controllers/factory/reconcile/service.go index 02ca11cb..911ad18c 100644 --- a/controllers/factory/reconcile/service.go +++ b/controllers/factory/reconcile/service.go @@ -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. diff --git a/controllers/factory/reconcile/service_account.go b/controllers/factory/reconcile/service_account.go index c51d20c5..2ae55eec 100644 --- a/controllers/factory/reconcile/service_account.go +++ b/controllers/factory/reconcile/service_account.go @@ -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" @@ -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) diff --git a/controllers/factory/reconcile/statefulset.go b/controllers/factory/reconcile/statefulset.go index 10bfe550..88dd8e3d 100644 --- a/controllers/factory/reconcile/statefulset.go +++ b/controllers/factory/reconcile/statefulset.go @@ -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" @@ -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, ¤tSts); 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 { diff --git a/controllers/factory/reconcile/vmservicescrape.go b/controllers/factory/reconcile/vmservicescrape.go index cacd7455..fee54636 100644 --- a/controllers/factory/reconcile/vmservicescrape.go +++ b/controllers/factory/reconcile/vmservicescrape.go @@ -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" @@ -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 diff --git a/controllers/factory/vmagent/rbac.go b/controllers/factory/vmagent/rbac.go index 6eb5b91c..45f19d7a 100644 --- a/controllers/factory/vmagent/rbac.go +++ b/controllers/factory/vmagent/rbac.go @@ -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" @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/controllers/factory/vmagent/vmagent.go b/controllers/factory/vmagent/vmagent.go index cbfd1842..1b2e4b1c 100644 --- a/controllers/factory/vmagent/vmagent.go +++ b/controllers/factory/vmagent/vmagent.go @@ -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) diff --git a/controllers/factory/vmagent/vmagent_scrapeconfig.go b/controllers/factory/vmagent/vmagent_scrapeconfig.go index 701b9927..1e9d7f91 100644 --- a/controllers/factory/vmagent/vmagent_scrapeconfig.go +++ b/controllers/factory/vmagent/vmagent_scrapeconfig.go @@ -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" @@ -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) diff --git a/controllers/factory/vmalert/rules.go b/controllers/factory/vmalert/rules.go index c579cc92..fe8151a5 100644 --- a/controllers/factory/vmalert/rules.go +++ b/controllers/factory/vmalert/rules.go @@ -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" @@ -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) } } diff --git a/controllers/factory/vmalert/vmalert.go b/controllers/factory/vmalert/vmalert.go index 7f06b1e7..f3a603a8 100644 --- a/controllers/factory/vmalert/vmalert.go +++ b/controllers/factory/vmalert/vmalert.go @@ -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) } @@ -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 } diff --git a/controllers/factory/vmauth/rbac.go b/controllers/factory/vmauth/rbac.go index eb800f67..cf289880 100644 --- a/controllers/factory/vmauth/rbac.go +++ b/controllers/factory/vmauth/rbac.go @@ -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" @@ -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 @@ -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 diff --git a/controllers/factory/vmauth/vmauth.go b/controllers/factory/vmauth/vmauth.go index d24a42d8..4be00ba7 100644 --- a/controllers/factory/vmauth/vmauth.go +++ b/controllers/factory/vmauth/vmauth.go @@ -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] @@ -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) diff --git a/controllers/factory/vmsingle/vmsingle.go b/controllers/factory/vmsingle/vmsingle.go index 3cfcdd4b..ee3147b8 100644 --- a/controllers/factory/vmsingle/vmsingle.go +++ b/controllers/factory/vmsingle/vmsingle.go @@ -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() { + 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) @@ -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) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 83c6d270..fc0a454a 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -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