From da40408cae22280b27bf43a09f08d9f9b5acc34a Mon Sep 17 00:00:00 2001 From: Andrii Chubatiuk Date: Sat, 31 Jan 2026 13:55:19 +0200 Subject: [PATCH] reconcile: updated recreation logic --- docs/CHANGELOG.md | 1 + .../operator/factory/finalize/common.go | 12 -- .../operator/factory/reconcile/configmap.go | 54 ++++--- .../operator/factory/reconcile/daemonset.go | 81 +++------- .../operator/factory/reconcile/deploy.go | 71 +++++---- .../operator/factory/reconcile/hpa.go | 40 ++--- .../operator/factory/reconcile/httproute.go | 49 ++---- .../operator/factory/reconcile/ingress.go | 45 ++++++ .../operator/factory/reconcile/pdb.go | 40 ++--- .../operator/factory/reconcile/pvc.go | 24 +-- .../operator/factory/reconcile/rbac.go | 147 +++++++++--------- .../operator/factory/reconcile/reconcile.go | 31 ++-- .../operator/factory/reconcile/secret.go | 58 +++---- .../operator/factory/reconcile/service.go | 111 +++++++------ .../factory/reconcile/service_account.go | 38 ++--- .../operator/factory/reconcile/statefulset.go | 113 +++++++------- .../reconcile/statefulset_pvc_expand.go | 15 +- .../factory/reconcile/statefulset_test.go | 18 ++- .../operator/factory/reconcile/vmscrape.go | 87 +++++++++++ .../factory/reconcile/vmservicescrape.go | 93 ----------- .../operator/factory/vlagent/vlagent.go | 18 +-- .../operator/factory/vlcluster/vlcluster.go | 7 +- .../operator/factory/vlcluster/vlinsert.go | 23 +-- .../operator/factory/vlcluster/vlselect.go | 20 +-- .../operator/factory/vlcluster/vlstorage.go | 21 +-- .../operator/factory/vlcluster/vmauth_lb.go | 19 +-- .../operator/factory/vlsingle/vlogs.go | 17 +- .../operator/factory/vlsingle/vlsingle.go | 17 +- .../operator/factory/vmagent/vmagent.go | 43 ++--- .../factory/vmagent/vmagent_scrapeconfig.go | 15 +- .../vmagent/vmagent_scrapeconfig_test.go | 4 +- .../operator/factory/vmagent/vmagent_test.go | 4 +- .../operator/factory/vmalert/rules.go | 104 ++----------- .../operator/factory/vmalert/rules_test.go | 138 ---------------- .../operator/factory/vmalert/vmalert.go | 22 +-- .../factory/vmalertmanager/alertmanager.go | 12 +- .../factory/vmalertmanager/statefulset.go | 13 +- .../operator/factory/vmanomaly/config.go | 17 +- .../operator/factory/vmanomaly/statefulset.go | 15 +- .../factory/vmanomaly/statefulset_test.go | 2 +- .../operator/factory/vmauth/vmauth.go | 66 ++------ .../operator/factory/vmcluster/vmcluster.go | 94 +++-------- .../operator/factory/vmsingle/vmsingle.go | 29 +--- .../operator/factory/vtcluster/cluster.go | 7 +- .../operator/factory/vtcluster/insert.go | 23 +-- .../operator/factory/vtcluster/select.go | 19 +-- .../operator/factory/vtcluster/storage.go | 21 +-- .../operator/factory/vtcluster/vmauth_lb.go | 18 +-- .../operator/factory/vtsingle/vtsingle.go | 17 +- 49 files changed, 674 insertions(+), 1279 deletions(-) create mode 100644 internal/controller/operator/factory/reconcile/ingress.go create mode 100644 internal/controller/operator/factory/reconcile/vmscrape.go delete mode 100644 internal/controller/operator/factory/reconcile/vmservicescrape.go diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 24aa9c930..9e263d623 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -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 diff --git a/internal/controller/operator/factory/finalize/common.go b/internal/controller/operator/factory/finalize/common.go index 8dde29aba..a55cb395a 100644 --- a/internal/controller/operator/factory/finalize/common.go +++ b/internal/controller/operator/factory/finalize/common.go @@ -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()) -} diff --git a/internal/controller/operator/factory/reconcile/configmap.go b/internal/controller/operator/factory/reconcile/configmap.go index f6d8b4c64..4468763dd 100644 --- a/internal/controller/operator/factory/reconcile/configmap.go +++ b/internal/controller/operator/factory/reconcile/configmap.go @@ -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}, ¤tCM); 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, ¤tCM) - } - 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(¤tCM, newCM, prevCM) { - return nil - } - - vmv1beta1.AddFinalizer(newCM, ¤tCM) - mergeObjectMetadataIntoNew(newCM, ¤tCM, 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) && + 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 } diff --git a/internal/controller/operator/factory/reconcile/daemonset.go b/internal/controller/operator/factory/reconcile/daemonset.go index 58d6f7a6d..d7403369d 100644 --- a/internal/controller/operator/factory/reconcile/daemonset.go +++ b/internal/controller/operator/factory/reconcile/daemonset.go @@ -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}, ¤tDs) - 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, ¤tDs) + return fmt.Errorf("cannot get DaemonSet=%s: %w", nsn, err) } - if err := finalize.FreeIfNeeded(ctx, rclient, ¤tDs); 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(¤tDs, newDs, prevDs) { - return waitDaemonSetReady(ctx, rclient, newDs, appWaitReadyDeadline) - } - var prevTemplateAnnotations map[string]string - if prevDs != nil { - prevTemplateAnnotations = prevDs.Annotations - } - - vmv1beta1.AddFinalizer(newDs, ¤tDs) - newDs.Spec.Template.Annotations = mergeAnnotations(currentDs.Spec.Template.Annotations, newDs.Spec.Template.Annotations, prevTemplateAnnotations) - mergeObjectMetadataIntoNew(¤tDs, 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) }) } diff --git a/internal/controller/operator/factory/reconcile/deploy.go b/internal/controller/operator/factory/reconcile/deploy.go index 027714141..5e05d6210 100644 --- a/internal/controller/operator/factory/reconcile/deploy.go +++ b/internal/controller/operator/factory/reconcile/deploy.go @@ -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}, ¤tDeploy) - 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, ¤tDeploy) - } - if err := finalize.FreeIfNeeded(ctx, rclient, ¤tDeploy); 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(¤tDeploy, 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, ¤tDeploy) - newDeploy.Spec.Template.Annotations = mergeAnnotations(currentDeploy.Spec.Template.Annotations, newDeploy.Spec.Template.Annotations, prevTemplateAnnotations) - mergeObjectMetadataIntoNew(¤tDeploy, 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) }) } @@ -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), } } diff --git a/internal/controller/operator/factory/reconcile/hpa.go b/internal/controller/operator/factory/reconcile/hpa.go index cbc24ac22..83a1e73bd 100644 --- a/internal/controller/operator/factory/reconcile/hpa.go +++ b/internal/controller/operator/factory/reconcile/hpa.go @@ -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()}, ¤tHPA); 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, ¤tHPA) - } - if err := finalize.FreeIfNeeded(ctx, rclient, ¤tHPA); 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(¤tHPA, 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(¤tHPA, 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) }) } diff --git a/internal/controller/operator/factory/reconcile/httproute.go b/internal/controller/operator/factory/reconcile/httproute.go index c0d7fbfe5..68b534f20 100644 --- a/internal/controller/operator/factory/reconcile/httproute.go +++ b/internal/controller/operator/factory/reconcile/httproute.go @@ -10,50 +10,35 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" - "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/finalize" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger" ) // HTTPRoute creates or updates HTTPRoute object -func HTTPRoute(ctx context.Context, rclient client.Client, newHTTPRoute, prevHTTPRoute *gwapiv1.HTTPRoute) error { +func HTTPRoute(ctx context.Context, rclient client.Client, newObj *gwapiv1.HTTPRoute) error { + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} return retryOnConflict(func() error { - var curHTTPRoute gwapiv1.HTTPRoute - var isPrevEqual bool - - if err := rclient.Get(ctx, types.NamespacedName{Name: newHTTPRoute.GetName(), Namespace: newHTTPRoute.GetNamespace()}, &curHTTPRoute); err != nil { + var existingObj gwapiv1.HTTPRoute + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating HTTPRoute %s configuration", newHTTPRoute.Name)) - return rclient.Create(ctx, newHTTPRoute) + logger.WithContext(ctx).Info(fmt.Sprintf("creating HTTPRoute=%s", nsn)) + return rclient.Create(ctx, newObj) } - return fmt.Errorf("cannot get existing HTTPRoute object: %w", err) - } - if !curHTTPRoute.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, &curHTTPRoute) + return fmt.Errorf("cannot get existing HTTPRoute=%s: %w", nsn, err) } - if err := finalize.FreeIfNeeded(ctx, rclient, &curHTTPRoute); err != nil { + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { return err } - - // use DeepDerivative compare since gatewayapi controller sets default fields to the parentRefs fields - isEqual := equality.Semantic.DeepDerivative(newHTTPRoute.Spec, curHTTPRoute.Spec) - if prevHTTPRoute != nil { - isPrevEqual = equality.Semantic.DeepDerivative(prevHTTPRoute.Spec, newHTTPRoute.Spec) - } + isEqual := equality.Semantic.DeepDerivative(newObj.Spec, existingObj.Spec) if isEqual && - isPrevEqual && - isObjectMetaEqual(&curHTTPRoute, newHTTPRoute, prevHTTPRoute) { + equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) && + equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) { return nil } - - mergeObjectMetadataIntoNew(&curHTTPRoute, newHTTPRoute, prevHTTPRoute) - newHTTPRoute.Status = curHTTPRoute.Status - - logMsg := fmt.Sprintf("updating HTTPRoute %s configuration spec_diff: %s"+ - "is_prev_equal=%v,is_current_equal=%v,is_prev_nil=%v", - newHTTPRoute.Name, diffDeepDerivative(newHTTPRoute.Spec, curHTTPRoute.Spec), isPrevEqual, isEqual, prevHTTPRoute == nil) - - logger.WithContext(ctx).Info(logMsg) - - return rclient.Update(ctx, newHTTPRoute) + 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 HTTPRoute=%s, spec_diff=%s", nsn, specDiff)) + return rclient.Update(ctx, &existingObj) }) } diff --git a/internal/controller/operator/factory/reconcile/ingress.go b/internal/controller/operator/factory/reconcile/ingress.go new file mode 100644 index 000000000..1524b85e2 --- /dev/null +++ b/internal/controller/operator/factory/reconcile/ingress.go @@ -0,0 +1,45 @@ +package reconcile + +import ( + "context" + "fmt" + + networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/api/equality" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger" +) + +// Ingress creates or updates Ingress object +func Ingress(ctx context.Context, rclient client.Client, newObj *networkingv1.Ingress) error { + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} + return retryOnConflict(func() error { + var existingObj networkingv1.Ingress + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { + if k8serrors.IsNotFound(err) { + logger.WithContext(ctx).Info(fmt.Sprintf("creating Ingress=%s", nsn)) + return rclient.Create(ctx, newObj) + } + return fmt.Errorf("cannot get existing Ingress=%s: %w", nsn, err) + } + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { + return err + } + + isEqual := equality.Semantic.DeepEqual(newObj.Spec, existingObj.Spec) + if isEqual && + equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) && + equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) { + return nil + } + 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 Ingress=%s, spec_diff=%s", nsn, specDiff)) + return rclient.Update(ctx, &existingObj) + }) +} diff --git a/internal/controller/operator/factory/reconcile/pdb.go b/internal/controller/operator/factory/reconcile/pdb.go index bfca3e64c..9a8d6a604 100644 --- a/internal/controller/operator/factory/reconcile/pdb.go +++ b/internal/controller/operator/factory/reconcile/pdb.go @@ -10,41 +10,35 @@ 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" ) // PDB creates or updates PodDisruptionBudget -func PDB(ctx context.Context, rclient client.Client, newPDB, prevPDB *policyv1.PodDisruptionBudget) error { +func PDB(ctx context.Context, rclient client.Client, newObj *policyv1.PodDisruptionBudget) error { + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} return retryOnConflict(func() error { - currentPDB := &policyv1.PodDisruptionBudget{} - err := rclient.Get(ctx, types.NamespacedName{Namespace: newPDB.Namespace, Name: newPDB.Name}, currentPDB) - if err != nil { + var existingObj policyv1.PodDisruptionBudget + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating new PDB %s", newPDB.Name)) - return rclient.Create(ctx, newPDB) + logger.WithContext(ctx).Info(fmt.Sprintf("creating new PDB=%s", nsn)) + return rclient.Create(ctx, newObj) } - return fmt.Errorf("cannot get existing pdb: %s, err: %w", newPDB.Name, err) + return fmt.Errorf("cannot get existing PDB=%s: %w", nsn, err) } - if !currentPDB.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, currentPDB) - } - if err := finalize.FreeIfNeeded(ctx, rclient, currentPDB); err != nil { + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { return err } - if equality.Semantic.DeepEqual(newPDB.Spec, currentPDB.Spec) && - equality.Semantic.DeepEqual(newPDB.Labels, currentPDB.Labels) && - isObjectMetaEqual(currentPDB, newPDB, prevPDB) { + if equality.Semantic.DeepEqual(newObj.Spec, existingObj.Spec) && + equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) && + equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) { return nil } - logMsg := fmt.Sprintf("updating PDB %s configuration spec_diff: %s", newPDB.Name, diffDeep(newPDB.Spec, currentPDB.Spec)) - logger.WithContext(ctx).Info(logMsg) - - mergeObjectMetadataIntoNew(currentPDB, newPDB, prevPDB) - // for some reason Status is not marked as status sub-resource at PDB CRD - newPDB.Status = currentPDB.Status - - return rclient.Update(ctx, newPDB) + 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 PDB=%s, spec_diff=%s", nsn, specDiff)) + return rclient.Update(ctx, &existingObj) }) } diff --git a/internal/controller/operator/factory/reconcile/pvc.go b/internal/controller/operator/factory/reconcile/pvc.go index ca48628d8..196319a09 100644 --- a/internal/controller/operator/factory/reconcile/pvc.go +++ b/internal/controller/operator/factory/reconcile/pvc.go @@ -18,25 +18,25 @@ import ( // Makes attempt to resize pvc if needed // in case of deletion timestamp > 0 does nothing // user must manually remove finalizer if needed -func PersistentVolumeClaim(ctx context.Context, rclient client.Client, newPVC, prevPVC *corev1.PersistentVolumeClaim) error { +func PersistentVolumeClaim(ctx context.Context, rclient client.Client, newObj *corev1.PersistentVolumeClaim) error { l := logger.WithContext(ctx) - currentPVC := &corev1.PersistentVolumeClaim{} - err := rclient.Get(ctx, types.NamespacedName{Namespace: newPVC.Namespace, Name: newPVC.Name}, currentPVC) - if err != nil { + var existingObj corev1.PersistentVolumeClaim + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { - l.Info(fmt.Sprintf("creating new PVC %s", newPVC.Name)) - if err := rclient.Create(ctx, newPVC); err != nil { - return fmt.Errorf("cannot create new PVC: %w", err) + l.Info(fmt.Sprintf("creating new PVC=%s", nsn)) + if err := rclient.Create(ctx, newObj); err != nil { + return fmt.Errorf("cannot create new PVC=%s: %w", nsn, err) } return nil } - return fmt.Errorf("cannot get existing PVC: %w", err) + return fmt.Errorf("cannot get existing PVC=%s: %w", nsn, err) } - if !currentPVC.DeletionTimestamp.IsZero() { - l.Info("PVC has non zero DeletionTimestamp, skip update." + - " To fix this, make backup for this pvc, delete pvc finalizers and restore from backup.") + if !existingObj.DeletionTimestamp.IsZero() { + l.Info(fmt.Sprintf("PVC=%s has non zero DeletionTimestamp, skip update."+ + " To fix this, make backup for this pvc, delete pvc finalizers and restore from backup.", nsn)) return nil } - return updatePVC(ctx, rclient, currentPVC, newPVC, prevPVC) + return updatePVC(ctx, rclient, &existingObj, newObj) } diff --git a/internal/controller/operator/factory/reconcile/rbac.go b/internal/controller/operator/factory/reconcile/rbac.go index 7de1fae72..f25fcae54 100644 --- a/internal/controller/operator/factory/reconcile/rbac.go +++ b/internal/controller/operator/factory/reconcile/rbac.go @@ -7,128 +7,133 @@ import ( rbacv1 "k8s.io/api/rbac/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/finalize" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger" ) // RoleBinding reconciles rolebindg object -func RoleBinding(ctx context.Context, rclient client.Client, newRB, prevRB *rbacv1.RoleBinding) error { - var currentRB rbacv1.RoleBinding - if err := rclient.Get(ctx, types.NamespacedName{Namespace: newRB.Namespace, Name: newRB.Name}, ¤tRB); err != nil { +func RoleBinding(ctx context.Context, rclient client.Client, newObj, prevObj *rbacv1.RoleBinding) error { + var existingObj rbacv1.RoleBinding + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating new RoleBinding %s", newRB.Name)) - return rclient.Create(ctx, newRB) + logger.WithContext(ctx).Info(fmt.Sprintf("creating new RoleBinding=%s", nsn)) + return rclient.Create(ctx, newObj) } - return fmt.Errorf("cannot get exist rolebinding: %w", err) + return fmt.Errorf("cannot get exist RoleBinding=%s: %w", nsn, err) } - if !currentRB.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, ¤tRB) - } - if err := finalize.FreeIfNeeded(ctx, rclient, ¤tRB); err != nil { + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { return err } - - if equality.Semantic.DeepEqual(newRB.Subjects, currentRB.Subjects) && - equality.Semantic.DeepEqual(newRB.RoleRef, currentRB.RoleRef) && - isObjectMetaEqual(¤tRB, newRB, prevRB) { + var prevMeta *metav1.ObjectMeta + if prevObj != nil { + prevMeta = &prevObj.ObjectMeta + } + if equality.Semantic.DeepEqual(newObj.Subjects, existingObj.Subjects) && + equality.Semantic.DeepEqual(newObj.RoleRef, existingObj.RoleRef) && + isObjectMetaEqual(&existingObj, newObj, prevMeta) { return nil } - logger.WithContext(ctx).Info(fmt.Sprintf("updating RoleBinding %s configuration", newRB.Name)) - mergeObjectMetadataIntoNew(¤tRB, newRB, prevRB) - vmv1beta1.AddFinalizer(newRB, ¤tRB) + logger.WithContext(ctx).Info(fmt.Sprintf("updating RoleBinding=%s configuration", nsn)) + mergeObjectMetadataIntoNew(&existingObj, newObj, prevMeta) + vmv1beta1.AddFinalizer(newObj, &existingObj) - return rclient.Update(ctx, newRB) + return rclient.Update(ctx, newObj) } // Role reconciles role object -func Role(ctx context.Context, rclient client.Client, newRL, prevRL *rbacv1.Role) error { - var currentRL rbacv1.Role - if err := rclient.Get(ctx, types.NamespacedName{Namespace: newRL.Namespace, Name: newRL.Name}, ¤tRL); err != nil { +func Role(ctx context.Context, rclient client.Client, newObj, prevObj *rbacv1.Role) error { + var existingObj rbacv1.Role + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating new Role %s", newRL.Name)) - return rclient.Create(ctx, newRL) + logger.WithContext(ctx).Info(fmt.Sprintf("creating new Role=%s", nsn)) + return rclient.Create(ctx, newObj) } - return fmt.Errorf("cannot get exist role: %w", err) - } - if !currentRL.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, ¤tRL) + return fmt.Errorf("cannot get exist Role=%s: %w", nsn, err) } - if err := finalize.FreeIfNeeded(ctx, rclient, ¤tRL); err != nil { + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { return err } - - if equality.Semantic.DeepEqual(newRL.Rules, currentRL.Rules) && - isObjectMetaEqual(¤tRL, newRL, prevRL) { + var prevMeta *metav1.ObjectMeta + if prevObj != nil { + prevMeta = &prevObj.ObjectMeta + } + if equality.Semantic.DeepEqual(newObj.Rules, existingObj.Rules) && + isObjectMetaEqual(&existingObj, newObj, prevMeta) { return nil } - logger.WithContext(ctx).Info(fmt.Sprintf("updating Role %s configuration", newRL.Name)) - mergeObjectMetadataIntoNew(¤tRL, newRL, prevRL) - vmv1beta1.AddFinalizer(newRL, ¤tRL) + logger.WithContext(ctx).Info(fmt.Sprintf("updating Role=%s configuration", nsn)) + mergeObjectMetadataIntoNew(&existingObj, newObj, prevMeta) + vmv1beta1.AddFinalizer(newObj, &existingObj) - return rclient.Update(ctx, newRL) + return rclient.Update(ctx, newObj) } // ClusterRoleBinding reconciles cluster role binding object -func ClusterRoleBinding(ctx context.Context, rclient client.Client, newCRB, prevCRB *rbacv1.ClusterRoleBinding) error { - var currentCRB rbacv1.ClusterRoleBinding - - if err := rclient.Get(ctx, types.NamespacedName{Name: newCRB.Name, Namespace: newCRB.Namespace}, ¤tCRB); err != nil { +func ClusterRoleBinding(ctx context.Context, rclient client.Client, newObj, prevObj *rbacv1.ClusterRoleBinding) error { + var existingObj rbacv1.ClusterRoleBinding + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating new ClusterRoleBinding %s", newCRB.Name)) - return rclient.Create(ctx, newCRB) + logger.WithContext(ctx).Info(fmt.Sprintf("creating new ClusterRoleBinding=%s", newObj.Name)) + return rclient.Create(ctx, newObj) } - return fmt.Errorf("cannot get crb: %w", err) + return fmt.Errorf("cannot get ClusterRoleBinding=%s: %w", newObj.Name, err) } - if !currentCRB.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, ¤tCRB) - } - if err := finalize.FreeIfNeeded(ctx, rclient, ¤tCRB); err != nil { + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { return err } + var prevMeta *metav1.ObjectMeta + if prevObj != nil { + prevMeta = &prevObj.ObjectMeta + } // fast path - if equality.Semantic.DeepEqual(newCRB.Subjects, currentCRB.Subjects) && - equality.Semantic.DeepEqual(newCRB.RoleRef, currentCRB.RoleRef) && - isObjectMetaEqual(¤tCRB, newCRB, prevCRB) { + if equality.Semantic.DeepEqual(newObj.Subjects, existingObj.Subjects) && + equality.Semantic.DeepEqual(newObj.RoleRef, existingObj.RoleRef) && + isObjectMetaEqual(&existingObj, newObj, prevMeta) { return nil } - logger.WithContext(ctx).Info(fmt.Sprintf("updating ClusterRoleBinding %s", newCRB.Name)) - mergeObjectMetadataIntoNew(¤tCRB, newCRB, prevCRB) - vmv1beta1.AddFinalizer(newCRB, ¤tCRB) + logger.WithContext(ctx).Info(fmt.Sprintf("updating ClusterRoleBinding=%s", newObj.Name)) + mergeObjectMetadataIntoNew(&existingObj, newObj, prevMeta) + vmv1beta1.AddFinalizer(newObj, &existingObj) - return rclient.Update(ctx, newCRB) + return rclient.Update(ctx, newObj) } // ClusterRole reconciles cluster role object -func ClusterRole(ctx context.Context, rclient client.Client, newClusterRole, prevClusterRole *rbacv1.ClusterRole) error { - var currentClusterRole rbacv1.ClusterRole - - if err := rclient.Get(ctx, types.NamespacedName{Name: newClusterRole.Name, Namespace: newClusterRole.Namespace}, ¤tClusterRole); err != nil { +func ClusterRole(ctx context.Context, rclient client.Client, newObj, prevObj *rbacv1.ClusterRole) error { + var existingObj rbacv1.ClusterRole + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating new ClusterRole %s", newClusterRole.Name)) - return rclient.Create(ctx, newClusterRole) + logger.WithContext(ctx).Info(fmt.Sprintf("creating new ClusterRole=%s", newObj.Name)) + return rclient.Create(ctx, newObj) } - return fmt.Errorf("cannot get exist cluster role: %w", err) + return fmt.Errorf("cannot get exist ClusterRole=%s: %w", newObj.Name, err) } - if !currentClusterRole.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, ¤tClusterRole) - } - if err := finalize.FreeIfNeeded(ctx, rclient, ¤tClusterRole); err != nil { + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { return err } + var prevMeta *metav1.ObjectMeta + if prevObj != nil { + prevMeta = &prevObj.ObjectMeta + } + // fast path - if equality.Semantic.DeepEqual(newClusterRole.Rules, currentClusterRole.Rules) && - isObjectMetaEqual(¤tClusterRole, newClusterRole, prevClusterRole) { + if equality.Semantic.DeepEqual(newObj.Rules, existingObj.Rules) && + isObjectMetaEqual(&existingObj, newObj, prevMeta) { return nil } - logger.WithContext(ctx).Info(fmt.Sprintf("updating ClusterRole %s", newClusterRole.Name)) + logger.WithContext(ctx).Info(fmt.Sprintf("updating ClusterRole=%s", newObj.Name)) - mergeObjectMetadataIntoNew(¤tClusterRole, newClusterRole, prevClusterRole) - vmv1beta1.AddFinalizer(newClusterRole, ¤tClusterRole) - return rclient.Update(ctx, newClusterRole) + mergeObjectMetadataIntoNew(&existingObj, newObj, prevMeta) + vmv1beta1.AddFinalizer(newObj, &existingObj) + return rclient.Update(ctx, newObj) } diff --git a/internal/controller/operator/factory/reconcile/reconcile.go b/internal/controller/operator/factory/reconcile/reconcile.go index aef82eff6..7916f3078 100644 --- a/internal/controller/operator/factory/reconcile/reconcile.go +++ b/internal/controller/operator/factory/reconcile/reconcile.go @@ -4,14 +4,15 @@ import ( "context" "errors" "fmt" - "reflect" "strings" "time" k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/util/retry" "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" ) @@ -58,7 +59,7 @@ func mergeAnnotations(currentA, newA, prevA map[string]string) map[string]string // isObjectMetaEqual properly track changes at object annotations // it preserves 3rd party annotations -func isObjectMetaEqual(currObj, newObj, prevObj client.Object) bool { +func isObjectMetaEqual(currObj, newObj client.Object, prevMeta *metav1.ObjectMeta) bool { isMapsEqual := func(currentA, newA, prevA map[string]string) bool { for k, v := range newA { cv, ok := currentA[k] @@ -80,9 +81,9 @@ func isObjectMetaEqual(currObj, newObj, prevObj client.Object) bool { return true } var prevLabels, prevAnnotations map[string]string - if prevObj != nil && !reflect.ValueOf(prevObj).IsNil() { - prevLabels = prevObj.GetLabels() - prevAnnotations = prevObj.GetAnnotations() + if prevMeta != nil { + prevLabels = prevMeta.Labels + prevAnnotations = prevMeta.Annotations } annotationsEqual := isMapsEqual(currObj.GetAnnotations(), newObj.GetAnnotations(), prevAnnotations) labelsEqual := isMapsEqual(currObj.GetLabels(), newObj.GetLabels(), prevLabels) @@ -90,7 +91,7 @@ func isObjectMetaEqual(currObj, newObj, prevObj client.Object) bool { return annotationsEqual && labelsEqual } -func mergeObjectMetadataIntoNew(currObj, newObj, prevObj client.Object) { +func mergeObjectMetadataIntoNew(currObj, newObj client.Object, prevMeta *metav1.ObjectMeta) { // empty ResourceVersion for some resources produces the following error // is invalid: metadata.resourceVersion: Invalid value: 0x0: must be specified for an update // so keep it from current resource @@ -103,9 +104,9 @@ func mergeObjectMetadataIntoNew(currObj, newObj, prevObj client.Object) { newObj.SetSelfLink(currObj.GetSelfLink()) var prevLabels, prevAnnotations map[string]string - if prevObj != nil && !reflect.ValueOf(prevObj).IsNil() { - prevLabels = prevObj.GetLabels() - prevAnnotations = prevObj.GetAnnotations() + if prevMeta != nil { + prevLabels = prevMeta.Labels + prevAnnotations = prevMeta.Annotations } annotations := mergeAnnotations(currObj.GetAnnotations(), newObj.GetAnnotations(), prevAnnotations) labels := mergeAnnotations(currObj.GetLabels(), newObj.GetLabels(), prevLabels) @@ -171,3 +172,15 @@ func isConflict(err error) bool { func retryOnConflict(fn func() error) error { return retry.OnError(retry.DefaultRetry, isConflict, fn) } + +// freeIfNeeded checks if resource must be freed from finalizer and garbage collected by kubernetes +func freeIfNeeded(ctx context.Context, rclient client.Client, obj client.Object) error { + if obj.GetDeletionTimestamp().IsZero() { + // fast path + return nil + } + if err := finalize.RemoveFinalizer(ctx, rclient, obj); err != nil { + return fmt.Errorf("cannot remove finalizer from %s=%s/%s: %w", obj.GetObjectKind().GroupVersionKind().Kind, obj.GetNamespace(), obj.GetName(), err) + } + return newErrRecreate(ctx, obj) +} diff --git a/internal/controller/operator/factory/reconcile/secret.go b/internal/controller/operator/factory/reconcile/secret.go index 1d0f49d52..ee8814b60 100644 --- a/internal/controller/operator/factory/reconcile/secret.go +++ b/internal/controller/operator/factory/reconcile/secret.go @@ -7,48 +7,36 @@ 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" - "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/finalize" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger" ) // Secret reconciles secret object -func Secret(ctx context.Context, rclient client.Client, newS *corev1.Secret, prevMeta *metav1.ObjectMeta) error { - var currentS corev1.Secret - - if err := rclient.Get(ctx, types.NamespacedName{Namespace: newS.Namespace, Name: newS.Name}, ¤tS); err != nil { - if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating new Secret %s", newS.Name)) - return rclient.Create(ctx, newS) +func Secret(ctx context.Context, rclient client.Client, newObj *corev1.Secret) error { + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} + return retryOnConflict(func() error { + var existingObj corev1.Secret + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { + if k8serrors.IsNotFound(err) { + logger.WithContext(ctx).Info(fmt.Sprintf("creating new Secret=%s", nsn)) + return rclient.Create(ctx, newObj) + } + return err } - return err - } - if !currentS.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, ¤tS) - } - if err := finalize.FreeIfNeeded(ctx, rclient, ¤tS); err != nil { - return err - } - - var prevS *corev1.Secret - if prevMeta != nil { - prevS = &corev1.Secret{ - ObjectMeta: *prevMeta, + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { + return err } - } - - if equality.Semantic.DeepEqual(newS.Data, currentS.Data) && - equality.Semantic.DeepEqual(newS.Labels, currentS.Labels) && - isObjectMetaEqual(¤tS, newS, prevS) { - return nil - } - - mergeObjectMetadataIntoNew(¤tS, newS, prevS) - - logger.WithContext(ctx).Info(fmt.Sprintf("updating configuration Secret %s", newS.Name)) - - return rclient.Update(ctx, newS) + if equality.Semantic.DeepEqual(newObj.Data, existingObj.Data) && + equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) && + equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) { + return nil + } + existingObj.Labels = newObj.Labels + existingObj.Annotations = newObj.Annotations + existingObj.Data = newObj.Data + logger.WithContext(ctx).Info(fmt.Sprintf("updating configuration Secret=%s", nsn)) + return rclient.Update(ctx, &existingObj) + }) } diff --git a/internal/controller/operator/factory/reconcile/service.go b/internal/controller/operator/factory/reconcile/service.go index 73ec52a5d..140f35b89 100644 --- a/internal/controller/operator/factory/reconcile/service.go +++ b/internal/controller/operator/factory/reconcile/service.go @@ -7,6 +7,7 @@ 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" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -21,91 +22,90 @@ import ( // NOTE it doesn't perform validation: // in case of spec.type= LoadBalancer or NodePort, clusterIP: None is not allowed, // its users responsibility to define it correctly. -func Service(ctx context.Context, rclient client.Client, newService, prevService *corev1.Service) error { - svcForReconcile := newService.DeepCopy() +func Service(ctx context.Context, rclient client.Client, newObj, prevObj *corev1.Service) error { + svcForReconcile := newObj.DeepCopy() return retryOnConflict(func() error { - return reconcileService(ctx, rclient, svcForReconcile, prevService) + return reconcileService(ctx, rclient, svcForReconcile, prevObj) }) } -func reconcileService(ctx context.Context, rclient client.Client, newService, prevService *corev1.Service) error { +func reconcileService(ctx context.Context, rclient client.Client, newObj, prevObj *corev1.Service) error { // helper for proper service deletion. + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} recreateService := func(svc *corev1.Service) error { if err := finalize.RemoveFinalizer(ctx, rclient, svc); err != nil { return err } if err := finalize.SafeDelete(ctx, rclient, svc); err != nil { - return fmt.Errorf("cannot delete service at recreate: %w", err) + return fmt.Errorf("cannot delete Service=%s at recreate: %w", nsn, err) } - logger.WithContext(ctx).Info(fmt.Sprintf("recreating new Service %s", newService.Name)) - if err := rclient.Create(ctx, newService); err != nil { - return fmt.Errorf("cannot create service at recreate: %w", err) + logger.WithContext(ctx).Info(fmt.Sprintf("recreating new Service=%s", nsn)) + if err := rclient.Create(ctx, newObj); err != nil { + return fmt.Errorf("cannot create Service=%s at recreate: %w", nsn, err) } return nil } - currentService := &corev1.Service{} - err := rclient.Get(ctx, types.NamespacedName{Name: newService.Name, Namespace: newService.Namespace}, currentService) - if err != nil { + var existingObj corev1.Service + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating new Service %s", newService.Name)) - err := rclient.Create(ctx, newService) + logger.WithContext(ctx).Info(fmt.Sprintf("creating new Service=%s", nsn)) + err := rclient.Create(ctx, newObj) if err != nil { - return fmt.Errorf("cannot create new service: %w", err) + return fmt.Errorf("cannot create new Service=%s: %w", nsn, err) } return nil } - return fmt.Errorf("cannot get service for existing service: %w", err) + return fmt.Errorf("cannot get service for existing Service=%s: %w", nsn, err) } - if !currentService.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, currentService) - } - if err := finalize.FreeIfNeeded(ctx, rclient, currentService); err != nil { + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { return err } var isPrevServiceEqual bool var prevSpecDiff string - if prevService != nil { - isPrevServiceEqual = equality.Semantic.DeepDerivative(prevService, newService) + var prevMeta *metav1.ObjectMeta + if prevObj != nil { + prevMeta = &prevObj.ObjectMeta + isPrevServiceEqual = equality.Semantic.DeepDerivative(prevObj, newObj) if !isPrevServiceEqual { - prevSpecDiff = diffDeepDerivative(prevService, newService) + prevSpecDiff = diffDeepDerivative(prevObj, newObj) } // keep LoadBalancerClass assigned by cloud-controller // See https://github.com/VictoriaMetrics/operator/issues/1550 - if prevService.Spec.LoadBalancerClass == nil && newService.Spec.LoadBalancerClass == nil { - newService.Spec.LoadBalancerClass = currentService.Spec.LoadBalancerClass + if prevObj.Spec.LoadBalancerClass == nil && newObj.Spec.LoadBalancerClass == nil { + newObj.Spec.LoadBalancerClass = existingObj.Spec.LoadBalancerClass } } // invariants switch { - case newService.Spec.Type != currentService.Spec.Type: + case newObj.Spec.Type != existingObj.Spec.Type: // type mismatch. // need to remove it and recreate. - return recreateService(currentService) - case newService.Spec.ClusterIP != "" && - newService.Spec.ClusterIP != "None" && - newService.Spec.ClusterIP != currentService.Spec.ClusterIP: + return recreateService(&existingObj) + case newObj.Spec.ClusterIP != "" && + newObj.Spec.ClusterIP != "None" && + newObj.Spec.ClusterIP != existingObj.Spec.ClusterIP: // ip was changed by user, remove old service and create new one. - return recreateService(currentService) - case newService.Spec.ClusterIP == "None" && currentService.Spec.ClusterIP != "None": + return recreateService(&existingObj) + case newObj.Spec.ClusterIP == "None" && existingObj.Spec.ClusterIP != "None": // serviceType changed from clusterIP to headless - return recreateService(currentService) - case newService.Spec.ClusterIP == "" && currentService.Spec.ClusterIP == "None": + return recreateService(&existingObj) + case newObj.Spec.ClusterIP == "" && existingObj.Spec.ClusterIP == "None": // serviceType changes from headless to clusterIP - return recreateService(currentService) - case ptr.Deref(newService.Spec.LoadBalancerClass, "") != ptr.Deref(currentService.Spec.LoadBalancerClass, ""): - return recreateService(currentService) + return recreateService(&existingObj) + case ptr.Deref(newObj.Spec.LoadBalancerClass, "") != ptr.Deref(existingObj.Spec.LoadBalancerClass, ""): + return recreateService(&existingObj) } // keep given clusterIP for service. - if newService.Spec.ClusterIP != "None" { - newService.Spec.ClusterIP = currentService.Spec.ClusterIP + if newObj.Spec.ClusterIP != "None" { + newObj.Spec.ClusterIP = existingObj.Spec.ClusterIP } // keep allocated node ports. - if newService.Spec.Type == currentService.Spec.Type { - for i := range currentService.Spec.Ports { - existPort := currentService.Spec.Ports[i] - for j := range newService.Spec.Ports { - newPort := &newService.Spec.Ports[j] + if newObj.Spec.Type == existingObj.Spec.Type { + for i := range existingObj.Spec.Ports { + existPort := existingObj.Spec.Ports[i] + for j := range newObj.Spec.Ports { + newPort := &newObj.Spec.Ports[j] // add missing port, only if its not defined by user. if existPort.Name == newPort.Name && newPort.NodePort == 0 { newPort.NodePort = existPort.NodePort @@ -115,33 +115,28 @@ func reconcileService(ctx context.Context, rclient client.Client, newService, pr } } - rclient.Scheme().Default(newService) - isEqual := equality.Semantic.DeepDerivative(newService.Spec, currentService.Spec) + rclient.Scheme().Default(newObj) + isEqual := equality.Semantic.DeepDerivative(newObj.Spec, existingObj.Spec) if isEqual && isPrevServiceEqual && - equality.Semantic.DeepEqual(newService.Labels, currentService.Labels) && - isObjectMetaEqual(currentService, newService, prevService) { + equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) && + isObjectMetaEqual(&existingObj, newObj, prevMeta) { return nil } - vmv1beta1.AddFinalizer(newService, currentService) - mergeObjectMetadataIntoNew(currentService, newService, prevService) + vmv1beta1.AddFinalizer(newObj, &existingObj) + mergeObjectMetadataIntoNew(&existingObj, newObj, prevMeta) - logMsg := fmt.Sprintf("updating service %s configuration, is_current_equal=%v, is_prev_equal=%v, is_prev_nil=%v", - newService.Name, isEqual, isPrevServiceEqual, prevService == nil) + logMsg := fmt.Sprintf("updating service=%s configuration, is_current_equal=%v, is_prev_equal=%v, is_prev_nil=%v", + nsn, isEqual, isPrevServiceEqual, prevObj == nil) if len(prevSpecDiff) > 0 { logMsg += fmt.Sprintf(", prev_spec_diff=%s", prevSpecDiff) } if !isEqual { - logMsg += fmt.Sprintf(", curr_spec_diff=%s", diffDeepDerivative(newService.Spec, currentService.Spec)) + logMsg += fmt.Sprintf(", curr_spec_diff=%s", diffDeepDerivative(newObj.Spec, existingObj.Spec)) } logger.WithContext(ctx).Info(logMsg) - err = rclient.Update(ctx, newService) - if err != nil { - return err - } - - return nil + return rclient.Update(ctx, newObj) } diff --git a/internal/controller/operator/factory/reconcile/service_account.go b/internal/controller/operator/factory/reconcile/service_account.go index 34f449371..086cd38f6 100644 --- a/internal/controller/operator/factory/reconcile/service_account.go +++ b/internal/controller/operator/factory/reconcile/service_account.go @@ -5,43 +5,37 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" k8serrors "k8s.io/apimachinery/pkg/api/errors" "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/finalize" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger" ) // ServiceAccount creates service account or updates exist one -func ServiceAccount(ctx context.Context, rclient client.Client, newSA, prevSA *corev1.ServiceAccount) error { +func ServiceAccount(ctx context.Context, rclient client.Client, newObj *corev1.ServiceAccount) error { + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} return retryOnConflict(func() error { - var currentSA corev1.ServiceAccount - if err := rclient.Get(ctx, types.NamespacedName{Name: newSA.Name, Namespace: newSA.Namespace}, ¤tSA); err != nil { + var existingObj corev1.ServiceAccount + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating new ServiceAccount %s", newSA.Name)) - return rclient.Create(ctx, newSA) + logger.WithContext(ctx).Info(fmt.Sprintf("creating new ServiceAccount=%s", nsn)) + return rclient.Create(ctx, newObj) } - return fmt.Errorf("cannot get ServiceAccount: %w", err) + return fmt.Errorf("cannot get ServiceAccount=%s: %w", nsn, err) } - if !currentSA.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, ¤tSA) - } - if err := finalize.FreeIfNeeded(ctx, rclient, ¤tSA); err != nil { + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { return err } - if isObjectMetaEqual(¤tSA, newSA, prevSA) { + + if equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) && + equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) { return nil } - mergeObjectMetadataIntoNew(¤tSA, newSA, prevSA) - vmv1beta1.AddFinalizer(newSA, ¤tSA) - // keep significant fields - newSA.Secrets = currentSA.Secrets - newSA.AutomountServiceAccountToken = currentSA.AutomountServiceAccountToken - newSA.ImagePullSecrets = currentSA.ImagePullSecrets - logger.WithContext(ctx).Info(fmt.Sprintf("updating ServiceAccount %s metadata", newSA.Name)) - - return rclient.Update(ctx, newSA) + existingObj.Labels = newObj.Labels + existingObj.Annotations = newObj.Annotations + logger.WithContext(ctx).Info(fmt.Sprintf("updating ServiceAccount=%s", nsn)) + return rclient.Update(ctx, &existingObj) }) } diff --git a/internal/controller/operator/factory/reconcile/statefulset.go b/internal/controller/operator/factory/reconcile/statefulset.go index 8a717e8d9..aeeccf1c0 100644 --- a/internal/controller/operator/factory/reconcile/statefulset.go +++ b/internal/controller/operator/factory/reconcile/statefulset.go @@ -14,6 +14,7 @@ import ( policyv1 "k8s.io/api/policy/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/intstr" @@ -21,7 +22,6 @@ import ( "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" ) @@ -37,81 +37,81 @@ type STSOptions struct { UpdateBehavior *vmv1beta1.StatefulSetUpdateStrategyBehavior } -func waitForStatefulSetReady(ctx context.Context, rclient client.Client, newSts *appsv1.StatefulSet) error { +func waitForStatefulSetReady(ctx context.Context, rclient client.Client, newObj *appsv1.StatefulSet) error { + nsn := types.NamespacedName{Namespace: newObj.Namespace, Name: newObj.Name} err := wait.PollUntilContextTimeout(ctx, podWaitReadyIntervalCheck, appWaitReadyDeadline, true, func(ctx context.Context) (done bool, err error) { // fast path - if newSts.Spec.Replicas == nil { + if newObj.Spec.Replicas == nil { return true, nil } var stsForStatus appsv1.StatefulSet - if err := rclient.Get(ctx, types.NamespacedName{Namespace: newSts.Namespace, Name: newSts.Name}, &stsForStatus); err != nil { + if err := rclient.Get(ctx, nsn, &stsForStatus); err != nil { return false, err } if stsForStatus.Generation > stsForStatus.Status.ObservedGeneration || // special case to prevent possible race condition between updated object and local cache // See this issue https://github.com/VictoriaMetrics/operator/issues/1579 - newSts.Generation > stsForStatus.Generation { + newObj.Generation > stsForStatus.Generation { return false, nil } - if *newSts.Spec.Replicas != stsForStatus.Status.ReadyReplicas || *newSts.Spec.Replicas != stsForStatus.Status.UpdatedReplicas { + if *newObj.Spec.Replicas != stsForStatus.Status.ReadyReplicas || *newObj.Spec.Replicas != stsForStatus.Status.UpdatedReplicas { return false, nil } return true, nil }) if err != nil { - return reportFirstNotReadyPodOnError(ctx, rclient, fmt.Errorf("cannot wait for statefulSet=%s to become ready: %w", newSts.Name, err), newSts.Namespace, labels.SelectorFromSet(newSts.Spec.Selector.MatchLabels), newSts.Spec.MinReadySeconds) + return reportFirstNotReadyPodOnError(ctx, rclient, fmt.Errorf("cannot wait for statefulSet=%s to become ready: %w", newObj.Name, err), newObj.Namespace, labels.SelectorFromSet(newObj.Spec.Selector.MatchLabels), newObj.Spec.MinReadySeconds) } return nil } // HandleSTSUpdate performs create and update operations for given statefulSet with STSOptions -func HandleSTSUpdate(ctx context.Context, rclient client.Client, cr STSOptions, newSts, prevSts *appsv1.StatefulSet) error { - if err := validateStatefulSet(newSts); err != nil { +func HandleSTSUpdate(ctx context.Context, rclient client.Client, cr STSOptions, newObj, prevObj *appsv1.StatefulSet) error { + if err := validateStatefulSet(newObj); err != nil { return err } var isPrevEqual bool var prevSpecDiff string - if prevSts != nil { - isPrevEqual = equality.Semantic.DeepDerivative(prevSts.Spec, newSts.Spec) + var prevMeta *metav1.ObjectMeta + if prevObj != nil { + prevMeta = &prevObj.ObjectMeta + isPrevEqual = equality.Semantic.DeepDerivative(prevObj.Spec, newObj.Spec) if !isPrevEqual { - prevSpecDiff = diffDeepDerivative(prevSts.Spec, newSts.Spec) + prevSpecDiff = diffDeepDerivative(prevObj.Spec, newObj.Spec) } } - rclient.Scheme().Default(newSts) - + rclient.Scheme().Default(newObj) + nsn := types.NamespacedName{Name: newObj.Name, Namespace: newObj.Namespace} return retryOnConflict(func() error { - var currentSts appsv1.StatefulSet - if err := rclient.Get(ctx, types.NamespacedName{Name: newSts.Name, Namespace: newSts.Namespace}, ¤tSts); err != nil { + var existingObj appsv1.StatefulSet + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating new StatefulSet %s", newSts.Name)) - if err = rclient.Create(ctx, newSts); err != nil { - return fmt.Errorf("cannot create new sts %s under namespace %s: %w", newSts.Name, newSts.Namespace, err) + logger.WithContext(ctx).Info(fmt.Sprintf("creating new StatefulSet=%s", nsn)) + if err = rclient.Create(ctx, newObj); err != nil { + return fmt.Errorf("cannot create new StatefulSet=%s: %w", nsn, err) } - return waitForStatefulSetReady(ctx, rclient, newSts) + return waitForStatefulSetReady(ctx, rclient, newObj) } - return fmt.Errorf("cannot get sts %s under namespace %s: %w", newSts.Name, newSts.Namespace, err) - } - if !currentSts.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, ¤tSts) + return fmt.Errorf("cannot get StatefulSet=%s: %w", nsn, err) } - if err := finalize.FreeIfNeeded(ctx, rclient, ¤tSts); err != nil { + if err := freeIfNeeded(ctx, rclient, &existingObj); 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 { - cr.UpdateReplicaCount(currentSts.Spec.Replicas) + cr.UpdateReplicaCount(existingObj.Spec.Replicas) } // do not change replicas count. if cr.HPA != nil { - newSts.Spec.Replicas = currentSts.Spec.Replicas + newObj.Spec.Replicas = existingObj.Spec.Replicas } // hack for kubernetes 1.18 - newSts.Status.Replicas = currentSts.Status.Replicas + newObj.Status.Replicas = existingObj.Status.Replicas - stsRecreated, podMustRecreate, err := recreateSTSIfNeed(ctx, rclient, newSts, ¤tSts) + stsRecreated, podMustRecreate, err := recreateSTSIfNeed(ctx, rclient, newObj, &existingObj) if err != nil { return err } @@ -120,25 +120,24 @@ func HandleSTSUpdate(ctx context.Context, rclient client.Client, cr STSOptions, // before making call for performRollingUpdateOnSts if !stsRecreated { var prevTemplateAnnotations map[string]string - if prevSts != nil { - prevTemplateAnnotations = prevSts.Spec.Template.Annotations + if prevObj != nil { + prevTemplateAnnotations = prevObj.Spec.Template.Annotations } - isEqual := equality.Semantic.DeepDerivative(newSts.Spec, currentSts.Spec) + isEqual := equality.Semantic.DeepDerivative(newObj.Spec, existingObj.Spec) shouldSkipUpdate := isPrevEqual && isEqual && - equality.Semantic.DeepEqual(newSts.Labels, currentSts.Labels) && - isObjectMetaEqual(¤tSts, newSts, prevSts) + equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) && + isObjectMetaEqual(&existingObj, newObj, prevMeta) if !shouldSkipUpdate { + vmv1beta1.AddFinalizer(newObj, &existingObj) + newObj.Spec.Template.Annotations = mergeAnnotations(existingObj.Spec.Template.Annotations, newObj.Spec.Template.Annotations, prevTemplateAnnotations) + mergeObjectMetadataIntoNew(&existingObj, newObj, prevMeta) - vmv1beta1.AddFinalizer(newSts, ¤tSts) - newSts.Spec.Template.Annotations = mergeAnnotations(currentSts.Spec.Template.Annotations, newSts.Spec.Template.Annotations, prevTemplateAnnotations) - mergeObjectMetadataIntoNew(¤tSts, newSts, prevSts) - - logMsg := fmt.Sprintf("updating statefulset %s configuration, is_current_equal=%v,is_prev_equal=%v,is_prev_nil=%v", - newSts.Name, isEqual, isPrevEqual, prevSts == nil) + logMsg := fmt.Sprintf("updating StatefulSet=%s configuration, is_current_equal=%v,is_prev_equal=%v,is_prev_nil=%v", + nsn, isEqual, isPrevEqual, prevMeta == nil) if !isEqual { - logMsg += fmt.Sprintf(", current_spec_diff=%s", diffDeepDerivative(newSts.Spec, currentSts.Spec)) + logMsg += fmt.Sprintf(", current_spec_diff=%s", diffDeepDerivative(newObj.Spec, existingObj.Spec)) } if len(prevSpecDiff) > 0 { logMsg += fmt.Sprintf(", prev_spec_diff=%s", prevSpecDiff) @@ -146,8 +145,8 @@ func HandleSTSUpdate(ctx context.Context, rclient client.Client, cr STSOptions, logger.WithContext(ctx).Info(logMsg) - if err := rclient.Update(ctx, newSts); err != nil { - return fmt.Errorf("cannot perform update on sts: %s, err: %w", newSts.Name, err) + if err := rclient.Update(ctx, newObj); err != nil { + return fmt.Errorf("cannot perform update on StatefulSet=%s: %w", nsn, err) } } } @@ -155,30 +154,30 @@ func HandleSTSUpdate(ctx context.Context, rclient client.Client, cr STSOptions, // determine manual update behavior only with OnDelete policy, one by one by default podMaxUnavailable := 1 if cr.UpdateBehavior != nil { - if newSts.Spec.UpdateStrategy.Type == appsv1.OnDeleteStatefulSetStrategyType { - podMaxUnavailable, err = intstr.GetScaledValueFromIntOrPercent(cr.UpdateBehavior.MaxUnavailable, int(*newSts.Spec.Replicas), false) + if newObj.Spec.UpdateStrategy.Type == appsv1.OnDeleteStatefulSetStrategyType { + podMaxUnavailable, err = intstr.GetScaledValueFromIntOrPercent(cr.UpdateBehavior.MaxUnavailable, int(*newObj.Spec.Replicas), false) if err != nil { return err } } else { - logger.WithContext(ctx).Info(fmt.Sprintf("ignoring custom update behavior settings with update strategy=%s on sts: %s", newSts.Spec.UpdateStrategy.Type, newSts.Name)) + logger.WithContext(ctx).Info(fmt.Sprintf("ignoring custom update behavior settings with update strategy=%s on sts: %s", newObj.Spec.UpdateStrategy.Type, newObj.Name)) } } // perform manual update only with OnDelete policy, which is default. - if newSts.Spec.UpdateStrategy.Type == appsv1.OnDeleteStatefulSetStrategyType { - if err := performRollingUpdateOnSts(ctx, podMustRecreate, rclient, newSts.Name, newSts.Namespace, cr.SelectorLabels(), podMaxUnavailable); err != nil { - return fmt.Errorf("cannot handle rolling-update on sts: %s, err: %w", newSts.Name, err) + if newObj.Spec.UpdateStrategy.Type == appsv1.OnDeleteStatefulSetStrategyType { + if err := performRollingUpdateOnSts(ctx, podMustRecreate, rclient, nsn, cr.SelectorLabels(), podMaxUnavailable); err != nil { + return fmt.Errorf("cannot handle rolling-update on StatefulSet=%s: %w", nsn, err) } } else { - if err := waitForStatefulSetReady(ctx, rclient, newSts); err != nil { - return fmt.Errorf("cannot ensure that statefulset is ready with strategy=%q: %w", newSts.Spec.UpdateStrategy.Type, err) + if err := waitForStatefulSetReady(ctx, rclient, newObj); err != nil { + return fmt.Errorf("cannot ensure that statefulset is ready with strategy=%q: %w", newObj.Spec.UpdateStrategy.Type, err) } } // check if pvcs need to resize if cr.HasClaim { - err = updateSTSPVC(ctx, rclient, newSts) + err = updateSTSPVC(ctx, rclient, newObj) } return err @@ -214,9 +213,9 @@ func getLatestStsState(ctx context.Context, rclient client.Client, targetSTS typ // // we always check if sts.Status.CurrentRevision needs update, to keep it equal to UpdateRevision // see https://github.com/kubernetes/kube-state-metrics/issues/1324#issuecomment-1779751992 -func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclient client.Client, stsName string, ns string, podLabels map[string]string, podMaxUnavailable int) error { +func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclient client.Client, nsn types.NamespacedName, podLabels map[string]string, podMaxUnavailable int) error { time.Sleep(podWaitReadyIntervalCheck) - sts, err := getLatestStsState(ctx, rclient, types.NamespacedName{Name: stsName, Namespace: ns}) + sts, err := getLatestStsState(ctx, rclient, nsn) if err != nil { return err } @@ -238,7 +237,7 @@ func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclien l.Info(fmt.Sprintf("check if pod update needed to desiredVersion=%s, podMustRecreate=%v", stsVersion, podMustRecreate)) podList := &corev1.PodList{} labelSelector := labels.SelectorFromSet(podLabels) - listOps := &client.ListOptions{Namespace: ns, LabelSelector: labelSelector} + listOps := &client.ListOptions{Namespace: nsn.Namespace, LabelSelector: labelSelector} if err := rclient.List(ctx, podList, listOps); err != nil { return fmt.Errorf("cannot list pods for statefulset rolling update: %w", err) } @@ -302,7 +301,7 @@ func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclien // check updated, by not ready pods for _, pod := range updatedPods { l.Info(fmt.Sprintf("checking ready status for already updated pod %s to revision version=%q", pod.Name, stsVersion)) - podNsn := types.NamespacedName{Namespace: ns, Name: pod.Name} + podNsn := types.NamespacedName{Namespace: nsn.Namespace, Name: pod.Name} err := waitForPodReady(ctx, rclient, podNsn, stsVersion, sts.Spec.MinReadySeconds) if err != nil { return fmt.Errorf("cannot wait for pod ready state for already updated pod: %w", err) @@ -344,7 +343,7 @@ func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclien return fmt.Errorf("cannot perform pod eviction: %w", evictErr) } // wait for pod to be re-created - podNsn := types.NamespacedName{Namespace: ns, Name: pod.Name} + podNsn := types.NamespacedName{Namespace: nsn.Namespace, Name: pod.Name} if err := waitForPodReady(ctx, rclient, podNsn, stsVersion, sts.Spec.MinReadySeconds); err != nil { return fmt.Errorf("cannot wait for pod ready state during re-creation for pod %s: %w", pod.Name, err) } diff --git a/internal/controller/operator/factory/reconcile/statefulset_pvc_expand.go b/internal/controller/operator/factory/reconcile/statefulset_pvc_expand.go index db0baf157..5729b4975 100644 --- a/internal/controller/operator/factory/reconcile/statefulset_pvc_expand.go +++ b/internal/controller/operator/factory/reconcile/statefulset_pvc_expand.go @@ -120,21 +120,23 @@ func updateSTSPVC(ctx context.Context, rclient client.Client, sts *appsv1.Statef continue } // update PVC size and metadata if it's needed - if err := updatePVC(ctx, rclient, &pvc, &stsClaim, nil); err != nil { + if err := updatePVC(ctx, rclient, &pvc, &stsClaim); err != nil { return err } } return nil } -func updatePVC(ctx context.Context, rclient client.Client, src, dst, prev *corev1.PersistentVolumeClaim) error { +func updatePVC(ctx context.Context, rclient client.Client, src, dst *corev1.PersistentVolumeClaim) error { srcSize := src.Spec.Resources.Requests.Storage() dstSize := dst.Spec.Resources.Requests.Storage() if srcSize == nil || dstSize == nil { return nil } direction := dstSize.Cmp(*srcSize) - if direction == 0 && equality.Semantic.DeepEqual(src.Labels, dst.Labels) && isObjectMetaEqual(src, dst, prev) { + if direction == 0 && + equality.Semantic.DeepEqual(src.Labels, dst.Labels) && + equality.Semantic.DeepEqual(src.Annotations, dst.Annotations) { return nil } @@ -164,7 +166,7 @@ func updatePVC(ctx context.Context, rclient client.Client, src, dst, prev *corev return nil } - l.Info(fmt.Sprintf("need to expand pvc=%s size from=%s to=%s", dst.Name, srcSize, dstSize)) + l.Info(fmt.Sprintf("need to expand PVC=%s size from=%s to=%s", dst.Name, srcSize, dstSize)) if !expandable { // check if storage class is expandable var err error @@ -182,7 +184,6 @@ func updatePVC(ctx context.Context, rclient client.Client, src, dst, prev *corev pvc.Spec.Resources = *dst.Spec.Resources.DeepCopy() } vmv1beta1.AddFinalizer(pvc, src) - mergeObjectMetadataIntoNew(dst, pvc, prev) if err := rclient.Update(ctx, pvc); err != nil { return fmt.Errorf("failed to expand size for pvc %s: %v", dst.Name, err) } @@ -316,9 +317,9 @@ func removeStatefulSetKeepPods(ctx context.Context, rclient client.Client, state // try to restore previous one and throw error oldStatefulSet.ResourceVersion = "" if err2 := rclient.Create(ctx, oldStatefulSet); err2 != nil { - return fmt.Errorf("cannot restore previous sts: %s configuration after remove original error: %s: restore error %w", oldStatefulSet.Name, err, err2) + return fmt.Errorf("cannot restore previous StatefulSet=%s configuration after remove original error: %s: restore error %w", nsn, err, err2) } - return fmt.Errorf("cannot create new sts: %s instead of replaced, perform manual action to handle this error or report BUG, err: %w", statefulSet.Name, err) + return fmt.Errorf("cannot create new StatefulSet=%s instead of replaced, perform manual action to handle this error or report BUG: %w", nsn, err) } return nil } diff --git a/internal/controller/operator/factory/reconcile/statefulset_test.go b/internal/controller/operator/factory/reconcile/statefulset_test.go index ade9dace3..b12dfcedd 100644 --- a/internal/controller/operator/factory/reconcile/statefulset_test.go +++ b/internal/controller/operator/factory/reconcile/statefulset_test.go @@ -262,8 +262,7 @@ func Test_podIsReady(t *testing.T) { func Test_performRollingUpdateOnSts(t *testing.T) { type opts struct { - stsName string - ns string + nsn types.NamespacedName podLabels map[string]string podMaxUnavailable int wantErr bool @@ -272,16 +271,17 @@ func Test_performRollingUpdateOnSts(t *testing.T) { f := func(o opts) { t.Helper() fclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) - - if err := performRollingUpdateOnSts(context.Background(), false, fclient, o.stsName, o.ns, o.podLabels, o.podMaxUnavailable); (err != nil) != o.wantErr { + if err := performRollingUpdateOnSts(context.Background(), false, fclient, o.nsn, o.podLabels, o.podMaxUnavailable); (err != nil) != o.wantErr { t.Errorf("performRollingUpdateOnSts() error = %v, wantErr %v", err, o.wantErr) } } // rolling update is not needed f(opts{ - stsName: "vmselect-sts", - ns: "default", + nsn: types.NamespacedName{ + Name: "vmselect-sts", + Namespace: "default", + }, podLabels: map[string]string{"app": "vmselect"}, podMaxUnavailable: 1, predefinedObjects: []runtime.Object{ @@ -317,8 +317,10 @@ func Test_performRollingUpdateOnSts(t *testing.T) { // rolling update is timeout f(opts{ - stsName: "vmselect-sts", - ns: "default", + nsn: types.NamespacedName{ + Name: "vmselect-sts", + Namespace: "default", + }, podLabels: map[string]string{"app": "vmselect"}, podMaxUnavailable: 1, predefinedObjects: []runtime.Object{ diff --git a/internal/controller/operator/factory/reconcile/vmscrape.go b/internal/controller/operator/factory/reconcile/vmscrape.go new file mode 100644 index 000000000..5e7b88eeb --- /dev/null +++ b/internal/controller/operator/factory/reconcile/vmscrape.go @@ -0,0 +1,87 @@ +package reconcile + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/api/equality" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "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/build" + "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger" +) + +// VMServiceScrapeForCRD creates or updates given object +func VMServiceScrapeForCRD(ctx context.Context, rclient client.Client, newObj *vmv1beta1.VMServiceScrape) error { + if build.IsControllerDisabled("VMServiceScrape") { + return nil + } + nsn := types.NamespacedName{Namespace: newObj.Namespace, Name: newObj.Name} + return retryOnConflict(func() error { + var existingObj vmv1beta1.VMServiceScrape + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { + if k8serrors.IsNotFound(err) { + logger.WithContext(ctx).Info(fmt.Sprintf("creating VMServiceScrape=%s", nsn)) + return rclient.Create(ctx, newObj) + } + return err + } + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { + return err + } + + if equality.Semantic.DeepEqual(newObj.Spec, existingObj.Spec) && + equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) && + equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) { + return nil + } + // TODO: @f41gh7 allow 3rd party applications to add annotations for generated VMServiceScrape + specDiff := diffDeep(newObj.Spec, existingObj.Spec) + existingObj.Annotations = newObj.Annotations + existingObj.Spec = newObj.Spec + existingObj.Labels = newObj.Labels + logMsg := fmt.Sprintf("updating VMServiceScrape=%s for CRD object spec_diff: %s", nsn, specDiff) + logger.WithContext(ctx).Info(logMsg) + + return rclient.Update(ctx, &existingObj) + }) +} + +// VMPodScrapeForCRD creates or updates given object +func VMPodScrapeForCRD(ctx context.Context, rclient client.Client, newObj *vmv1beta1.VMPodScrape) error { + if build.IsControllerDisabled("VMPodScrape") { + return nil + } + nsn := types.NamespacedName{Namespace: newObj.Namespace, Name: newObj.Name} + return retryOnConflict(func() error { + var existingObj vmv1beta1.VMPodScrape + if err := rclient.Get(ctx, nsn, &existingObj); err != nil { + if k8serrors.IsNotFound(err) { + logger.WithContext(ctx).Info(fmt.Sprintf("creating VMPodScrape=%s", nsn)) + return rclient.Create(ctx, newObj) + } + return err + } + if err := freeIfNeeded(ctx, rclient, &existingObj); err != nil { + return err + } + + if equality.Semantic.DeepEqual(newObj.Spec, existingObj.Spec) && + equality.Semantic.DeepEqual(newObj.Labels, existingObj.Labels) && + equality.Semantic.DeepEqual(newObj.Annotations, existingObj.Annotations) { + return nil + } + // TODO: @f41gh7 allow 3rd party applications to add annotations for generated VMPodScrape + specDiff := diffDeep(newObj.Spec, existingObj.Spec) + existingObj.Annotations = newObj.Annotations + existingObj.Spec = newObj.Spec + existingObj.Labels = newObj.Labels + logMsg := fmt.Sprintf("updating VMPodScrape=%s for CRD object spec_diff: %s", nsn, specDiff) + logger.WithContext(ctx).Info(logMsg) + + return rclient.Update(ctx, &existingObj) + }) +} diff --git a/internal/controller/operator/factory/reconcile/vmservicescrape.go b/internal/controller/operator/factory/reconcile/vmservicescrape.go deleted file mode 100644 index 0985f4ea4..000000000 --- a/internal/controller/operator/factory/reconcile/vmservicescrape.go +++ /dev/null @@ -1,93 +0,0 @@ -package reconcile - -import ( - "context" - "fmt" - - "k8s.io/apimachinery/pkg/api/equality" - k8serrors "k8s.io/apimachinery/pkg/api/errors" - "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/build" - "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/finalize" - "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger" -) - -// VMServiceScrapeForCRD creates or updates given object -func VMServiceScrapeForCRD(ctx context.Context, rclient client.Client, vss *vmv1beta1.VMServiceScrape) error { - if build.IsControllerDisabled("VMServiceScrape") { - return nil - } - return retryOnConflict(func() error { - var existVSS vmv1beta1.VMServiceScrape - err := rclient.Get(ctx, types.NamespacedName{Namespace: vss.Namespace, Name: vss.Name}, &existVSS) - if err != nil { - if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating VMServiceScrape %s", vss.Name)) - return rclient.Create(ctx, vss) - } - return err - } - if !existVSS.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, &existVSS) - } - if err := finalize.FreeIfNeeded(ctx, rclient, &existVSS); err != nil { - return err - } - - if equality.Semantic.DeepEqual(vss.Spec, existVSS.Spec) && - equality.Semantic.DeepEqual(vss.Labels, existVSS.Labels) && - equality.Semantic.DeepEqual(vss.Annotations, existVSS.Annotations) { - return nil - } - // TODO: @f41gh7 allow 3rd party applications to add annotations for generated VMServiceScrape - specDiff := diffDeep(vss.Spec, existVSS.Spec) - existVSS.Annotations = vss.Annotations - existVSS.Spec = vss.Spec - existVSS.Labels = vss.Labels - logMsg := fmt.Sprintf("updating VMServiceScrape %s for CRD object spec_diff: %s", vss.Name, specDiff) - logger.WithContext(ctx).Info(logMsg) - - return rclient.Update(ctx, &existVSS) - }) -} - -// VMPodScrapeForCRD creates or updates given object -func VMPodScrapeForCRD(ctx context.Context, rclient client.Client, vps *vmv1beta1.VMPodScrape) error { - if build.IsControllerDisabled("VMPodScrape") { - return nil - } - return retryOnConflict(func() error { - var existVPS vmv1beta1.VMPodScrape - err := rclient.Get(ctx, types.NamespacedName{Namespace: vps.Namespace, Name: vps.Name}, &existVPS) - if err != nil { - if k8serrors.IsNotFound(err) { - logger.WithContext(ctx).Info(fmt.Sprintf("creating VMPodScrape %s", vps.Name)) - return rclient.Create(ctx, vps) - } - return err - } - if !existVPS.DeletionTimestamp.IsZero() { - return newErrRecreate(ctx, &existVPS) - } - if err := finalize.FreeIfNeeded(ctx, rclient, &existVPS); err != nil { - return err - } - - if equality.Semantic.DeepEqual(vps.Spec, existVPS.Spec) && - equality.Semantic.DeepEqual(vps.Labels, existVPS.Labels) && - equality.Semantic.DeepEqual(vps.Annotations, existVPS.Annotations) { - return nil - } - // TODO: @f41gh7 allow 3rd party applications to add annotations for generated VMPodScrape - existVPS.Annotations = vps.Annotations - existVPS.Spec = vps.Spec - existVPS.Labels = vps.Labels - logMsg := fmt.Sprintf("updating VMPodScrape %s for CRD object spec_diff: %s", vps.Name, diffDeep(vps.Spec, existVPS.Spec)) - logger.WithContext(ctx).Info(logMsg) - - return rclient.Update(ctx, &existVPS) - }) -} diff --git a/internal/controller/operator/factory/vlagent/vlagent.go b/internal/controller/operator/factory/vlagent/vlagent.go index 7e1807dae..ea42db539 100644 --- a/internal/controller/operator/factory/vlagent/vlagent.go +++ b/internal/controller/operator/factory/vlagent/vlagent.go @@ -84,11 +84,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1.VLAgent, rclient client.Client } } if cr.IsOwnsServiceAccount() { - var prevSA *corev1.ServiceAccount - if prevCR != nil { - prevSA = build.ServiceAccount(prevCR) - } - if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr)); err != nil { return fmt.Errorf("failed create service account: %w", err) } if cr.Spec.K8sCollector.Enabled { @@ -109,11 +105,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1.VLAgent, rclient client.Client } if cr.Spec.PodDisruptionBudget != nil && !cr.Spec.K8sCollector.Enabled { - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.PodDisruptionBudget != nil { - prevPDB = build.PodDisruptionBudget(prevCR, prevCR.Spec.PodDisruptionBudget) - } - if err := reconcile.PDB(ctx, rclient, build.PodDisruptionBudget(cr, cr.Spec.PodDisruptionBudget), prevPDB); err != nil { + if err := reconcile.PDB(ctx, rclient, build.PodDisruptionBudget(cr, cr.Spec.PodDisruptionBudget)); err != nil { return fmt.Errorf("cannot update pod disruption budget for vlagent: %w", err) } } @@ -135,11 +127,7 @@ func createOrUpdateDeploy(ctx context.Context, rclient client.Client, cr, prevCR } switch newApp := newAppObj.(type) { case *appsv1.DaemonSet: - var prevApp *appsv1.DaemonSet - if prevAppObj != nil { - prevApp, _ = prevAppObj.(*appsv1.DaemonSet) - } - if err := reconcile.DaemonSet(ctx, rclient, newApp, prevApp); err != nil { + if err := reconcile.DaemonSet(ctx, rclient, newApp); err != nil { return fmt.Errorf("cannot reconcile daemonset for vlagent: %w", err) } return nil diff --git a/internal/controller/operator/factory/vlcluster/vlcluster.go b/internal/controller/operator/factory/vlcluster/vlcluster.go index 9c8c65143..77822d0fe 100644 --- a/internal/controller/operator/factory/vlcluster/vlcluster.go +++ b/internal/controller/operator/factory/vlcluster/vlcluster.go @@ -31,12 +31,7 @@ func CreateOrUpdate(ctx context.Context, rclient client.Client, cr *vmv1.VLClust if cr.IsOwnsServiceAccount() { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentRoot) sa := build.ServiceAccount(b) - var prevSA *corev1.ServiceAccount - if prevCR != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentRoot) - prevSA = build.ServiceAccount(b) - } - if err := reconcile.ServiceAccount(ctx, rclient, sa, prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, sa); err != nil { return fmt.Errorf("failed create service account: %w", err) } } diff --git a/internal/controller/operator/factory/vlcluster/vlinsert.go b/internal/controller/operator/factory/vlcluster/vlinsert.go index 7c1505139..a966065db 100644 --- a/internal/controller/operator/factory/vlcluster/vlinsert.go +++ b/internal/controller/operator/factory/vlcluster/vlinsert.go @@ -9,7 +9,6 @@ import ( appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" @@ -28,7 +27,7 @@ func createOrUpdateVLInsert(ctx context.Context, rclient client.Client, cr, prev return nil } - if err := createOrUpdatePodDisruptionBudgetForVLInsert(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdatePodDisruptionBudgetForVLInsert(ctx, rclient, cr); err != nil { return err } if err := createOrUpdateVLInsertDeployment(ctx, rclient, cr, prevCR); err != nil { @@ -38,7 +37,7 @@ func createOrUpdateVLInsert(ctx context.Context, rclient client.Client, cr, prev if err != nil { return err } - if err := createOrUpdateVLInsertHPA(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateVLInsertHPA(ctx, rclient, cr); err != nil { return err } if !ptr.Deref(cr.Spec.VLInsert.DisableSelfServiceScrape, false) { @@ -56,18 +55,13 @@ func createOrUpdateVLInsert(ctx context.Context, rclient client.Client, cr, prev return nil } -func createOrUpdatePodDisruptionBudgetForVLInsert(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLCluster) error { +func createOrUpdatePodDisruptionBudgetForVLInsert(ctx context.Context, rclient client.Client, cr *vmv1.VLCluster) error { if cr.Spec.VLInsert.PodDisruptionBudget == nil { return nil } b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentInsert) pdb := build.PodDisruptionBudget(b, cr.Spec.VLInsert.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.VLInsert.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentInsert) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.VLInsert.PodDisruptionBudget) - } - return reconcile.PDB(ctx, rclient, pdb, prevPDB) + return reconcile.PDB(ctx, rclient, pdb) } func createOrUpdateVLInsertDeployment(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLCluster) error { @@ -265,7 +259,7 @@ func buildVLInsertPodSpec(cr *vmv1.VLCluster) (*corev1.PodTemplateSpec, error) { return podSpec, nil } -func createOrUpdateVLInsertHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLCluster) error { +func createOrUpdateVLInsertHPA(ctx context.Context, rclient client.Client, cr *vmv1.VLCluster) error { if cr.Spec.VLInsert.HPA == nil { return nil } @@ -276,12 +270,7 @@ func createOrUpdateVLInsertHPA(ctx context.Context, rclient client.Client, cr, p } b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentInsert) newHPA := build.HPA(b, targetRef, cr.Spec.VLInsert.HPA) - var prevHPA *autoscalingv2.HorizontalPodAutoscaler - if prevCR != nil && prevCR.Spec.VLInsert.HPA != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentInsert) - prevHPA = build.HPA(b, targetRef, prevCR.Spec.VLInsert.HPA) - } - return reconcile.HPA(ctx, rclient, newHPA, prevHPA) + return reconcile.HPA(ctx, rclient, newHPA) } func createOrUpdateVLInsertService(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLCluster) (*corev1.Service, error) { diff --git a/internal/controller/operator/factory/vlcluster/vlselect.go b/internal/controller/operator/factory/vlcluster/vlselect.go index 96e73173c..e926be9d2 100644 --- a/internal/controller/operator/factory/vlcluster/vlselect.go +++ b/internal/controller/operator/factory/vlcluster/vlselect.go @@ -9,7 +9,6 @@ import ( appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" @@ -30,17 +29,11 @@ func createOrUpdateVLSelect(ctx context.Context, rclient client.Client, cr, prev if cr.Spec.VLSelect.PodDisruptionBudget != nil { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentSelect) pdb := build.PodDisruptionBudget(b, cr.Spec.VLSelect.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.VLSelect.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentSelect) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.VLSelect.PodDisruptionBudget) - } - err := reconcile.PDB(ctx, rclient, pdb, prevPDB) - if err != nil { + if err := reconcile.PDB(ctx, rclient, pdb); err != nil { return err } } - if err := createOrUpdateVLSelectHPA(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateVLSelectHPA(ctx, rclient, cr); err != nil { return err } selectSvc, err := createOrUpdateVLSelectService(ctx, rclient, cr, prevCR) @@ -64,7 +57,7 @@ func createOrUpdateVLSelect(ctx context.Context, rclient client.Client, cr, prev return nil } -func createOrUpdateVLSelectHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLCluster) error { +func createOrUpdateVLSelectHPA(ctx context.Context, rclient client.Client, cr *vmv1.VLCluster) error { if cr.Spec.VLSelect.HPA == nil { return nil } @@ -75,12 +68,7 @@ func createOrUpdateVLSelectHPA(ctx context.Context, rclient client.Client, cr, p } b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentSelect) defaultHPA := build.HPA(b, targetRef, cr.Spec.VLSelect.HPA) - var prevHPA *autoscalingv2.HorizontalPodAutoscaler - if prevCR != nil && prevCR.Spec.VLSelect.HPA != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentSelect) - prevHPA = build.HPA(b, targetRef, prevCR.Spec.VLSelect.HPA) - } - return reconcile.HPA(ctx, rclient, defaultHPA, prevHPA) + return reconcile.HPA(ctx, rclient, defaultHPA) } func createOrUpdateVLSelectService(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLCluster) (*corev1.Service, error) { diff --git a/internal/controller/operator/factory/vlcluster/vlstorage.go b/internal/controller/operator/factory/vlcluster/vlstorage.go index eefaae5cf..0eedfed64 100644 --- a/internal/controller/operator/factory/vlcluster/vlstorage.go +++ b/internal/controller/operator/factory/vlcluster/vlstorage.go @@ -9,7 +9,6 @@ import ( appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" @@ -30,17 +29,11 @@ func createOrUpdateVLStorage(ctx context.Context, rclient client.Client, cr, pre if cr.Spec.VLStorage.PodDisruptionBudget != nil { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentStorage) pdb := build.PodDisruptionBudget(b, cr.Spec.VLStorage.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.VLStorage.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentStorage) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.VLStorage.PodDisruptionBudget) - } - err := reconcile.PDB(ctx, rclient, pdb, prevPDB) - if err != nil { + if err := reconcile.PDB(ctx, rclient, pdb); err != nil { return err } } - if err := createOrUpdateVLStorageHPA(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateVLStorageHPA(ctx, rclient, cr); err != nil { return err } if err := createOrUpdateVLStorageSTS(ctx, rclient, cr, prevCR); err != nil { @@ -95,7 +88,7 @@ func createOrUpdateVLStorageService(ctx context.Context, rclient client.Client, return newHeadless, nil } -func createOrUpdateVLStorageHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLCluster) error { +func createOrUpdateVLStorageHPA(ctx context.Context, rclient client.Client, cr *vmv1.VLCluster) error { hpa := cr.Spec.VLStorage.HPA if hpa == nil { return nil @@ -107,13 +100,7 @@ func createOrUpdateVLStorageHPA(ctx context.Context, rclient client.Client, cr, APIVersion: "apps/v1", } defaultHPA := build.HPA(b, targetRef, hpa) - var prevHPA *autoscalingv2.HorizontalPodAutoscaler - if prevCR != nil && prevCR.Spec.VLStorage.HPA != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentStorage) - prevHPA = build.HPA(b, targetRef, prevCR.Spec.VLStorage.HPA) - } - - return reconcile.HPA(ctx, rclient, defaultHPA, prevHPA) + return reconcile.HPA(ctx, rclient, defaultHPA) } func createOrUpdateVLStorageSTS(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLCluster) error { diff --git a/internal/controller/operator/factory/vlcluster/vmauth_lb.go b/internal/controller/operator/factory/vlcluster/vmauth_lb.go index 4d907f64e..615d2dbc7 100644 --- a/internal/controller/operator/factory/vlcluster/vmauth_lb.go +++ b/internal/controller/operator/factory/vlcluster/vmauth_lb.go @@ -8,7 +8,6 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" @@ -24,12 +23,7 @@ import ( ) func createOrUpdateVMAuthLB(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLCluster) error { - - var prevSecretMeta *metav1.ObjectMeta - if prevCR != nil { - prevSecretMeta = ptr.To(buildLBConfigSecretMeta(prevCR)) - } - if err := reconcile.Secret(ctx, rclient, buildVMauthLBSecret(cr), prevSecretMeta); err != nil { + if err := reconcile.Secret(ctx, rclient, buildVMauthLBSecret(cr)); err != nil { return fmt.Errorf("cannot reconcile vmauth lb secret: %w", err) } lbDep, err := buildVMauthLBDeployment(cr) @@ -50,7 +44,7 @@ func createOrUpdateVMAuthLB(ctx context.Context, rclient client.Client, cr, prev return err } if cr.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget != nil { - if err := createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx, rclient, cr); err != nil { return fmt.Errorf("cannot create or update PodDisruptionBudget for vmauth lb: %w", err) } } @@ -260,15 +254,10 @@ func createOrUpdateVMAuthLBService(ctx context.Context, rclient client.Client, c return nil } -func createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLCluster) error { +func createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx context.Context, rclient client.Client, cr *vmv1.VLCluster) error { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentBalancer) pdb := build.PodDisruptionBudget(b, cr.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentBalancer) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget) - } - return reconcile.PDB(ctx, rclient, pdb, prevPDB) + return reconcile.PDB(ctx, rclient, pdb) } // createOrUpdateLBProxyService builds vlinsert and vlselect external services to expose vlcluster components for access by vmauth diff --git a/internal/controller/operator/factory/vlsingle/vlogs.go b/internal/controller/operator/factory/vlsingle/vlogs.go index 6f496ef33..250df04d3 100644 --- a/internal/controller/operator/factory/vlsingle/vlogs.go +++ b/internal/controller/operator/factory/vlsingle/vlogs.go @@ -24,15 +24,6 @@ import ( // VLogs component is deprecated and will be transited into no-op // TODO: transit it into no-op at v0.60.0 -func createOrUpdateVLogsPVC(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VLogs) error { - newPvc := newVLogsPVC(cr) - var prevPVC *corev1.PersistentVolumeClaim - if prevCR != nil && prevCR.Spec.Storage != nil { - prevPVC = newVLogsPVC(prevCR) - } - return reconcile.PersistentVolumeClaim(ctx, rclient, newPvc, prevPVC) -} - func newVLogsPVC(r *vmv1beta1.VLogs) *corev1.PersistentVolumeClaim { pvcObject := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -65,17 +56,13 @@ func CreateOrUpdateVLogs(ctx context.Context, rclient client.Client, cr *vmv1bet } } if cr.Spec.Storage != nil && cr.Spec.StorageDataPath == "" { - if err := createOrUpdateVLogsPVC(ctx, rclient, cr, prevCR); err != nil { + if err := reconcile.PersistentVolumeClaim(ctx, rclient, newVLogsPVC(cr)); err != nil { return err } } if cr.IsOwnsServiceAccount() { - var prevSA *corev1.ServiceAccount - if prevCR != nil { - prevSA = build.ServiceAccount(prevCR) - } - if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr)); err != nil { return fmt.Errorf("failed create service account: %w", err) } } diff --git a/internal/controller/operator/factory/vlsingle/vlsingle.go b/internal/controller/operator/factory/vlsingle/vlsingle.go index c200606d3..907cad143 100644 --- a/internal/controller/operator/factory/vlsingle/vlsingle.go +++ b/internal/controller/operator/factory/vlsingle/vlsingle.go @@ -28,15 +28,6 @@ const ( tlsServerConfigMountPath = "/etc/vm/tls-server-secrets" ) -func createOrUpdatePVC(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VLSingle) error { - newPvc := newPVC(cr) - var prevPVC *corev1.PersistentVolumeClaim - if prevCR != nil && prevCR.Spec.Storage != nil { - prevPVC = newPVC(prevCR) - } - return reconcile.PersistentVolumeClaim(ctx, rclient, newPvc, prevPVC) -} - func newPVC(r *vmv1.VLSingle) *corev1.PersistentVolumeClaim { pvcObject := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -69,17 +60,13 @@ func CreateOrUpdate(ctx context.Context, rclient client.Client, cr *vmv1.VLSingl } } if cr.Spec.Storage != nil { - if err := createOrUpdatePVC(ctx, rclient, cr, prevCR); err != nil { + if err := reconcile.PersistentVolumeClaim(ctx, rclient, newPVC(cr)); err != nil { return err } } if cr.IsOwnsServiceAccount() { - var prevSA *corev1.ServiceAccount - if prevCR != nil { - prevSA = build.ServiceAccount(prevCR) - } - if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr)); err != nil { return fmt.Errorf("failed create service account: %w", err) } } diff --git a/internal/controller/operator/factory/vmagent/vmagent.go b/internal/controller/operator/factory/vmagent/vmagent.go index f61ead614..de1a7574d 100644 --- a/internal/controller/operator/factory/vmagent/vmagent.go +++ b/internal/controller/operator/factory/vmagent/vmagent.go @@ -13,7 +13,6 @@ import ( "gopkg.in/yaml.v2" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -98,11 +97,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAgent, rclient client.C } } if cr.IsOwnsServiceAccount() { - var prevSA *corev1.ServiceAccount - if prevCR != nil { - prevSA = build.ServiceAccount(prevCR) - } - if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr)); err != nil { return fmt.Errorf("failed create service account: %w", err) } if !ptr.Deref(cr.Spec.IngestOnlyMode, false) { @@ -129,15 +124,15 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAgent, rclient client.C } ac := getAssetsCache(ctx, rclient, cr) - if err = createOrUpdateScrapeConfig(ctx, rclient, cr, prevCR, nil, ac); err != nil { + if err = createOrUpdateScrapeConfig(ctx, rclient, cr, nil, ac); err != nil { return err } - if err = createOrUpdateRelabelConfigsAssets(ctx, rclient, cr, prevCR, ac); err != nil { + if err = createOrUpdateRelabelConfigsAssets(ctx, rclient, cr, ac); err != nil { return fmt.Errorf("cannot update relabeling asset for vmagent: %w", err) } - if err = createOrUpdateStreamAggrConfig(ctx, rclient, cr, prevCR, ac); err != nil { + if err = createOrUpdateStreamAggrConfig(ctx, rclient, cr, ac); err != nil { return fmt.Errorf("cannot update stream aggregation config for vmagent: %w", err) } @@ -194,11 +189,7 @@ func createOrUpdateApp(ctx context.Context, rclient client.Client, cr, prevCR *v if cr.Spec.PodDisruptionBudget != nil && !cr.Spec.DaemonSetMode { pdb := build.ShardPodDisruptionBudget(cr, cr.Spec.PodDisruptionBudget, shardNum) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.PodDisruptionBudget != nil { - prevPDB = build.ShardPodDisruptionBudget(prevCR, prevCR.Spec.PodDisruptionBudget, shardNum) - } - if err := reconcile.PDB(ctx, rclient, pdb, prevPDB); err != nil { + if err := reconcile.PDB(ctx, rclient, pdb); err != nil { rv.err = err return } @@ -289,11 +280,7 @@ func createOrUpdateApp(ctx context.Context, rclient client.Client, cr, prevCR *v rv.stsName = newApp.Name case *appsv1.DaemonSet: newApp := newAppTpl - var prevApp *appsv1.DaemonSet - if prevAppTpl != nil { - prevApp, _ = prevAppTpl.(*appsv1.DaemonSet) - } - if err := reconcile.DaemonSet(ctx, rclient, newApp, prevApp); err != nil { + if err := reconcile.DaemonSet(ctx, rclient, newApp); err != nil { rv.err = fmt.Errorf("cannot reconcile daemonset for vmagent: %w", err) return } @@ -794,7 +781,7 @@ func buildRelabelingsAssets(cr *vmv1beta1.VMAgent, ac *build.AssetsCache) (*core } // createOrUpdateRelabelConfigsAssets builds relabeling configs for vmagent at separate configmap, serialized as yaml -func createOrUpdateRelabelConfigsAssets(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent, ac *build.AssetsCache) error { +func createOrUpdateRelabelConfigsAssets(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAgent, ac *build.AssetsCache) error { if !cr.HasAnyRelabellingConfigs() { return nil } @@ -802,11 +789,8 @@ func createOrUpdateRelabelConfigsAssets(ctx context.Context, rclient client.Clie if err != nil { return err } - var prevConfigMeta *metav1.ObjectMeta - if prevCR != nil { - prevConfigMeta = ptr.To(build.ResourceMeta(build.RelabelConfigResourceKind, prevCR)) - } - return reconcile.ConfigMap(ctx, rclient, assetsCM, prevConfigMeta) + _, err = reconcile.ConfigMap(ctx, rclient, assetsCM) + return err } // buildStreamAggrConfig combines all possible stream aggregation configs and adding it to the configmap. @@ -867,7 +851,7 @@ func buildStreamAggrConfig(cr *vmv1beta1.VMAgent, ac *build.AssetsCache) (*corev } // createOrUpdateStreamAggrConfig builds stream aggregation configs for vmagent at separate configmap, serialized as yaml -func createOrUpdateStreamAggrConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent, ac *build.AssetsCache) error { +func createOrUpdateStreamAggrConfig(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAgent, ac *build.AssetsCache) error { // fast path if !cr.HasAnyStreamAggrRule() { return nil @@ -876,11 +860,8 @@ func createOrUpdateStreamAggrConfig(ctx context.Context, rclient client.Client, if err != nil { return err } - var prevConfigMeta *metav1.ObjectMeta - if prevCR != nil { - prevConfigMeta = ptr.To(build.ResourceMeta(build.StreamAggrConfigResourceKind, cr)) - } - return reconcile.ConfigMap(ctx, rclient, streamAggrCM, prevConfigMeta) + _, err = reconcile.ConfigMap(ctx, rclient, streamAggrCM) + return err } type item struct { diff --git a/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig.go b/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig.go index cd8f76cd4..290a44d49 100644 --- a/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig.go +++ b/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig.go @@ -23,19 +23,14 @@ import ( // CreateOrUpdateScrapeConfig builds scrape configuration for VMAgent func CreateOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAgent, childObject client.Object) error { - var prevCR *vmv1beta1.VMAgent - if cr.ParsedLastAppliedSpec != nil { - prevCR = cr.DeepCopy() - prevCR.Spec = *cr.ParsedLastAppliedSpec - } ac := getAssetsCache(ctx, rclient, cr) - if err := createOrUpdateScrapeConfig(ctx, rclient, cr, prevCR, childObject, ac); err != nil { + if err := createOrUpdateScrapeConfig(ctx, rclient, cr, childObject, ac); err != nil { return err } return nil } -func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAgent, childObject client.Object, ac *build.AssetsCache) error { +func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAgent, childObject client.Object, ac *build.AssetsCache) error { if ptr.Deref(cr.Spec.IngestOnlyMode, false) { return nil } @@ -79,10 +74,6 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, } for kind, secret := range ac.GetOutput() { - var prevSecretMeta *metav1.ObjectMeta - if prevCR != nil { - prevSecretMeta = ptr.To(build.ResourceMeta(kind, prevCR)) - } if kind == build.SecretConfigResourceKind { // Compress config to avoid 1mb secret limit for a while data, err := build.GzipConfig(generatedConfig) @@ -95,7 +86,7 @@ func createOrUpdateScrapeConfig(ctx context.Context, rclient client.Client, cr, secret.Annotations = map[string]string{ "generated": "true", } - if err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta); err != nil { + if err := reconcile.Secret(ctx, rclient, &secret); err != nil { return err } } diff --git a/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig_test.go b/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig_test.go index 5b5957c68..344b21258 100644 --- a/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig_test.go +++ b/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig_test.go @@ -123,7 +123,7 @@ func TestCreateOrUpdateScrapeConfig(t *testing.T) { } build.AddDefaults(testClient.Scheme()) ac := getAssetsCache(ctx, testClient, o.cr) - if err := createOrUpdateScrapeConfig(ctx, testClient, o.cr, nil, nil, ac); err != nil { + if err := createOrUpdateScrapeConfig(ctx, testClient, o.cr, nil, ac); err != nil { t.Errorf("CreateOrUpdateConfigurationSecret() error = %v", err) } var expectSecret corev1.Secret @@ -2230,7 +2230,7 @@ scrape_configs: [] }, } ac := getAssetsCache(ctx, testClient, cr) - if err := createOrUpdateScrapeConfig(ctx, testClient, cr, nil, nil, ac); err != nil { + if err := createOrUpdateScrapeConfig(ctx, testClient, cr, nil, ac); err != nil { t.Errorf("CreateOrUpdateConfigurationSecret() error = %s", err) } var configSecret corev1.Secret diff --git a/internal/controller/operator/factory/vmagent/vmagent_test.go b/internal/controller/operator/factory/vmagent/vmagent_test.go index 7f6022b99..355ab0b18 100644 --- a/internal/controller/operator/factory/vmagent/vmagent_test.go +++ b/internal/controller/operator/factory/vmagent/vmagent_test.go @@ -1898,7 +1898,7 @@ func TestCreateOrUpdateRelabelConfigsAssets(t *testing.T) { cl := k8stools.GetTestClientWithObjects(o.predefinedObjects) ctx := context.TODO() ac := build.NewAssetsCache(ctx, cl, nil) - if err := createOrUpdateRelabelConfigsAssets(ctx, cl, o.cr, nil, ac); err != nil { + if err := createOrUpdateRelabelConfigsAssets(ctx, cl, o.cr, ac); err != nil { t.Fatalf("CreateOrUpdateRelabelConfigsAssets() error = %v", err) } var createdCM corev1.ConfigMap @@ -2011,7 +2011,7 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { cl := k8stools.GetTestClientWithObjects(o.predefinedObjects) ctx := context.TODO() ac := build.NewAssetsCache(ctx, cl, nil) - if err := createOrUpdateStreamAggrConfig(ctx, cl, o.cr, nil, ac); err != nil { + if err := createOrUpdateStreamAggrConfig(ctx, cl, o.cr, ac); err != nil { t.Fatalf("CreateOrUpdateStreamAggrConfig() error = %v", err) } var createdCM corev1.ConfigMap diff --git a/internal/controller/operator/factory/vmalert/rules.go b/internal/controller/operator/factory/vmalert/rules.go index 2d7a25e35..d64b830c3 100644 --- a/internal/controller/operator/factory/vmalert/rules.go +++ b/internal/controller/operator/factory/vmalert/rules.go @@ -9,16 +9,11 @@ import ( "gopkg.in/yaml.v2" 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" "sigs.k8s.io/controller-runtime/pkg/client" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/build" - "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/finalize" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/reconcile" @@ -48,68 +43,21 @@ func CreateOrUpdateRuleConfigMaps(ctx context.Context, rclient client.Client, cr func reconcileConfigsData(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAlert, newRules map[string]string) ([]string, error) { newConfigMaps := makeRulesConfigMaps(cr, newRules) - currentCMs := make([]corev1.ConfigMap, len(newConfigMaps)) - for idx, cm := range newConfigMaps { - var existCM corev1.ConfigMap - if err := rclient.Get(ctx, types.NamespacedName{Namespace: cm.Namespace, Name: cm.Name}, &existCM); err != nil { - if k8serrors.IsNotFound(err) { - continue - } - return nil, err - } - currentCMs[idx] = existCM - } - - newConfigMapNames := make([]string, 0, len(newConfigMaps)) - for _, cm := range newConfigMaps { - newConfigMapNames = append(newConfigMapNames, cm.Name) - } - - if len(currentCMs) == 0 { - for _, cm := range newConfigMaps { - logger.WithContext(ctx).Info(fmt.Sprintf("creating new ConfigMap %s for rules", cm.Name)) - err := rclient.Create(ctx, &cm) - if err != nil { - if k8serrors.IsAlreadyExists(err) { - continue - } - return nil, fmt.Errorf("failed to create Configmap: %s, err: %w", cm.Name, err) - } - } - return newConfigMapNames, nil - } - - // sort - sort.Strings(newConfigMapNames) - sort.Slice(currentCMs, func(i, j int) bool { - return currentCMs[i].Name < currentCMs[j].Name - }) sort.Slice(newConfigMaps, func(i, j int) bool { return newConfigMaps[i].Name < newConfigMaps[j].Name }) - - // compute diff for current and needed rules configmaps. - toCreate, toUpdate := rulesCMDiff(currentCMs, newConfigMaps) - for _, cm := range toCreate { - logger.WithContext(ctx).Info(fmt.Sprintf("creating additional configmap=%s for rules", cm.Name)) - if err := rclient.Create(ctx, &cm); err != nil { - if k8serrors.IsAlreadyExists(err) { - continue - } - return nil, fmt.Errorf("failed to create new rules Configmap: %s, err: %w", cm.Name, err) - } - } - for _, cm := range toUpdate { - if err := finalize.FreeIfNeeded(ctx, rclient, &cm); err != nil { + var needReload bool + newConfigMapNames := make([]string, 0, len(newConfigMaps)) + for i := range newConfigMaps { + cm := &newConfigMaps[i] + if updated, err := reconcile.ConfigMap(ctx, rclient, cm); err != nil { return nil, err + } else if updated { + needReload = true } - logger.WithContext(ctx).Info(fmt.Sprintf("updating ConfigMap %s configuration", cm.Name)) - if err := rclient.Update(ctx, &cm); err != nil { - return nil, fmt.Errorf("failed to update rules Configmap: %s, err: %w", cm.Name, err) - } + newConfigMapNames = append(newConfigMapNames, cm.Name) } - - if len(toCreate) > 0 || len(toUpdate) > 0 { + if needReload { // trigger sync for configmap logger.WithContext(ctx).Info("triggering pod config reload by changing annotation") if err := k8stools.UpdatePodAnnotations(ctx, rclient, cr.PodLabels(), cr.Namespace); err != nil { @@ -119,40 +67,6 @@ func reconcileConfigsData(ctx context.Context, rclient client.Client, cr *vmv1be return newConfigMapNames, nil } -// rulesCMDiff - calculates diff between existing at k8s (current) configmaps with rules -// and generated by operator (new) configmaps. -// Configmaps are grouped by operations, that must be performed over them. -func rulesCMDiff(currentCMs []corev1.ConfigMap, newCMs []corev1.ConfigMap) (toCreate []corev1.ConfigMap, toUpdate []corev1.ConfigMap) { - if len(newCMs) == 0 { - return - } - if len(currentCMs) == 0 { - return newCMs, nil - } - // calculate maps for update - for _, newCM := range newCMs { - var found bool - for _, currentCM := range currentCMs { - if newCM.Name == currentCM.Name { - found = true - newCM.Annotations = labels.Merge(currentCM.Annotations, newCM.Annotations) - vmv1beta1.AddFinalizer(&newCM, ¤tCM) - if equality.Semantic.DeepEqual(newCM.Data, currentCM.Data) && - equality.Semantic.DeepEqual(newCM.Labels, currentCM.Labels) && - equality.Semantic.DeepEqual(newCM.Annotations, currentCM.Annotations) { - break - } - toUpdate = append(toUpdate, newCM) - break - } - } - if !found { - toCreate = append(toCreate, newCM) - } - } - return toCreate, toUpdate -} - func reconcileVMAlertConfig(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAlert, childCR *vmv1beta1.VMRule) ([]string, error) { pos, data, err := selectRules(ctx, rclient, cr) if err != nil { diff --git a/internal/controller/operator/factory/vmalert/rules_test.go b/internal/controller/operator/factory/vmalert/rules_test.go index fc018577d..ede537eee 100644 --- a/internal/controller/operator/factory/vmalert/rules_test.go +++ b/internal/controller/operator/factory/vmalert/rules_test.go @@ -381,141 +381,3 @@ func Test_deduplicateRules(t *testing.T) { }, }) } - -func Test_rulesCMDiff(t *testing.T) { - type opts struct { - oldCMs []corev1.ConfigMap - newCMs []corev1.ConfigMap - toCreate []corev1.ConfigMap - toUpdate []corev1.ConfigMap - } - - f := func(o opts) { - t.Helper() - got, got1 := rulesCMDiff(o.oldCMs, o.newCMs) - assert.Equal(t, o.toCreate, got) - assert.Equal(t, o.toUpdate, got1) - } - - // create one new - f(opts{ - oldCMs: []corev1.ConfigMap{}, - newCMs: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-1", - }, - }, - }, - toCreate: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-1", - }, - }, - }, - }) - - // skip exist - f(opts{ - oldCMs: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-1", - }, - }, - }, - newCMs: []corev1.ConfigMap{}, - }) - - // update one - f(opts{ - oldCMs: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-1", - }, - }, - }, - newCMs: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-1", - }, - Data: map[string]string{"rule": "content"}, - }, - }, - toUpdate: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-1", - Annotations: map[string]string{}, - Finalizers: []string{vmv1beta1.FinalizerName}, - }, - Data: map[string]string{"rule": "content"}, - }, - }, - }) - - // update two - f(opts{ - oldCMs: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-0", - Annotations: map[string]string{}, - Finalizers: []string{vmv1beta1.FinalizerName}, - }, - Data: map[string]string{"rule": "outdated-content"}, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-1", - Annotations: map[string]string{}, - Finalizers: []string{vmv1beta1.FinalizerName}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-3", - }, - }, - }, - newCMs: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-0", - Annotations: map[string]string{}, - Finalizers: []string{vmv1beta1.FinalizerName}, - }, - Data: map[string]string{"rule": "new-content"}, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-1", - Annotations: map[string]string{}, - Finalizers: []string{vmv1beta1.FinalizerName}, - }, - Data: map[string]string{"rule": "new-content"}, - }, - }, - toUpdate: []corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-0", - Annotations: map[string]string{}, - Finalizers: []string{vmv1beta1.FinalizerName}, - }, - Data: map[string]string{"rule": "new-content"}, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "rules-cm-1", - Annotations: map[string]string{}, - Finalizers: []string{vmv1beta1.FinalizerName}, - }, - Data: map[string]string{"rule": "new-content"}, - }, - }, - }) -} diff --git a/internal/controller/operator/factory/vmalert/vmalert.go b/internal/controller/operator/factory/vmalert/vmalert.go index a944fdac7..5d427dbc3 100644 --- a/internal/controller/operator/factory/vmalert/vmalert.go +++ b/internal/controller/operator/factory/vmalert/vmalert.go @@ -92,11 +92,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAlert, rclient client.C } } if cr.IsOwnsServiceAccount() { - var prevSA *corev1.ServiceAccount - if prevCR != nil { - prevSA = build.ServiceAccount(prevCR) - } - if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr)); err != nil { return fmt.Errorf("failed create service account: %w", err) } } @@ -118,11 +114,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAlert, rclient client.C } if cr.Spec.PodDisruptionBudget != nil { - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.PodDisruptionBudget != nil { - prevPDB = build.PodDisruptionBudget(prevCR, prevCR.Spec.PodDisruptionBudget) - } - if err := reconcile.PDB(ctx, rclient, build.PodDisruptionBudget(cr, cr.Spec.PodDisruptionBudget), prevPDB); err != nil { + if err := reconcile.PDB(ctx, rclient, build.PodDisruptionBudget(cr, cr.Spec.PodDisruptionBudget)); err != nil { return fmt.Errorf("cannot update pod disruption budget for vmalert: %w", err) } } @@ -140,7 +132,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAlert, rclient client.C return fmt.Errorf("cannot generate new deploy for vmalert: %w", err) } - err = createOrUpdateAssets(ctx, rclient, cr, prevCR, ac) + err = createOrUpdateAssets(ctx, rclient, cr, ac) if err != nil { return err } @@ -520,14 +512,10 @@ func buildArgs(cr *vmv1beta1.VMAlert, ruleConfigMapNames []string, ac *build.Ass return args, nil } -func createOrUpdateAssets(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAlert, ac *build.AssetsCache) error { +func createOrUpdateAssets(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAlert, ac *build.AssetsCache) error { for kind, secret := range ac.GetOutput() { secret.ObjectMeta = build.ResourceMeta(kind, cr) - var prevSecretMeta *metav1.ObjectMeta - if prevCR != nil { - prevSecretMeta = ptr.To(build.ResourceMeta(kind, prevCR)) - } - err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta) + err := reconcile.Secret(ctx, rclient, &secret) if err != nil { return err } diff --git a/internal/controller/operator/factory/vmalertmanager/alertmanager.go b/internal/controller/operator/factory/vmalertmanager/alertmanager.go index b212b7e61..ec8c139b3 100644 --- a/internal/controller/operator/factory/vmalertmanager/alertmanager.go +++ b/internal/controller/operator/factory/vmalertmanager/alertmanager.go @@ -30,11 +30,7 @@ func CreateOrUpdateAlertManager(ctx context.Context, cr *vmv1beta1.VMAlertmanage } } if cr.IsOwnsServiceAccount() { - var prevSA *corev1.ServiceAccount - if prevCR != nil { - prevSA = build.ServiceAccount(prevCR) - } - if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr)); err != nil { return fmt.Errorf("failed create service account: %w", err) } if err := createConfigSecretAccess(ctx, rclient, cr, prevCR); err != nil { @@ -53,11 +49,7 @@ func CreateOrUpdateAlertManager(ctx context.Context, cr *vmv1beta1.VMAlertmanage } if cr.Spec.PodDisruptionBudget != nil { - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.PodDisruptionBudget != nil { - prevPDB = build.PodDisruptionBudget(prevCR, prevCR.Spec.PodDisruptionBudget) - } - if err := reconcile.PDB(ctx, rclient, build.PodDisruptionBudget(cr, cr.Spec.PodDisruptionBudget), prevPDB); err != nil { + if err := reconcile.PDB(ctx, rclient, build.PodDisruptionBudget(cr, cr.Spec.PodDisruptionBudget)); err != nil { return err } } diff --git a/internal/controller/operator/factory/vmalertmanager/statefulset.go b/internal/controller/operator/factory/vmalertmanager/statefulset.go index 8b7ff38a8..26075d546 100644 --- a/internal/controller/operator/factory/vmalertmanager/statefulset.go +++ b/internal/controller/operator/factory/vmalertmanager/statefulset.go @@ -458,12 +458,6 @@ func getAssetsCache(ctx context.Context, rclient client.Client, cr *vmv1beta1.VM // if not create with predefined or user value. func CreateOrUpdateConfig(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAlertmanager, childCR *vmv1beta1.VMAlertmanagerConfig) error { l := logger.WithContext(ctx) - var prevCR *vmv1beta1.VMAlertmanager - if cr.ParsedLastAppliedSpec != nil { - prevCR = cr.DeepCopy() - prevCR.Spec = *cr.ParsedLastAppliedSpec - } - ac := getAssetsCache(ctx, rclient, cr) var configSourceName string var alertmanagerConfig []byte @@ -549,12 +543,7 @@ func CreateOrUpdateConfig(ctx context.Context, rclient client.Client, cr *vmv1be newAMSecretConfig.Data[gossipConfigKey] = gossipCfg } - var prevSecretMeta *metav1.ObjectMeta - if prevCR != nil { - prevSecretMeta = buildConfigSecretMeta(prevCR) - } - - if err := reconcile.Secret(ctx, rclient, newAMSecretConfig, prevSecretMeta); err != nil { + if err := reconcile.Secret(ctx, rclient, newAMSecretConfig); err != nil { return err } diff --git a/internal/controller/operator/factory/vmanomaly/config.go b/internal/controller/operator/factory/vmanomaly/config.go index 6a23facab..8bb9314a6 100644 --- a/internal/controller/operator/factory/vmanomaly/config.go +++ b/internal/controller/operator/factory/vmanomaly/config.go @@ -6,8 +6,6 @@ import ( "encoding/hex" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" vmv1 "github.com/VictoriaMetrics/operator/api/operator/v1" @@ -17,7 +15,7 @@ import ( ) // createOrUpdateConfig reconcile configuration for vmanomaly and returns configuration consistent hash -func createOrUpdateConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VMAnomaly, ac *build.AssetsCache) (string, error) { +func createOrUpdateConfig(ctx context.Context, rclient client.Client, cr *vmv1.VMAnomaly, ac *build.AssetsCache) (string, error) { data, err := config.Load(cr, ac) if err != nil { return "", err @@ -30,22 +28,13 @@ func createOrUpdateConfig(ctx context.Context, rclient client.Client, cr, prevCR } for kind, secret := range ac.GetOutput() { - var prevSecretMeta *metav1.ObjectMeta - if prevCR != nil { - prevSecretMeta = ptr.To(build.ResourceMeta(kind, prevCR)) - } secret.ObjectMeta = build.ResourceMeta(kind, cr) - if err := reconcile.Secret(ctx, rclient, &secret, prevSecretMeta); err != nil { + if err := reconcile.Secret(ctx, rclient, &secret); err != nil { return "", err } } - var prevSecretMeta *metav1.ObjectMeta - if prevCR != nil { - prevSecretMeta = ptr.To(build.ResourceMeta(build.SecretConfigResourceKind, prevCR)) - } - - if err := reconcile.Secret(ctx, rclient, newSecretConfig, prevSecretMeta); err != nil { + if err := reconcile.Secret(ctx, rclient, newSecretConfig); err != nil { return "", err } diff --git a/internal/controller/operator/factory/vmanomaly/statefulset.go b/internal/controller/operator/factory/vmanomaly/statefulset.go index a7588e855..1a170f7bf 100644 --- a/internal/controller/operator/factory/vmanomaly/statefulset.go +++ b/internal/controller/operator/factory/vmanomaly/statefulset.go @@ -10,7 +10,6 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/utils/ptr" @@ -35,11 +34,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1.VMAnomaly, rclient client.Clie } } if cr.IsOwnsServiceAccount() { - var prevSA *corev1.ServiceAccount - if prevCR != nil { - prevSA = build.ServiceAccount(prevCR) - } - if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr)); err != nil { return fmt.Errorf("failed create service account: %w", err) } } @@ -57,7 +52,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1.VMAnomaly, rclient client.Clie }, } ac := build.NewAssetsCache(ctx, rclient, rcfg) - configHash, err := createOrUpdateConfig(ctx, rclient, cr, prevCR, ac) + configHash, err := createOrUpdateConfig(ctx, rclient, cr, ac) if err != nil { return err } @@ -204,11 +199,7 @@ func createOrUpdateApp(ctx context.Context, rclient client.Client, cr, prevCR *v }() if cr.Spec.PodDisruptionBudget != nil { pdb := build.ShardPodDisruptionBudget(cr, cr.Spec.PodDisruptionBudget, num) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.PodDisruptionBudget != nil { - prevPDB = build.ShardPodDisruptionBudget(prevCR, prevCR.Spec.PodDisruptionBudget, num) - } - if err := reconcile.PDB(ctx, rclient, pdb, prevPDB); err != nil { + if err := reconcile.PDB(ctx, rclient, pdb); err != nil { rv.err = err return } diff --git a/internal/controller/operator/factory/vmanomaly/statefulset_test.go b/internal/controller/operator/factory/vmanomaly/statefulset_test.go index dbafc14bb..7eec049b9 100644 --- a/internal/controller/operator/factory/vmanomaly/statefulset_test.go +++ b/internal/controller/operator/factory/vmanomaly/statefulset_test.go @@ -297,7 +297,7 @@ func Test_createDefaultConfig(t *testing.T) { } ctx := context.TODO() ac := build.NewAssetsCache(ctx, fclient, cfg) - if _, err := createOrUpdateConfig(ctx, fclient, o.cr, nil, ac); (err != nil) != o.wantErr { + if _, err := createOrUpdateConfig(ctx, fclient, o.cr, ac); (err != nil) != o.wantErr { t.Fatalf("createOrUpdateConfig() error = %v, wantErr %v", err, o.wantErr) } if o.wantErr { diff --git a/internal/controller/operator/factory/vmauth/vmauth.go b/internal/controller/operator/factory/vmauth/vmauth.go index 74e4c5385..8a39e0c02 100644 --- a/internal/controller/operator/factory/vmauth/vmauth.go +++ b/internal/controller/operator/factory/vmauth/vmauth.go @@ -11,10 +11,8 @@ import ( corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" policyv1 "k8s.io/api/policy/v1" - 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/intstr" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -48,11 +46,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAuth, rclient client.Cl } cfg := config.MustGetBaseConfig() if cr.IsOwnsServiceAccount() { - var prevSA *corev1.ServiceAccount - if prevCR != nil { - prevSA = build.ServiceAccount(prevCR) - } - if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr)); err != nil { return fmt.Errorf("failed create service account: %w", err) } if err := createVMAuthSecretAccess(ctx, rclient, cr, prevCR); err != nil { @@ -70,11 +64,11 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAuth, rclient client.Cl if cr.Spec.HTTPRoute != nil && !cfg.GatewayAPIEnabled { return fmt.Errorf("spec.httpRoute is set but VM_GATEWAY_API_ENABLED=true env var was not provided") } - if err := createOrUpdateHTTPRoute(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateHTTPRoute(ctx, rclient, cr); err != nil { return fmt.Errorf("cannot create or update httpRoute for vmauth: %w", err) } - if err := createOrUpdateHPA(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateHPA(ctx, rclient, cr); err != nil { return fmt.Errorf("cannot create or update hpa for vmauth: %w", err) } @@ -90,11 +84,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAuth, rclient client.Cl } if cr.Spec.PodDisruptionBudget != nil { - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.PodDisruptionBudget != nil { - prevPDB = build.PodDisruptionBudget(prevCR, prevCR.Spec.PodDisruptionBudget) - } - if err := reconcile.PDB(ctx, rclient, build.PodDisruptionBudget(cr, cr.Spec.PodDisruptionBudget), prevPDB); err != nil { + if err := reconcile.PDB(ctx, rclient, build.PodDisruptionBudget(cr, cr.Spec.PodDisruptionBudget)); err != nil { return fmt.Errorf("cannot update pod disruption budget for vmauth: %w", err) } } @@ -121,7 +111,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAuth, rclient client.Cl return nil } -func createOrUpdateHTTPRoute(ctx context.Context, rclient client.Client, cr, prevCr *vmv1beta1.VMAuth) error { +func createOrUpdateHTTPRoute(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAuth) error { if cr.Spec.HTTPRoute == nil { return nil } @@ -130,16 +120,7 @@ func createOrUpdateHTTPRoute(ctx context.Context, rclient client.Client, cr, pre if err != nil { return err } - - var prevHTTPRoute *gwapiv1.HTTPRoute - if prevCr != nil && prevCr.Spec.HTTPRoute != nil { - prevHTTPRoute, err = build.HTTPRoute(cr, cr.Spec.Port, prevCr.Spec.HTTPRoute) - if err != nil { - return err - } - } - - return reconcile.HTTPRoute(ctx, rclient, newHTTPRoute, prevHTTPRoute) + return reconcile.HTTPRoute(ctx, rclient, newHTTPRoute) } func newDeployForVMAuth(cr *vmv1beta1.VMAuth) (*appsv1.Deployment, error) { @@ -386,11 +367,6 @@ func CreateOrUpdateConfig(ctx context.Context, rclient client.Client, cr *vmv1be if cr.Spec.SecretRef != nil || cr.Spec.LocalPath != "" { return nil } - var prevCR *vmv1beta1.VMAuth - if cr.ParsedLastAppliedSpec != nil { - prevCR = cr.DeepCopy() - prevCR.Spec = *cr.ParsedLastAppliedSpec - } s := &corev1.Secret{ ObjectMeta: buildConfigSecretMeta(cr), Data: map[string][]byte{ @@ -419,11 +395,7 @@ func CreateOrUpdateConfig(ctx context.Context, rclient client.Client, cr *vmv1be return fmt.Errorf("cannot gzip config for vmauth: %w", err) } s.Data[vmAuthConfigNameGz] = data - var prevSecretMeta *metav1.ObjectMeta - if prevCR != nil { - prevSecretMeta = ptr.To(buildConfigSecretMeta(prevCR)) - } - if err := reconcile.Secret(ctx, rclient, s, prevSecretMeta); err != nil { + if err := reconcile.Secret(ctx, rclient, s); err != nil { return err } pos.users.UpdateMetrics(ctx) @@ -460,21 +432,9 @@ func createOrUpdateIngress(ctx context.Context, rclient client.Client, cr *vmv1b if cr.Spec.Ingress == nil { return nil } + newIngress := buildIngressConfig(cr) - var existIngress networkingv1.Ingress - if err := rclient.Get(ctx, types.NamespacedName{Namespace: newIngress.Namespace, Name: newIngress.Name}, &existIngress); err != nil { - if k8serrors.IsNotFound(err) { - return rclient.Create(ctx, newIngress) - } - return err - } - if err := finalize.FreeIfNeeded(ctx, rclient, &existIngress); err != nil { - return err - } - // TODO compare - newIngress.Annotations = labels.Merge(existIngress.Annotations, newIngress.Annotations) - vmv1beta1.AddFinalizer(newIngress, &existIngress) - return rclient.Update(ctx, newIngress) + return reconcile.Ingress(ctx, rclient, newIngress) } func buildIngressConfig(cr *vmv1beta1.VMAuth) *networkingv1.Ingress { @@ -585,7 +545,7 @@ func createOrUpdateService(ctx context.Context, rclient client.Client, cr, prevC } return newService, nil } -func createOrUpdateHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMAuth) error { +func createOrUpdateHPA(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAuth) error { if cr.Spec.HPA == nil { return nil } @@ -595,11 +555,7 @@ func createOrUpdateHPA(ctx context.Context, rclient client.Client, cr, prevCR *v APIVersion: "apps/v1", } newHPA := build.HPA(cr, targetRef, cr.Spec.HPA) - var prevHPA *autoscalingv2.HorizontalPodAutoscaler - if prevCR != nil && prevCR.Spec.HPA != nil { - prevHPA = build.HPA(prevCR, targetRef, prevCR.Spec.HPA) - } - return reconcile.HPA(ctx, rclient, newHPA, prevHPA) + return reconcile.HPA(ctx, rclient, newHPA) } func deleteOrphaned(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAuth) error { diff --git a/internal/controller/operator/factory/vmcluster/vmcluster.go b/internal/controller/operator/factory/vmcluster/vmcluster.go index 2423e856d..102873649 100644 --- a/internal/controller/operator/factory/vmcluster/vmcluster.go +++ b/internal/controller/operator/factory/vmcluster/vmcluster.go @@ -10,7 +10,6 @@ import ( appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" @@ -46,12 +45,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMCluster, rclient client if cr.IsOwnsServiceAccount() { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentRoot) sa := build.ServiceAccount(b) - var prevSA *corev1.ServiceAccount - if prevCR != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentRoot) - prevSA = build.ServiceAccount(b) - } - if err := reconcile.ServiceAccount(ctx, rclient, sa, prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, sa); err != nil { return fmt.Errorf("failed create service account: %w", err) } } @@ -65,7 +59,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMCluster, rclient client if cr.Spec.VMStorage != nil { if cr.Spec.VMStorage.PodDisruptionBudget != nil { - err := createOrUpdatePodDisruptionBudgetForVMStorage(ctx, rclient, cr, prevCR) + err := createOrUpdatePodDisruptionBudgetForVMStorage(ctx, rclient, cr) if err != nil { return err } @@ -78,7 +72,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMCluster, rclient client if err != nil { return err } - if err := createOrUpdateVMStorageHPA(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateVMStorageHPA(ctx, rclient, cr); err != nil { return err } if !ptr.Deref(cr.Spec.VMStorage.DisableSelfServiceScrape, false) { @@ -90,7 +84,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMCluster, rclient client if cr.Spec.VMSelect != nil { if cr.Spec.VMSelect.PodDisruptionBudget != nil { - if err := createOrUpdatePodDisruptionBudgetForVMSelect(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdatePodDisruptionBudgetForVMSelect(ctx, rclient, cr); err != nil { return err } } @@ -98,7 +92,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMCluster, rclient client return err } - if err := createOrUpdateVMSelectHPA(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateVMSelectHPA(ctx, rclient, cr); err != nil { return err } // create vmselect service @@ -121,7 +115,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMCluster, rclient client if cr.Spec.VMInsert != nil { if cr.Spec.VMInsert.PodDisruptionBudget != nil { - if err := createOrUpdatePodDisruptionBudgetForVMInsert(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdatePodDisruptionBudgetForVMInsert(ctx, rclient, cr); err != nil { return err } } @@ -132,7 +126,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMCluster, rclient client if err != nil { return err } - if err := createOrUpdateVMInsertHPA(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateVMInsertHPA(ctx, rclient, cr); err != nil { return err } if !ptr.Deref(cr.Spec.VMInsert.DisableSelfServiceScrape, false) { @@ -650,15 +644,10 @@ func makePodSpecForVMSelect(cr *vmv1beta1.VMCluster) (*corev1.PodTemplateSpec, e return vmSelectPodSpec, nil } -func createOrUpdatePodDisruptionBudgetForVMSelect(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMCluster) error { +func createOrUpdatePodDisruptionBudgetForVMSelect(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMCluster) error { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentSelect) pdb := build.PodDisruptionBudget(b, cr.Spec.VMSelect.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.VMSelect.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentSelect) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.VMSelect.PodDisruptionBudget) - } - return reconcile.PDB(ctx, rclient, pdb, prevPDB) + return reconcile.PDB(ctx, rclient, pdb) } func genVMInsertSpec(cr *vmv1beta1.VMCluster) (*appsv1.Deployment, error) { @@ -853,15 +842,10 @@ func makePodSpecForVMInsert(cr *vmv1beta1.VMCluster) (*corev1.PodTemplateSpec, e return vmInsertPodSpec, nil } -func createOrUpdatePodDisruptionBudgetForVMInsert(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMCluster) error { +func createOrUpdatePodDisruptionBudgetForVMInsert(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMCluster) error { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentInsert) pdb := build.PodDisruptionBudget(b, cr.Spec.VMInsert.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.VMInsert.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentInsert) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.VMInsert.PodDisruptionBudget) - } - return reconcile.PDB(ctx, rclient, pdb, prevPDB) + return reconcile.PDB(ctx, rclient, pdb) } func buildVMStorageSpec(ctx context.Context, cr *vmv1beta1.VMCluster) (*appsv1.StatefulSet, error) { @@ -1102,18 +1086,13 @@ func makePodSpecForVMStorage(ctx context.Context, cr *vmv1beta1.VMCluster) (*cor return vmStoragePodSpec, nil } -func createOrUpdatePodDisruptionBudgetForVMStorage(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMCluster) error { +func createOrUpdatePodDisruptionBudgetForVMStorage(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMCluster) error { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentStorage) pdb := build.PodDisruptionBudget(b, cr.Spec.VMStorage.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.VMStorage.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentStorage) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.VMStorage.PodDisruptionBudget) - } - return reconcile.PDB(ctx, rclient, pdb, prevPDB) + return reconcile.PDB(ctx, rclient, pdb) } -func createOrUpdateVMInsertHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMCluster) error { +func createOrUpdateVMInsertHPA(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMCluster) error { if cr.Spec.VMInsert.HPA == nil { return nil } @@ -1124,15 +1103,10 @@ func createOrUpdateVMInsertHPA(ctx context.Context, rclient client.Client, cr, p APIVersion: "apps/v1", } newHPA := build.HPA(b, targetRef, cr.Spec.VMInsert.HPA) - var prevHPA *autoscalingv2.HorizontalPodAutoscaler - if prevCR != nil && prevCR.Spec.VMInsert.HPA != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentInsert) - prevHPA = build.HPA(b, targetRef, prevCR.Spec.VMInsert.HPA) - } - return reconcile.HPA(ctx, rclient, newHPA, prevHPA) + return reconcile.HPA(ctx, rclient, newHPA) } -func createOrUpdateVMSelectHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMCluster) error { +func createOrUpdateVMSelectHPA(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMCluster) error { if cr.Spec.VMSelect.HPA == nil { return nil } @@ -1143,16 +1117,10 @@ func createOrUpdateVMSelectHPA(ctx context.Context, rclient client.Client, cr, p APIVersion: "apps/v1", } defaultHPA := build.HPA(b, targetRef, cr.Spec.VMSelect.HPA) - var prevHPA *autoscalingv2.HorizontalPodAutoscaler - if prevCR != nil && prevCR.Spec.VMSelect.HPA != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentSelect) - prevHPA = build.HPA(b, targetRef, prevCR.Spec.VMSelect.HPA) - } - - return reconcile.HPA(ctx, rclient, defaultHPA, prevHPA) + return reconcile.HPA(ctx, rclient, defaultHPA) } -func createOrUpdateVMStorageHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMCluster) error { +func createOrUpdateVMStorageHPA(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMCluster) error { hpa := cr.Spec.VMStorage.HPA if hpa == nil { return nil @@ -1164,13 +1132,7 @@ func createOrUpdateVMStorageHPA(ctx context.Context, rclient client.Client, cr, APIVersion: "apps/v1", } defaultHPA := build.HPA(b, targetRef, hpa) - var prevHPA *autoscalingv2.HorizontalPodAutoscaler - if prevCR != nil && prevCR.Spec.VMStorage.HPA != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentStorage) - prevHPA = build.HPA(b, targetRef, prevCR.Spec.VMStorage.HPA) - } - - return reconcile.HPA(ctx, rclient, defaultHPA, prevHPA) + return reconcile.HPA(ctx, rclient, defaultHPA) } func deleteOrphaned(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMCluster) error { @@ -1479,12 +1441,7 @@ func createOrUpdateVMAuthLBService(ctx context.Context, rclient client.Client, c } func createOrUpdateVMAuthLB(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMCluster) error { - - var prevSecretMeta *metav1.ObjectMeta - if prevCR != nil { - prevSecretMeta = ptr.To(buildLBConfigSecretMeta(prevCR)) - } - if err := reconcile.Secret(ctx, rclient, buildVMAuthLBSecret(cr), prevSecretMeta); err != nil { + if err := reconcile.Secret(ctx, rclient, buildVMAuthLBSecret(cr)); err != nil { return fmt.Errorf("cannot reconcile vmauth lb secret: %w", err) } lbDep, err := buildVMAuthLBDeployment(cr) @@ -1505,20 +1462,15 @@ func createOrUpdateVMAuthLB(ctx context.Context, rclient client.Client, cr, prev return err } if cr.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget != nil { - if err := createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx, rclient, cr); err != nil { return fmt.Errorf("cannot create or update PodDisruptionBudget for vmauth lb: %w", err) } } return nil } -func createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMCluster) error { +func createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMCluster) error { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentBalancer) pdb := build.PodDisruptionBudget(b, cr.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentBalancer) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget) - } - return reconcile.PDB(ctx, rclient, pdb, prevPDB) + return reconcile.PDB(ctx, rclient, pdb) } diff --git a/internal/controller/operator/factory/vmsingle/vmsingle.go b/internal/controller/operator/factory/vmsingle/vmsingle.go index ded465e7c..adc7567dd 100644 --- a/internal/controller/operator/factory/vmsingle/vmsingle.go +++ b/internal/controller/operator/factory/vmsingle/vmsingle.go @@ -28,16 +28,6 @@ const ( streamAggrSecretKey = "config.yaml" ) -func createStorage(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMSingle) error { - newPvc := makePvc(cr) - var prevPVC *corev1.PersistentVolumeClaim - if prevCR != nil && prevCR.Spec.Storage != nil { - prevPVC = makePvc(prevCR) - } - - return reconcile.PersistentVolumeClaim(ctx, rclient, newPvc, prevPVC) -} - func makePvc(cr *vmv1beta1.VMSingle) *corev1.PersistentVolumeClaim { pvcObject := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -70,21 +60,17 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMSingle, rclient client. } } ac := getAssetsCache(ctx, rclient) - if err := createOrUpdateStreamAggrConfig(ctx, rclient, cr, prevCR, ac); err != nil { + if err := createOrUpdateStreamAggrConfig(ctx, rclient, cr, ac); err != nil { return fmt.Errorf("cannot update stream aggregation config for vmsingle: %w", err) } if cr.IsOwnsServiceAccount() { - var prevSA *corev1.ServiceAccount - if prevCR != nil { - prevSA = build.ServiceAccount(prevCR) - } - if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr)); err != nil { return fmt.Errorf("failed create service account: %w", err) } } if cr.Spec.Storage != nil { - if err := createStorage(ctx, rclient, cr, prevCR); err != nil { + if err := reconcile.PersistentVolumeClaim(ctx, rclient, makePvc(cr)); err != nil { return fmt.Errorf("cannot create storage: %w", err) } } @@ -403,7 +389,7 @@ func buildStreamAggrConfig(cr *vmv1beta1.VMSingle, ac *build.AssetsCache) (*core } // createOrUpdateStreamAggrConfig builds stream aggregation configs for vmsingle at separate configmap, serialized as yaml -func createOrUpdateStreamAggrConfig(ctx context.Context, rclient client.Client, cr, prevCR *vmv1beta1.VMSingle, ac *build.AssetsCache) error { +func createOrUpdateStreamAggrConfig(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMSingle, ac *build.AssetsCache) error { if !cr.HasAnyStreamAggrRule() { return nil } @@ -411,11 +397,8 @@ func createOrUpdateStreamAggrConfig(ctx context.Context, rclient client.Client, if err != nil { return err } - var prevCMMeta *metav1.ObjectMeta - if prevCR != nil { - prevCMMeta = ptr.To(build.ResourceMeta(build.StreamAggrConfigResourceKind, prevCR)) - } - return reconcile.ConfigMap(ctx, rclient, streamAggrCM, prevCMMeta) + _, err = reconcile.ConfigMap(ctx, rclient, streamAggrCM) + return err } func deleteOrphaned(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMSingle) error { diff --git a/internal/controller/operator/factory/vtcluster/cluster.go b/internal/controller/operator/factory/vtcluster/cluster.go index b66319c0c..4811a4ff8 100644 --- a/internal/controller/operator/factory/vtcluster/cluster.go +++ b/internal/controller/operator/factory/vtcluster/cluster.go @@ -31,12 +31,7 @@ func CreateOrUpdate(ctx context.Context, rclient client.Client, cr *vmv1.VTClust if cr.IsOwnsServiceAccount() { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentRoot) sa := build.ServiceAccount(b) - var prevSA *corev1.ServiceAccount - if prevCR != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentRoot) - prevSA = build.ServiceAccount(b) - } - if err := reconcile.ServiceAccount(ctx, rclient, sa, prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, sa); err != nil { return fmt.Errorf("failed create service account: %w", err) } } diff --git a/internal/controller/operator/factory/vtcluster/insert.go b/internal/controller/operator/factory/vtcluster/insert.go index 699bb540b..beefa9324 100644 --- a/internal/controller/operator/factory/vtcluster/insert.go +++ b/internal/controller/operator/factory/vtcluster/insert.go @@ -9,7 +9,6 @@ import ( appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" @@ -28,7 +27,7 @@ func createOrUpdateVTInsert(ctx context.Context, rclient client.Client, cr, prev return nil } - if err := createOrUpdatePodDisruptionBudgetForVTInsert(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdatePodDisruptionBudgetForVTInsert(ctx, rclient, cr); err != nil { return err } if err := createOrUpdateVTInsertDeployment(ctx, rclient, cr, prevCR); err != nil { @@ -38,7 +37,7 @@ func createOrUpdateVTInsert(ctx context.Context, rclient client.Client, cr, prev if err != nil { return err } - if err := createOrUpdateVTInsertHPA(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateVTInsertHPA(ctx, rclient, cr); err != nil { return err } if !ptr.Deref(cr.Spec.Insert.DisableSelfServiceScrape, false) { @@ -56,18 +55,13 @@ func createOrUpdateVTInsert(ctx context.Context, rclient client.Client, cr, prev return nil } -func createOrUpdatePodDisruptionBudgetForVTInsert(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTCluster) error { +func createOrUpdatePodDisruptionBudgetForVTInsert(ctx context.Context, rclient client.Client, cr *vmv1.VTCluster) error { if cr.Spec.Insert.PodDisruptionBudget == nil { return nil } b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentInsert) pdb := build.PodDisruptionBudget(b, cr.Spec.Insert.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.Insert.PodDisruptionBudget != nil { - b := build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentInsert) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.Insert.PodDisruptionBudget) - } - return reconcile.PDB(ctx, rclient, pdb, prevPDB) + return reconcile.PDB(ctx, rclient, pdb) } func createOrUpdateVTInsertDeployment(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTCluster) error { @@ -257,7 +251,7 @@ func buildVTInsertPodSpec(cr *vmv1.VTCluster) (*corev1.PodTemplateSpec, error) { return podSpec, nil } -func createOrUpdateVTInsertHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTCluster) error { +func createOrUpdateVTInsertHPA(ctx context.Context, rclient client.Client, cr *vmv1.VTCluster) error { if cr.Spec.Insert.HPA == nil { return nil } @@ -268,12 +262,7 @@ func createOrUpdateVTInsertHPA(ctx context.Context, rclient client.Client, cr, p } b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentInsert) newHPA := build.HPA(b, targetRef, cr.Spec.Insert.HPA) - var prevHPA *autoscalingv2.HorizontalPodAutoscaler - if prevCR != nil && prevCR.Spec.Insert.HPA != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentInsert) - prevHPA = build.HPA(b, targetRef, prevCR.Spec.Insert.HPA) - } - return reconcile.HPA(ctx, rclient, newHPA, prevHPA) + return reconcile.HPA(ctx, rclient, newHPA) } func createOrUpdateVTInsertService(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTCluster) (*corev1.Service, error) { diff --git a/internal/controller/operator/factory/vtcluster/select.go b/internal/controller/operator/factory/vtcluster/select.go index c263e07fd..8ac8e7577 100644 --- a/internal/controller/operator/factory/vtcluster/select.go +++ b/internal/controller/operator/factory/vtcluster/select.go @@ -9,7 +9,6 @@ import ( appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" @@ -30,17 +29,12 @@ func createOrUpdateVTSelect(ctx context.Context, rclient client.Client, cr, prev if cr.Spec.Select.PodDisruptionBudget != nil { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentSelect) pdb := build.PodDisruptionBudget(b, cr.Spec.Select.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.Select.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentSelect) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.Select.PodDisruptionBudget) - } - err := reconcile.PDB(ctx, rclient, pdb, prevPDB) + err := reconcile.PDB(ctx, rclient, pdb) if err != nil { return err } } - if err := createOrUpdateVTSelectHPA(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateVTSelectHPA(ctx, rclient, cr); err != nil { return err } selectSvc, err := createOrUpdateVTSelectService(ctx, rclient, cr, prevCR) @@ -64,7 +58,7 @@ func createOrUpdateVTSelect(ctx context.Context, rclient client.Client, cr, prev return nil } -func createOrUpdateVTSelectHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTCluster) error { +func createOrUpdateVTSelectHPA(ctx context.Context, rclient client.Client, cr *vmv1.VTCluster) error { if cr.Spec.Select.HPA == nil { return nil } @@ -75,12 +69,7 @@ func createOrUpdateVTSelectHPA(ctx context.Context, rclient client.Client, cr, p } b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentSelect) defaultHPA := build.HPA(b, targetRef, cr.Spec.Select.HPA) - var prevHPA *autoscalingv2.HorizontalPodAutoscaler - if prevCR != nil && prevCR.Spec.Select.HPA != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentSelect) - prevHPA = build.HPA(b, targetRef, prevCR.Spec.Select.HPA) - } - return reconcile.HPA(ctx, rclient, defaultHPA, prevHPA) + return reconcile.HPA(ctx, rclient, defaultHPA) } func createOrUpdateVTSelectService(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTCluster) (*corev1.Service, error) { diff --git a/internal/controller/operator/factory/vtcluster/storage.go b/internal/controller/operator/factory/vtcluster/storage.go index b41665365..970be75a3 100644 --- a/internal/controller/operator/factory/vtcluster/storage.go +++ b/internal/controller/operator/factory/vtcluster/storage.go @@ -9,7 +9,6 @@ import ( appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" @@ -31,17 +30,11 @@ func createOrUpdateVTStorage(ctx context.Context, rclient client.Client, cr, pre if cr.Spec.Storage.PodDisruptionBudget != nil { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentStorage) pdb := build.PodDisruptionBudget(b, cr.Spec.Storage.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.Storage.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentStorage) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.Storage.PodDisruptionBudget) - } - err := reconcile.PDB(ctx, rclient, pdb, prevPDB) - if err != nil { + if err := reconcile.PDB(ctx, rclient, pdb); err != nil { return err } } - if err := createOrUpdateVTStorageHPA(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdateVTStorageHPA(ctx, rclient, cr); err != nil { return err } if err := createOrUpdateVTStorageSTS(ctx, rclient, cr, prevCR); err != nil { @@ -96,7 +89,7 @@ func createOrUpdateVTStorageService(ctx context.Context, rclient client.Client, return newHeadless, nil } -func createOrUpdateVTStorageHPA(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTCluster) error { +func createOrUpdateVTStorageHPA(ctx context.Context, rclient client.Client, cr *vmv1.VTCluster) error { hpa := cr.Spec.Storage.HPA if hpa == nil { return nil @@ -108,13 +101,7 @@ func createOrUpdateVTStorageHPA(ctx context.Context, rclient client.Client, cr, APIVersion: "apps/v1", } defaultHPA := build.HPA(b, targetRef, hpa) - var prevHPA *autoscalingv2.HorizontalPodAutoscaler - if prevCR != nil && prevCR.Spec.Storage.HPA != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentStorage) - prevHPA = build.HPA(b, targetRef, prevCR.Spec.Storage.HPA) - } - - return reconcile.HPA(ctx, rclient, defaultHPA, prevHPA) + return reconcile.HPA(ctx, rclient, defaultHPA) } func createOrUpdateVTStorageSTS(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTCluster) error { diff --git a/internal/controller/operator/factory/vtcluster/vmauth_lb.go b/internal/controller/operator/factory/vtcluster/vmauth_lb.go index 56446253f..855e441a5 100644 --- a/internal/controller/operator/factory/vtcluster/vmauth_lb.go +++ b/internal/controller/operator/factory/vtcluster/vmauth_lb.go @@ -7,7 +7,6 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" @@ -23,11 +22,7 @@ import ( ) func createOrUpdateVMAuthLB(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTCluster) error { - var prevSecretMeta *metav1.ObjectMeta - if prevCR != nil { - prevSecretMeta = ptr.To(buildLBConfigSecretMeta(prevCR)) - } - if err := reconcile.Secret(ctx, rclient, buildVMauthLBSecret(cr), prevSecretMeta); err != nil { + if err := reconcile.Secret(ctx, rclient, buildVMauthLBSecret(cr)); err != nil { return fmt.Errorf("cannot reconcile vmauth lb secret: %w", err) } lbDep, err := buildVMauthLBDeployment(cr) @@ -48,7 +43,7 @@ func createOrUpdateVMAuthLB(ctx context.Context, rclient client.Client, cr, prev return err } if cr.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget != nil { - if err := createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx, rclient, cr, prevCR); err != nil { + if err := createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx, rclient, cr); err != nil { return fmt.Errorf("cannot create or update PodDisruptionBudget for vmauth lb: %w", err) } } @@ -254,15 +249,10 @@ func createOrUpdateVMAuthLBService(ctx context.Context, rclient client.Client, c return nil } -func createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTCluster) error { +func createOrUpdatePodDisruptionBudgetForVMAuthLB(ctx context.Context, rclient client.Client, cr *vmv1.VTCluster) error { b := build.NewChildBuilder(cr, vmv1beta1.ClusterComponentBalancer) pdb := build.PodDisruptionBudget(b, cr.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget) - var prevPDB *policyv1.PodDisruptionBudget - if prevCR != nil && prevCR.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget != nil { - b = build.NewChildBuilder(prevCR, vmv1beta1.ClusterComponentBalancer) - prevPDB = build.PodDisruptionBudget(b, prevCR.Spec.RequestsLoadBalancer.Spec.PodDisruptionBudget) - } - return reconcile.PDB(ctx, rclient, pdb, prevPDB) + return reconcile.PDB(ctx, rclient, pdb) } // createOrUpdateLBProxyService builds vtinsert and vtselect external services to expose vtcluster components for access by vmauth diff --git a/internal/controller/operator/factory/vtsingle/vtsingle.go b/internal/controller/operator/factory/vtsingle/vtsingle.go index 119e8f9e8..94360fb73 100644 --- a/internal/controller/operator/factory/vtsingle/vtsingle.go +++ b/internal/controller/operator/factory/vtsingle/vtsingle.go @@ -28,15 +28,6 @@ const ( tlsServerConfigMountPath = "/etc/vm/tls-server-secrets" ) -func createOrUpdatePVC(ctx context.Context, rclient client.Client, cr, prevCR *vmv1.VTSingle) error { - newPvc := newPVC(cr) - var prevPVC *corev1.PersistentVolumeClaim - if prevCR != nil && prevCR.Spec.Storage != nil { - prevPVC = newPVC(prevCR) - } - return reconcile.PersistentVolumeClaim(ctx, rclient, newPvc, prevPVC) -} - func newPVC(r *vmv1.VTSingle) *corev1.PersistentVolumeClaim { pvcObject := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -69,17 +60,13 @@ func CreateOrUpdate(ctx context.Context, rclient client.Client, cr *vmv1.VTSingl } } if cr.Spec.Storage != nil { - if err := createOrUpdatePVC(ctx, rclient, cr, prevCR); err != nil { + if err := reconcile.PersistentVolumeClaim(ctx, rclient, newPVC(cr)); err != nil { return err } } if cr.IsOwnsServiceAccount() { - var prevSA *corev1.ServiceAccount - if prevCR != nil { - prevSA = build.ServiceAccount(prevCR) - } - if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr), prevSA); err != nil { + if err := reconcile.ServiceAccount(ctx, rclient, build.ServiceAccount(cr)); err != nil { return fmt.Errorf("failed create service account: %w", err) } }