ft: Helm chart for backbeat lifecycle #103

tmacro · 2018-05-17T23:32:07Z

No description provided.

vrancurel · 2018-05-18T00:40:27Z

@rahulreddy do we name the charts backbeat-lifecycle or lifecycle ?

vrancurel

Can you move the files in charts/backbeat/templates/lifecycle/* ?

See #105

giacomoguiulfo · 2018-05-18T20:48:56Z

charts/backbeat-lifecycle/templates/conductor/deployment.yaml

+metadata:
+  name: {{ template "backbeat.fullname" . }}-conductor-lifecycle
+  labels:
+    app: {{ template "backbeat.name" . }}


Maybe the app label should be {{ template "backbeat.name" . }}-lifecycle?

giacomoguiulfo · 2018-05-18T20:52:36Z

charts/backbeat-lifecycle/templates/conductor/deployment.yaml

+  template:
+    metadata:
+      labels:
+        app: {{ template "backbeat.name" . }}


giacomoguiulfo · 2018-05-18T20:52:48Z

charts/backbeat-lifecycle/templates/consumer/deployment.yaml

+metadata:
+  name: {{ template "backbeat.fullname" . }}-consumer-lifecycle
+  labels:
+    app: {{ template "backbeat.name" . }}


giacomoguiulfo · 2018-05-18T20:53:05Z

charts/backbeat-lifecycle/templates/consumer/deployment.yaml

+  template:
+    metadata:
+      labels:
+        app: {{ template "backbeat.name" . }}


giacomoguiulfo · 2018-05-18T20:57:07Z

charts/backbeat-lifecycle/templates/conductor/deployment.yaml

+        release: {{ .Release.Name }}
+    spec:
+      containers:
+        - name: {{ .Chart.Name }}


It might be best if the container name matches the pod instead of just backbeat. Like this {{ template "backbeat.fullname" . }}-conductor-lifecycle

giacomoguiulfo · 2018-05-18T20:57:36Z

charts/backbeat-lifecycle/templates/consumer/deployment.yaml

+        release: {{ .Release.Name }}
+    spec:
+      containers:
+        - name: {{ .Chart.Name }}


Same for this container name.

giacomoguiulfo · 2018-05-18T20:58:36Z

charts/backbeat-lifecycle/templates/producer/deployment.yaml

+        release: {{ .Release.Name }}
+    spec:
+      containers:
+        - name: {{ .Chart.Name }}


Same for this container name.

giacomoguiulfo · 2018-05-18T21:00:44Z

charts/backbeat-lifecycle/templates/producer/deployment.yaml

+metadata:
+  name: {{ template "backbeat.fullname" . }}-producer-lifecycle
+  labels:
+    app: {{ template "backbeat.name" . }}


Same change here for the app label.

giacomoguiulfo · 2018-05-18T21:00:48Z

charts/backbeat-lifecycle/templates/producer/deployment.yaml

+  template:
+    metadata:
+      labels:
+        app: {{ template "backbeat.name" . }}


fixed

giacomoguiulfo

Needs to be rebased from the master branch. It includes some updates that will make this PR much smaller.

giacomoguiulfo

Are the changes in the values file missing?

giacomoguiulfo · 2018-05-18T22:58:01Z

charts/backbeat/templates/lifecycle/conductor_deployment.yaml

+            - name: ZOOKEEPER_AUTO_CREATE_NAMESPACE
+              value: "1"
+            - name: ZOOKEEPER_CONNECTION_STRING
+              value: "{{- printf "%s-zenko-zookeeper:2181" .Release.Name | trunc 63 | trimSuffix "-" -}}"


Should be zenko-quorum instead of zenko-zookeeper.

giacomoguiulfo · 2018-05-18T22:59:24Z

charts/backbeat/templates/lifecycle/conductor_deployment.yaml

+            - name: ZOOKEEPER_CONNECTION_STRING
+              value: "{{- printf "%s-zenko-zookeeper:2181" .Release.Name | trunc 63 | trimSuffix "-" -}}"
+            - name: KAFKA_HOSTS
+              value: "{{- printf "%s-kafka-0.%s-kafka:9092" .Release.Name .Release.Name | trunc 63 | trimSuffix "-" -}}"


zenko-quorum instead of kafka.

giacomoguiulfo · 2018-05-18T22:59:46Z

charts/backbeat/templates/lifecycle/consumer_deployment.yaml

+            - name: ZOOKEEPER_AUTO_CREATE_NAMESPACE
+              value: "1"
+            - name: ZOOKEEPER_CONNECTION_STRING
+              value: "{{- printf "%s-zenko-zookeeper:2181" .Release.Name | trunc 63 | trimSuffix "-" -}}"


giacomoguiulfo · 2018-05-18T22:59:55Z

charts/backbeat/templates/lifecycle/consumer_deployment.yaml

+            - name: ZOOKEEPER_CONNECTION_STRING
+              value: "{{- printf "%s-zenko-zookeeper:2181" .Release.Name | trunc 63 | trimSuffix "-" -}}"
+            - name: KAFKA_HOSTS
+              value: "{{- printf "%s-kafka-0.%s-kafka:9092" .Release.Name .Release.Name | trunc 63 | trimSuffix "-" -}}"


Added values

jonathan-gramain · 2018-05-19T00:05:08Z

charts/backbeat/templates/lifecycle/conductor_deployment.yaml

+    release: {{ .Release.Name }}
+    heritage: {{ .Release.Service }}
+spec:
+  replicas: {{ .Values.lifecycle.conductor.replicaCount }}


Since having more than one replica for the conductor will not behave correctly, I would rather hard-code "1" as the value and remove it from values.yml.

jonathan-gramain · 2018-05-19T00:10:55Z

charts/backbeat/templates/lifecycle/conductor_deployment.yaml

+              value: "{{- printf "%s-zenko-quorum:2181" .Release.Name | trunc 63 | trimSuffix "-" -}}"
+            - name: KAFKA_HOSTS
+              value: "{{- printf "%s-kafka-0.%s-kafka:9092" .Release.Name .Release.Name | trunc 63 | trimSuffix "-" -}}"
+            - name: MONGODB_HOSTS


Lifecycle components do not need access to mongodb so we should get rid of this env var for all.

jonathan-gramain · 2018-05-19T00:44:36Z

charts/backbeat/values.yaml

+      interval: 60
+
+  kafka:
+    bucket_task_topic: backbeat-lifecycle-bucket-tasks


Could make sense to group them into a topics subsection, and for consistency maybe rename to bucket_tasks_topic, same for object_tasks_topic.

Now that I think about it, I think it's better not to have the topic names in values.yaml as those are only for internal use and should not be modified by the admin in normal circumstances (so having them declared with their actual topic name in each chart would be better).

ssalaues · 2018-05-21T19:05:32Z

charts/backbeat/templates/lifecycle/producer_deployment.yaml

+            - name: ZOOKEEPER_CONNECTION_STRING
+              value: "{{- printf "%s-zenko-quorum:2181" .Release.Name | trunc 63 | trimSuffix "-" -}}"
+            - name: KAFKA_HOSTS
+              value: "{{- printf "%s-kafka-0.%s-kafka:9092" .Release.Name .Release.Name | trunc 63 | trimSuffix "-" -}}"


Should be able to use just the service name now as in "%s-zenko-queue:9092"

NicolasT · 2018-05-21T20:55:26Z

charts/backbeat/templates/lifecycle/conductor_deployment.yaml

+        release: {{ .Release.Name }}
+    spec:
+      containers:
+        - name: {{ template "backbeat.fullname" . }}-lifecycle-conductor


No need to use long names for containers. Use a descriptive name instead.

NicolasT · 2018-05-21T20:56:37Z

charts/backbeat/templates/lifecycle/conductor_deployment.yaml

+              value: service-lifecycle
+            - name: EXTENSIONS_LIFECYCLE_ZOOKEEPER_PATH
+              value: "{{ .Values.lifecycle.zookeeper.path }}"
+            - name: EXTENSIONS_LIFECYCLE_BACKLOG_METRICS_ZKPATH


What's the relation between metrics and Zookeeper?

The name is taken from the backbeat config file

"extensions": { "lifecycle": { ... "backlogMetrics": { "zkPath": "/lifecycle/run/backlog-metrics", .... }, }, }

NicolasT · 2018-05-21T20:57:10Z

charts/backbeat/templates/lifecycle/conductor_deployment.yaml

+              value: "{{ .Values.lifecycle.kafka.bucket_task_topic }}"
+            - name: EXTENSIONS_LIFECYCLE_OBJECT_TASK_TOPIC
+              value: "{{ .Values.lifecycle.kafka.object_task_topic }}"
+            - name: EXTENSIONS_LIFECYCLE_CONDUCTOR_CRONRULE


If we are doing cron'y things in our solution, wouldn't it make sense to use Kubernetes' CronJob objects? @jonathan-gramain @rahulreddy @vrancurel

I have no particular knowledge of cron jobs in kub but why not since it might be easier to control and visualize, though I think it's better to do this in a next step (more code changes to conductor process than just deployment settings).

NicolasT · 2018-05-21T20:58:56Z

charts/backbeat/values.yaml

+    tolerations: []
+    affinity: {}
+
+  zookeeper:


On one hand, some want to use 'functional' names for objects etc. instead of service names. On the other hand, we use service names in configuration. This is... confusing. And also, hard to change later.

I think it makes sense to keep them the same as the env vars inside the container. Less specific knowledge would be required to grok it at first glance.

giacomoguiulfo · 2018-05-22T17:58:51Z

charts/backbeat/templates/lifecycle/producer_deployment.yaml

+            - name: ZOOKEEPER_CONNECTION_STRING
+              value: "{{- printf "%s-zenko-quorum:2181" .Release.Name | trunc 63 | trimSuffix "-" -}}"
+            - name: KAFKA_HOSTS
+              value: "{{- printf "%s-zenko-0.%s-queue:9092" .Release.Name .Release.Name | trunc 63 | trimSuffix "-" -}}"


Small typo. It should look like thisvalue: "{{- printf "%s-zenko-queue:9092" .Release.Name | trunc 63 | trimSuffix "-" -}}"

jonathan-gramain · 2018-05-22T18:23:44Z

charts/backbeat/values.yaml

@@ -31,6 +31,45 @@ api:
  tolerations: []
  affinity: {}

+lifecycle:
+  conductor:
+    replicaCount: 1


I think you can remove this.

giacomoguiulfo

I think you need to rebase from master, to not show the changes in charts/value-dev.yml. But the rest looks good. I'll proceed to test it.

.

jonathan-gramain · 2018-05-22T21:27:25Z

charts/backbeat/templates/lifecycle/conductor_deployment.yaml

+            - name: EXTENSIONS_LIFECYCLE_BACKLOG_METRICS_INTERVALS
+              value: "{{ .Values.lifecycle.zookeeper.backlog_metrics.interval }}"
+            - name: EXTENSIONS_LIFECYCLE_BUCKET_TASK_TOPIC
+              value: bucket_tasks_topic


The actual topic name we use is backbeat-lifecycle-bucket-tasks, should be set here and for other lifecycle components.

jonathan-gramain · 2018-05-22T21:27:41Z

charts/backbeat/templates/lifecycle/conductor_deployment.yaml

+            - name: EXTENSIONS_LIFECYCLE_BUCKET_TASK_TOPIC
+              value: bucket_tasks_topic
+            - name: EXTENSIONS_LIFECYCLE_OBJECT_TASK_TOPIC
+              value: object_tasks_topic


Topic name is backbeat-lifecycle-object-tasks

rahulreddy · 2018-05-23T19:16:21Z

Where do we stand on this PR?

NicolasT · 2018-05-29T19:11:06Z

charts/backbeat/templates/lifecycle/producer_deployment.yaml

@@ -0,0 +1,74 @@
+apiVersion: extensions/v1beta1


This looks like 3x almost-the-same file. Consider abstracting out the template, parametrize it and run it 3 times?

Definitely agree with Nicolas but lets leave that for another PR. I wanted to separate the burden of the refactor and the addition of this feature. We have a ticket ZENKO-388 specifically to do exactly what Nicolas is speaking of. 😄

vrancurel

there are 3 silent fixes, but I still approve at this stage of the dev (and knowing there is a refacto after)

vrancurel · 2018-05-29T23:04:03Z

charts/backbeat/templates/replication/consumer_deployment.yaml

@@ -30,6 +30,8 @@ spec:
              value: "{{- printf "%s-zenko-quorum:2181" .Release.Name | trunc 63 | trimSuffix "-" -}}"
            - name: KAFKA_HOSTS
              value: "{{- printf "%s-zenko-queue:9092" .Release.Name | trunc 63 | trimSuffix "-" -}}"
+            - name: LOG_LEVEL


this shall be in another PR. we want to avoid silent fixes

vrancurel · 2018-05-29T23:04:19Z

charts/backbeat/templates/replication/producer_deployment.yaml

@@ -35,6 +35,8 @@ spec:
              value: "{{- printf "%s-%s" .Release.Name "redis-ha-master-svc" | trunc 63 | trimSuffix "-" -}}"
            - name: REDIS_PORT
              value: "6379"
+            - name: LOG_LEVEL


same, silent fix => another PR

vrancurel · 2018-05-29T23:04:39Z

charts/backbeat/templates/replication/status_deployment.yaml

@@ -44,6 +44,8 @@ spec:
              value: "{{- printf "%s-%s" .Release.Name "redis" | trunc 63 | trimSuffix "-" -}}"
            - name: REDIS_PORT
              value: "6379"
+            - name: LOG_LEVEL
+              value: {{ .Values.logging.level }}


ssalaues · 2018-05-31T16:57:22Z

We are waiting on scality/backbeat#299 before merging.

vrancurel · 2018-06-08T23:33:06Z

tested manually, bumped into ZENKO-481

ssalaues · 2018-06-12T18:43:43Z

It looks like bumping to the new cloudserver causes the the CI tests to fail. Is it a regression possibly?

+ add log level setting support

vrancurel previously requested changes May 18, 2018

View reviewed changes

giacomoguiulfo reviewed May 18, 2018

View reviewed changes

tmacro force-pushed the ft/ZENKO-300 branch from beda874 to b06c829 Compare May 18, 2018 21:07

giacomoguiulfo suggested changes May 18, 2018

View reviewed changes

tmacro force-pushed the ft/ZENKO-300 branch from b06c829 to 5612e3c Compare May 18, 2018 22:51

giacomoguiulfo previously requested changes May 18, 2018

View reviewed changes

tmacro force-pushed the ft/ZENKO-300 branch from 5612e3c to d190068 Compare May 18, 2018 23:17

jonathan-gramain reviewed May 21, 2018

View reviewed changes

ssalaues reviewed May 21, 2018

View reviewed changes

NicolasT reviewed May 21, 2018

View reviewed changes

tmacro force-pushed the ft/ZENKO-300 branch 2 times, most recently from 113a615 to 79e954f Compare May 22, 2018 17:49

giacomoguiulfo suggested changes May 22, 2018

View reviewed changes

tmacro force-pushed the ft/ZENKO-300 branch from 79e954f to 30b50ca Compare May 22, 2018 18:02

jonathan-gramain approved these changes May 22, 2018

View reviewed changes

giacomoguiulfo approved these changes May 22, 2018

View reviewed changes

giacomoguiulfo previously requested changes May 22, 2018

View reviewed changes

tmacro force-pushed the ft/ZENKO-300 branch from e8f3b75 to d040a66 Compare May 22, 2018 19:36

jonathan-gramain approved these changes May 22, 2018

View reviewed changes

jonathan-gramain reviewed May 22, 2018

View reviewed changes

jonathan-gramain force-pushed the ft/ZENKO-300 branch 3 times, most recently from 0ae9dbb to 83c0bb8 Compare May 24, 2018 20:59

NicolasT reviewed May 29, 2018

View reviewed changes

vrancurel approved these changes May 29, 2018

View reviewed changes

jonathan-gramain force-pushed the ft/ZENKO-300 branch from c344ede to 4a1f0cb Compare June 1, 2018 23:12

tcarmet force-pushed the ft/ZENKO-300 branch from 4a1f0cb to b349d7d Compare June 8, 2018 22:54

ssalaues force-pushed the ft/ZENKO-300 branch from 143ff5c to f6d77aa Compare June 13, 2018 18:43

giacomoguiulfo approved these changes Jun 13, 2018

View reviewed changes

tmacro and others added 3 commits June 15, 2018 11:02

ft: ZENKO-300 Add backbeat lifecycle chart

08321b0

+ add log level setting support

ft: bumping backbeat container tag to 0.2.4

cbca8c8

ft: update cloudserver image to 0.2.3

45cdf5d

giacomoguiulfo force-pushed the ft/ZENKO-300 branch from f6d77aa to 45cdf5d Compare June 15, 2018 18:02

giacomoguiulfo merged commit 148bcc6 into master Jun 15, 2018

giacomoguiulfo deleted the ft/ZENKO-300 branch June 15, 2018 19:13

ft: Helm chart for backbeat lifecycle #103

ft: Helm chart for backbeat lifecycle #103

Conversation

tmacro commented May 17, 2018

vrancurel commented May 18, 2018

vrancurel left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giacomoguiulfo left a comment • edited Loading

Choose a reason for hiding this comment

giacomoguiulfo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NicolasT May 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giacomoguiulfo May 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giacomoguiulfo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahulreddy commented May 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vrancurel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssalaues commented May 31, 2018

vrancurel commented Jun 8, 2018

ssalaues commented Jun 12, 2018

vrancurel left a comment •

edited

Loading

giacomoguiulfo left a comment •

edited

Loading

NicolasT May 21, 2018 •

edited

Loading

giacomoguiulfo May 22, 2018 •

edited

Loading

giacomoguiulfo left a comment •

edited

Loading