Implement index operator logic for read replica rotation (#2444) (#2456)

* Remove job reconciler * Add leader election to index operator * Add roles for leader election * Add operator logic for rotation * style: format code with Gofumpt and Prettier This commit fixes the style issues introduced in 844cca1 according to the output from Gofumpt and Prettier. Details: #2444 * Remove unnecesary change * Fix build * nits * Refactor * Fix clusterrole * Move to podv2 * Add job concurrency check * Refactor * Refactor * Fix heavy copying * Update helm schema * Fix package doc * Remove unused code and refactor For() method * Update target read replica ID in config * Update internal/k8s/podv2/option.go * Refactor * rename podv2 to v2/pod * Add client DI * Fix envkey helper * Set namespace from values.yaml * Use annotations to specify the id * Update schema * Update sample * Update values * Update schema * Fix spelling * Fix package name * Fix read replica e2e * Add job templates for rotation, creation, saving, and correction * Update config.go to decode k8s object * Update sample.yaml for index operator * Use job template from cfg * style: format code with Gofumpt and Prettier This commit fixes the style issues introduced in 584d1f0 according to the output from Gofumpt and Prettier. Details: #2444 * Fix tagalign * Fix config test * Fix config test * Disable read replica e2e for now --------- Signed-off-by: Yusuke Kadowaki <[email protected]> Co-authored-by: Yusuke Kadowaki <[email protected]> Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com> Co-authored-by: Hiroto Funakoshi <[email protected]>
vdaas · Mar 15, 2024 · 4956c62 · 4956c62
1 parent 29a81fd
commit 4956c62
Show file tree

Hide file tree

Showing 33 changed files with 2,205 additions and 1,340 deletions.
diff --git a/.github/helm/values/values-readreplica.yaml b/.github/helm/values/values-readreplica.yaml
@@ -80,7 +80,3 @@ manager:
     readreplica:
       rotator:
         enabled: true
-        initContainers: []
-        env:
-          - name: MY_TARGET_REPLICA_ID
-            value: "0"
diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml
@@ -279,6 +279,9 @@ jobs:
   e2e-stream-crud-with-readreplica:
     name: "E2E test (Stream CRUD) with read replica"
     needs: [dump-contexts-to-log]
+    # FIXME: This job is disabled because it is not working properly for a moment.
+    #        Needs to fix TestE2EReadReplica not to use CronJob since there is no CronJob for read replica anymore.
+    if: false
     runs-on: ubuntu-latest
     timeout-minutes: 60
     steps:

diff --git a/charts/vald-helm-operator/crds/valdrelease.yaml b/charts/vald-helm-operator/crds/valdrelease.yaml
@@ -10064,6 +10064,9 @@ spec:
                                   type: string
                                 maxUnavailable:
                                   type: string
+                            rotation_job_concurrency:
+                              type: integer
+                              minimum: 1
                             securityContext:
                               type: object
                               x-kubernetes-preserve-unknown-fields: true
@@ -10726,8 +10729,6 @@ spec:
                                 podSecurityContext:
                                   type: object
                                   x-kubernetes-preserve-unknown-fields: true
-                                read_replica_id:
-                                  type: string
                                 securityContext:
                                   type: object
                                   x-kubernetes-preserve-unknown-fields: true
@@ -11240,6 +11241,8 @@ spec:
                                       type: boolean
                                     name:
                                       type: string
+                                target_read_replica_id_annotations_key:
+                                  type: string
                                 ttlSecondsAfterFinished:
                                   type: integer
                                 version:

diff --git a/charts/vald/README.md b/charts/vald/README.md
diff --git a/charts/vald/templates/_helpers.tpl b/charts/vald/templates/_helpers.tpl
@@ -24,6 +24,19 @@ If release name contains chart name it will be used as a full name.
 {{- end -}}
 {{- end -}}
 
+{{/*
+Create a envkey for read replica target id.
+We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
+If release name contains chart name it will be used as a full name.
+*/}}
+{{- define "vald.target_read_replica_envkey" -}}
+{{- if .Values.fullnameOverride -}}
+{{- printf "%s_%s" "TARGET_READREPLICA_ID" .Values.fullnameOverride  | upper | replace "-" "_" | trunc 63 -}}
+{{- else -}}
+{{- printf "%s_%s_%s_%s" "TARGET_READREPLICA_ID" .Release.Name .Release.Namespace .Chart.Name | upper | replace "-" "_" | trunc 63 -}}
+{{- end -}}
+{{- end -}}
+
 {{/*
 Create chart name and version as used by the chart label.
 */}}
@@ -35,13 +48,17 @@ Create chart name and version as used by the chart label.
 Common labels
 */}}
 {{- define "vald.labels" -}}
-app.kubernetes.io/name: {{ include "vald.name" . }}
-helm.sh/chart: {{ include "vald.chart" . }}
-app.kubernetes.io/instance: {{ .Release.Name }}
-{{- if .Chart.AppVersion }}
-app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
+app: {{ .Values.name }}
+app.kubernetes.io/name: {{ include "vald.name" .default }}
+helm.sh/chart: {{ include "vald.chart" .default }}
+app.kubernetes.io/managed-by: {{ .default.Release.Service }}
+app.kubernetes.io/instance: {{ .default.Release.Name }}
+app.kubernetes.io/component: {{ .Values.name }}
+{{- if .default.Chart.AppVersion }}
+app.kubernetes.io/version: {{ .default.Chart.AppVersion | quote }}
+{{- else }}
+app.kubernetes.io/version: {{ .default.Chart.Version }}
 {{- end }}
-app.kubernetes.io/managed-by: {{ .Release.Service }}
 {{- end -}}
 
 {{/*
@@ -848,3 +865,70 @@ service:
   fields: {}
   {{- end }}
 {{- end -}}
+
+{{/*
+Vald index job templates
+*/}}
+{{- define "vald.index_job" -}}
+spec:
+  ttlSecondsAfterFinished: {{ .Job.ttlSecondsAfterFinished }}
+  template:
+    metadata:
+      labels:
+        {{- include "vald.labels" (dict "Values" .Job "default" .default) | nindent 8 }}
+      annotations:
+        {{- $pprof := default .default.Values.defaults.server_config.metrics.pprof .Job.server_config.metrics.pprof -}}
+        {{- if $pprof.enabled }}
+        pyroscope.io/scrape: "true"
+        pyroscope.io/application-name: {{ .Job.name }}
+        pyroscope.io/profile-cpu-enabled: "true"
+        pyroscope.io/profile-mem-enabled: "true"
+        pyroscope.io/port: {{ $pprof.port | quote }}
+        {{- end }}
+    spec:
+      {{- if .Job.initContainers }}
+      initContainers:
+        {{- $initContainers := dict "initContainers" .Job.initContainers "Values" .default.Values "namespace" .default.Release.Namespace -}}
+        {{- include "vald.initContainers" $initContainers | trim | nindent 8 }}
+        {{- if .Job.securityContext }}
+        securityContext:
+          {{- toYaml .Job.securityContext | nindent 12 }}
+        {{- end }}
+      {{- end }}
+      containers:
+        - name: {{ .Job.name }}
+          image: "{{ .Job.image.repository }}:{{ default .default.Values.defaults.image.tag .Job.image.tag }}"
+          imagePullPolicy: {{ .Job.image.pullPolicy }}
+          volumeMounts:
+            - name: {{ .Job.name }}-config
+              mountPath: /etc/server/
+          {{- $servers := dict "Values" .Job.server_config "default" .default.Values.defaults.server_config -}}
+          {{- include "vald.containerPorts" $servers | trim | nindent 10 }}
+          {{- if .Job.securityContext }}
+          securityContext:
+            {{- toYaml .Job.securityContext | nindent 12 }}
+          {{- end }}
+          {{- if .Job.env }}
+          env:
+            {{- toYaml .Job.env | nindent 12 }}
+            {{- if eq .type "rotator" }}
+            - name: {{ include "vald.target_read_replica_envkey" .default }}
+              valueFrom:
+                fieldRef:
+                  fieldPath: metadata.annotations['{{ .Job.target_read_replica_id_annotations_key }}']
+            {{- end }}
+          {{- end }}
+      {{- if .Job.podSecurityContext }}
+      securityContext:
+        {{- toYaml .Job.podSecurityContext | nindent 8 }}
+      {{- end }}
+      restartPolicy: OnFailure
+      volumes:
+        - name: {{ .Job.name }}-config
+          configMap:
+            defaultMode: 420
+            name: {{ .Job.name }}-config
+      {{- if .Job.serviceAccount }}
+      serviceAccountName: {{ .Job.serviceAccount.name }}
+      {{- end }}
+{{- end -}}
diff --git a/charts/vald/templates/discoverer/clusterrole.yaml b/charts/vald/templates/discoverer/clusterrole.yaml
@@ -30,6 +30,7 @@ rules:
   - apiGroups:
       - apps
     resources:
+      - deployments
       - replicasets
     verbs:
       - get
@@ -47,6 +48,12 @@ rules:
       - get
       - list
       - watch
+  - apiGroups:
+      - ""
+    resources:
+      - events
+    verbs:
+      - create
   - nonResourceURLs:
       - /metrics
     verbs:
@@ -63,8 +70,18 @@ rules:
       - batch
     resources:
       - jobs
+      - cronjobs
     verbs:
       - get
       - list
       - watch
+      - create
+  - apiGroups:
+      - "coordination.k8s.io"
+    resources:
+      - leases
+    verbs:
+      - get
+      - update
+      - create
 {{- end }}
diff --git a/charts/vald/templates/index/job/correction/cronjob.yaml b/charts/vald/templates/index/job/correction/cronjob.yaml
@@ -20,56 +20,12 @@ kind: CronJob
 metadata:
   name: {{ $corrector.name }}
   labels:
-    app: {{ $corrector.name }}
-    app.kubernetes.io/name: {{ include "vald.name" . }}
-    helm.sh/chart: {{ include "vald.chart" . }}
-    app.kubernetes.io/managed-by: {{ .Release.Service }}
-    app.kubernetes.io/instance: {{ .Release.Name }}
-    app.kubernetes.io/version: {{ .Chart.Version }}
+    {{- include "vald.labels" (dict "Values" $corrector "default" .) | nindent 10 }}
 spec:
   schedule: {{ $corrector.schedule | quote }}
   concurrencyPolicy: Forbid
   suspend: {{ $corrector.suspend }}
   startingDeadlineSeconds: {{ $corrector.startingDeadlineSeconds }}
   jobTemplate:
-    spec:
-      ttlSecondsAfterFinished: {{ $corrector.ttlSecondsAfterFinished }}
-      template:
-        metadata:
-          labels:
-            app: {{ $corrector.name }}
-          annotations:
-            {{- $pprof := default .Values.defaults.server_config.metrics.pprof $corrector.server_config.metrics.pprof -}}
-            {{- if $pprof.enabled }}
-            pyroscope.io/scrape: "true"
-            pyroscope.io/application-name: {{ $corrector.name }}
-            pyroscope.io/profile-cpu-enabled: "true"
-            pyroscope.io/profile-mem-enabled: "true"
-            pyroscope.io/port: {{ $pprof.port | quote }}
-            {{- end }}
-        spec:
-          {{- if $corrector.initContainers }}
-          initContainers:
-            {{- $initContainers := dict "initContainers" $corrector.initContainers "Values" .Values "namespace" .Release.Namespace -}}
-            {{- include "vald.initContainers" $initContainers | trim | nindent 12 }}
-          {{- end }}
-          containers:
-            - name: {{ $corrector.name }}
-              image: "{{ $corrector.image.repository }}:{{ default .Values.defaults.image.tag $corrector.image.tag }}"
-              imagePullPolicy: {{ $corrector.image.pullPolicy }}
-              volumeMounts:
-                - name: {{ $corrector.name }}-config
-                  mountPath: /etc/server/
-              {{- $servers := dict "Values" $corrector.server_config "default" .Values.defaults.server_config -}}
-              {{- include "vald.containerPorts" $servers | trim | nindent 14 }}
-              {{- if $corrector.env }}
-              env:
-                {{- toYaml $corrector.env | nindent 16 }}
-              {{- end }}
-          restartPolicy: OnFailure
-          volumes:
-            - name: {{ $corrector.name }}-config
-              configMap:
-                defaultMode: 420
-                name: {{ $corrector.name }}-config
+    {{- include "vald.index_job" (dict "Job" $corrector "default" . "type" "corrector") | nindent 10 }}
 {{- end }}
diff --git a/charts/vald/templates/index/job/creation/cronjob.yaml b/charts/vald/templates/index/job/creation/cronjob.yaml
@@ -20,59 +20,12 @@ kind: CronJob
 metadata:
   name: {{ $creator.name }}
   labels:
-    app: {{ $creator.name }}
-    app.kubernetes.io/name: {{ include "vald.name" . }}
-    helm.sh/chart: {{ include "vald.chart" . }}
-    app.kubernetes.io/managed-by: {{ .Release.Service }}
-    app.kubernetes.io/instance: {{ .Release.Name }}
-    app.kubernetes.io/version: {{ .Chart.Version }}
+    {{- include "vald.labels" (dict "Values" $creator "default" .) | nindent 10 }}
 spec:
   schedule: {{ $creator.schedule | quote }}
   concurrencyPolicy: Forbid
   suspend: {{ $creator.suspend }}
   startingDeadlineSeconds: {{ $creator.startingDeadlineSeconds }}
   jobTemplate:
-    spec:
-      ttlSecondsAfterFinished: {{ $creator.ttlSecondsAfterFinished }}
-      template:
-        metadata:
-          labels:
-            app: {{ $creator.name }}
-            app.kubernetes.io/name: {{ include "vald.name" . }}
-            app.kubernetes.io/instance: {{ .Release.Name }}
-            app.kubernetes.io/component: {{ $creator.name }}
-          annotations:
-            {{- $pprof := default .Values.defaults.server_config.metrics.pprof $creator.server_config.metrics.pprof -}}
-            {{- if $pprof.enabled }}
-            pyroscope.io/scrape: "true"
-            pyroscope.io/application-name: {{ $creator.name }}
-            pyroscope.io/profile-cpu-enabled: "true"
-            pyroscope.io/profile-mem-enabled: "true"
-            pyroscope.io/port: {{ $pprof.port | quote }}
-            {{- end }}
-        spec:
-          {{- if $creator.initContainers }}
-          initContainers:
-            {{- $initContainers := dict "initContainers" $creator.initContainers "Values" .Values "namespace" .Release.Namespace -}}
-            {{- include "vald.initContainers" $initContainers | trim | nindent 12 }}
-          {{- end }}
-          containers:
-            - name: {{ $creator.name }}
-              image: "{{ $creator.image.repository }}:{{ default .Values.defaults.image.tag $creator.image.tag }}"
-              imagePullPolicy: {{ $creator.image.pullPolicy }}
-              volumeMounts:
-                - name: {{ $creator.name }}-config
-                  mountPath: /etc/server/
-              {{- $servers := dict "Values" $creator.server_config "default" .Values.defaults.server_config -}}
-              {{- include "vald.containerPorts" $servers | trim | nindent 14 }}
-              {{- if $creator.env }}
-              env:
-                {{- toYaml $creator.env | nindent 16 }}
-              {{- end }}
-          restartPolicy: OnFailure
-          volumes:
-            - name: {{ $creator.name }}-config
-              configMap:
-                defaultMode: 420
-                name: {{ $creator.name }}-config
+    {{- include "vald.index_job" (dict "Job" $creator "default" . "type" "creator") | nindent 10 }}
 {{- end }}
diff --git a/charts/vald/templates/index/job/readreplica/rotate/configmap.yaml b/charts/vald/templates/index/job/readreplica/rotate/configmap.yaml
@@ -48,6 +48,6 @@ data:
     rotator:
       agent_namespace: {{ $rotator.agent_namespace | quote }}
       read_replica_label_key: {{ $agent.readreplica.label_key | quote }}
-      read_replica_id: "_MY_TARGET_REPLICA_ID_"
+      target_read_replica_id: _{{ include "vald.target_read_replica_envkey" . }}_
       volume_name: {{ $agent.readreplica.volume_name | quote }}
 {{- end }}