feature request: configurable rollout strategy for vmcluster

[dctrwatson]

I recently upgraded from v0.60.2 to v0.63.0 and the operator now seems to do something between minimum and no time downtime strategy when performing vmcluster changes.

It will update the vmstorage and vmselect STS in parallel. And then once the vmselect STS rollout is complete, it will update the vminsert deployment. This makes for a very noisy rollout and cluster availability that gets worse the larger the cluster is.

I much prefer the old behavior of doing the equivalent to No Downtime Strategy. We have tuned many aspects of the VictoriaMetrics components and k8s to have little or no interruptions when rollouts are performed in that manner.

[f41gh7]

I recently upgraded from v0.60.2 to v0.63.0 and the operator now seems to do something between minimum and no time downtime strategy when performing vmcluster changes.

It shouldn't. Operator performs new parallel updates only if rollingUpdateStrategyBehavior is set. Otherwise, it performs old no downtime strategy with updating pods one by one.

Could you please provide an example of VMCluster yaml configuration?

[dctrwatson]

apiVersion: operator.victoriametrics.com/v1beta1
kind: VMCluster
metadata:
  name: canary
  namespace: monitoring
spec:
  replicationFactor: 2
  retentionPeriod: "3"
  vminsert:
    extraEnvs:
      - name: GOMAXPROCS
        value: "2"
    hpa:
      maxReplicas: 12
      metrics:
        - pods:
            metric:
              name: vm_concurrent_insert_utilization
            target:
              averageValue: 800m
              type: AverageValue
          type: Pods
      minReplicas: 6
    image:
      tag: v1.128.0-cluster
    minReadySeconds: 180
    podDisruptionBudget:
      maxUnavailable: 1
    port: "8480"
    priorityClassName: monitoring-canary
    replicaCount: 6
    resources:
      limits:
        memory: 2Gi
      requests:
        cpu: 500m
    rollingUpdate:
      maxSurge: 0
      maxUnavailable: 1
    serviceSpec:
      metadata:
        annotations:
          service.kubernetes.io/topology-mode: Auto
        name: vminsert-canary-az-aware
      spec: {}
    terminationGracePeriodSeconds: 300
    topologySpreadConstraints:
      - maxSkew: 1
        topologyKey: topology.kubernetes.io/zone
        whenUnsatisfiable: DoNotSchedule
      - maxSkew: 1
        topologyKey: kubernetes.io/hostname
        whenUnsatisfiable: DoNotSchedule
  vmselect:
    cacheMountPath: /select-cache
    clusterNativeListenPort: "8401"
    extraArgs:
      dedup.minScrapeInterval: 60s
    extraEnvs:
      - name: GOMAXPROCS
        value: "4"
    hpa:
      maxReplicas: 6
      metrics:
        - pods:
            metric:
              name: vm_concurrent_select_utilization
            target:
              averageValue: 800m
              type: AverageValue
          type: Pods
      minReplicas: 3
    image:
      tag: v1.128.0-cluster
    minReadySeconds: 30
    podDisruptionBudget:
      maxUnavailable: 1
    port: "8481"
    priorityClassName: monitoring-canary
    replicaCount: 2
    resources:
      limits:
        memory: 3Gi
      requests:
        cpu: 1500m
    rollingUpdateStrategy: RollingUpdate
    serviceSpec:
      metadata:
        annotations:
          service.kubernetes.io/topology-mode: Auto
        name: vmselect-canary-az-aware
      spec: {}
    storage:
      emptyDir: {}
      volumeClaimTemplate:
        spec:
          resources:
            requests:
              storage: 2Gi
    terminationGracePeriodSeconds: 60
    topologySpreadConstraints:
      - maxSkew: 1
        topologyKey: topology.kubernetes.io/zone
        whenUnsatisfiable: DoNotSchedule
      - maxSkew: 1
        topologyKey: kubernetes.io/hostname
        whenUnsatisfiable: DoNotSchedule
  vmstorage:
    extraArgs:
      dedup.minScrapeInterval: 60s
    image:
      tag: v1.128.0-cluster
    minReadySeconds: 180
    podDisruptionBudget:
      maxUnavailable: 1
    priorityClassName: canary-monitoring
    replicaCount: 6
    resources:
      requests:
        cpu: '7'
      limits:
        memory: 27Gi
    rollingUpdateStrategy: RollingUpdate
    storage:
      volumeClaimTemplate:
        spec:
          resources:
            requests:
              storage: 500Gi
          storageClassName: hdd
    storageDataPath: /vm-data
    terminationGracePeriodSeconds: 900
    topologySpreadConstraints:
      - maxSkew: 1
        topologyKey: topology.kubernetes.io/zone
        whenUnsatisfiable: DoNotSchedule
      - maxSkew: 1
        topologyKey: kubernetes.io/hostname
        whenUnsatisfiable: DoNotSchedule

[f41gh7]

Thanks for provided example. It seems to be the race condition at waitForStatefulSetReady. When operator performs StatefulSet ready checks, it fetches statefulSet status from the local cache, which potentially may not have new Generation from previously executed rclient.Update call. In this case, it uses fast path and treats StatefulSet as ready.

Possible fix should to be to check Generation from objects used for client.Update call:

diff --git a/internal/controller/operator/factory/reconcile/statefulset.go b/internal/controller/operator/factory/reconcile/statefulset.go
index a03864cb..87f13e28 100644
--- a/internal/controller/operator/factory/reconcile/statefulset.go
+++ b/internal/controller/operator/factory/reconcile/statefulset.go
@@ -48,12 +48,15 @@ func waitForStatefulSetReady(ctx context.Context, rclient client.Client, newSts
 		if err := rclient.Get(ctx, types.NamespacedName{Namespace: newSts.Namespace, Name: newSts.Name}, &stsForStatus); err != nil {
 			return false, err
 		}
-		if stsForStatus.Generation > stsForStatus.Status.ObservedGeneration {
+		if stsForStatus.Generation > stsForStatus.Status.ObservedGeneration || newSts.Generation > stsForStatus.Generation {
 			return false, nil
 		}

[f41gh7]

Bugfix will be included into the next operator release.

[AndrewChubatiuk]

available in operator 0.65.0 and in helm chart 0.55.0