Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New version of zdm-proxy helm chart using StatefulSets #89

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
9f32a90
initial collection of helm charts to deploy zdm into k8s
weideng1 Dec 14, 2022
9896ee1
added README file
weideng1 Dec 14, 2022
e842935
pull zdm-proxy docker image version from helm chart's AppVersion
weideng1 Dec 14, 2022
29b0b4d
fixed a bug so release name can be customized now
weideng1 Dec 14, 2022
3d46813
added a big more verification info in README
weideng1 Dec 15, 2022
0e765f3
added Cassandra Data Migrator (CDM) to the helm chart and adjusted hi…
weideng1 Dec 15, 2022
9647c87
use cdm image 2.10.3 to solve python3 dependency
weideng1 Dec 15, 2022
83a7cc6
modify README instructions for local dev machine
weideng1 Dec 15, 2022
bb8c038
changed README instructions to move secrets lifecycle outside of helm…
weideng1 Dec 15, 2022
461139e
address another small comment from Jim
weideng1 Dec 15, 2022
7235572
move ZDM_ORIGIN_PORT from configmap to secret, which is preset before…
weideng1 Dec 15, 2022
eb5c49b
mpromoted a bunch of values that should be user configurable to value…
weideng1 Dec 15, 2022
6eaf66f
fixed a quote bug in configmap
weideng1 Dec 15, 2022
286f7c9
fixed three issues Phil found in his tests
weideng1 Dec 15, 2022
a42d5e9
address Joao's comments to reduce zdm-proxy memory down to 2GB
weideng1 Dec 15, 2022
f58e7b3
Merge branch 'main' into k8s-helm
weideng1 Dec 16, 2022
5d52bd0
change cdm to pod instead of deployment, as it doesn't need to be aut…
weideng1 Dec 23, 2022
f86e2c8
fixed a bug caused by merging two commits
weideng1 Dec 23, 2022
6f57a02
switched k8s deployment to statefulsets for proxy pods #88
weideng1 Dec 28, 2022
e8dfd54
Merge branch 'main' into k8s-helm
weideng1 Dec 28, 2022
9f1affc
Merge branch 'main' into k8s-helm
weideng1 Jan 13, 2023
692a11b
remove CDM from helm chart
weideng1 Jan 18, 2023
4ef5754
remove deployment.yaml
weideng1 Jan 24, 2023
98636c4
revised helm chart according to John's comments on values.yaml best p…
weideng1 Jan 25, 2023
dd1c433
fix values in README.md that are no longer accurate
weideng1 Jan 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions k8s_helm_charts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,27 @@ Usage:

4. Verify that all components are up and running.

```kubectl -n zdmproxy get svc,cm,secret,deploy,po -o wide --show-labels```
```kubectl -n zdmproxy get svc,ep,po,cm,secret -o wide --show-labels```

You can also run ```kubectl -n zdmproxy logs pod/zdm-proxy-0``` to see if there are the following entries in the log, which means everything is working as expected:

You can also run ```kubectl -n zdmproxy logs pod/zdm-proxy-0-xxxxxxx``` to see if there are the following entries in the log, which means everything is working as expected:

```
time="2022-12-14T21:19:57Z" level=info msg="Proxy connected and ready to accept queries on 172.25.132.116:9042"
time="2022-12-14T21:19:57Z" level=info msg="Proxy started. Waiting for SIGINT/SIGTERM to shutdown."
```

5. When you're done, run helm uninstall to remove all objects.
5. Switch primary cluster to target (all proxy pods will automatically roll-restart after the change).

```helm -n zdmproxy upgrade zdm-proxy ./zdm --set proxy.primaryCluster=TARGET```

6. Scale out/in to different number of proxy pods.

```helm -n zdmproxy upgrade zdm-proxy ./zdm --set proxy.count=5```

Note: if you've already switched primary cluster to target, make sure you add ```--set proxy.primaryCluster=TARGET``` in this command line as well. An alternative is to directly edit zdm/values.yaml then run helm upgrade.

7. When you're done, run helm uninstall to remove all objects.


```helm -n zdmproxy uninstall zdm-proxy```
74 changes: 0 additions & 74 deletions k8s_helm_charts/zdm/templates/cdm.yaml

This file was deleted.

2 changes: 1 addition & 1 deletion k8s_helm_charts/zdm/templates/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ spec:
name: cql
selector:
{{- $zdm_selectorLabels | nindent 4 }}
app: {{ $zdm_fullname }}-{{ $index }}
statefulset.kubernetes.io/pod-name: {{ $zdm_fullname }}-{{ $index }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you trying to match a per-pod label?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsanda The reason for us to create individual services and match them 1:1 at pod level is because unlike C*, zdm-proxy processes don't have gossip protocol to detect/notify topology changes among themselves; whatever set of IP addresses and their respective ordinal index (see here) specified at the start of the proxy process is going to stuck for the rest of its lifecycle. To allow dynamical IP address change to the pods due to reschedule/cordon/crash, we decided to for N number of proxies, implement N number of services, and them map 1-on-1. Given that pods managed by k8s Deployment object doesn't have a static pod name, we switched to using StatefulSets. In StatefulSets, each pod has a unique/static pod-name that we can map the service to. This will allow orderly rolling restart of the zdm-proxy pods to happen.

---
{{- end -}}
134 changes: 134 additions & 0 deletions k8s_helm_charts/zdm/templates/sts.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
# pull in these templates outside of range loop
{{ $zdm_fullname := include "zdm.fullname" . -}}
{{- $zdm_labels := include "zdm.labels" . -}}
{{- $zdm_selectorLabels := include "zdm.selectorLabels" . -}}

# calculate a variable that contains all proxy service addresses
{{ $service_addresses := "" -}}
{{- range $index := until (.Values.proxy.count | int) -}}
{{- $service_addresses = printf "%s,$(%s_%s_SERVICE_HOST)" $service_addresses ($zdm_fullname | upper | replace "-" "_") ($index | toString) -}}
{{- end -}}
{{- $service_addresses = $service_addresses | trimPrefix "," -}}

weideng1 marked this conversation as resolved.
Show resolved Hide resolved
apiVersion: apps/v1
kind: StatefulSet
metadata:
labels:
{{- $zdm_labels | nindent 4 }}
app: {{ $zdm_fullname }}
name: {{ $zdm_fullname }}
namespace: {{ $.Values.namespace }}
spec:
serviceName: {{ $zdm_fullname }}
replicas: {{ $.Values.proxy.count | int }}
selector:
matchLabels:
{{- $zdm_selectorLabels | nindent 6 }}
app: {{ $zdm_fullname }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider these recommended labels from the k8s docs.

https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/

template:
metadata:
labels:
{{- $zdm_selectorLabels | nindent 8 }}
app: {{ $zdm_fullname }}
spec:
containers:
- image: "{{ $.Values.proxy.image.repository }}:{{ $.Values.proxy.image.tag | default $.Chart.AppVersion }}"
name: {{ $zdm_fullname }}
command:
- sh
- "-c"
- "ZDM_PROXY_TOPOLOGY_INDEX=`echo ${HOSTNAME##*-}` /main"
Comment on lines +38 to +40
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's surprising that you're specifying a command here vs just passing in arguments to the command specified in the Docker image (or relying on arguments there).

Comment on lines +38 to +40
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to populate an environment variable with a fieldRef from metadata.name. See https://kubernetes.io/docs/tasks/inject-data-application/environment-variable-expose-pod-information/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it looks like you're extracting the ordinal index which is not available to the pod directly. It's frustrating that this isn't available. (I see the link to kubernetes/kubernetes#40651 as well)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bradfordcp Yes exactly. That was the route I explored and ended up using the workaround as you pointed out. Should we resolve this comment?

livenessProbe:
httpGet:
path: /health/liveness
port: httpproxy
initialDelaySeconds: 10
periodSeconds: 10
readinessProbe:
httpGet:
path: /health/readiness
port: httpproxy
initialDelaySeconds: 10
periodSeconds: 10
ports:
- name: httpproxy
containerPort: 14001
protocol: TCP
- name: cql
containerPort: 9042
protocol: TCP
resources:
weideng1 marked this conversation as resolved.
Show resolved Hide resolved
requests:
memory: {{ $.Values.proxy.resources.requests.memory | quote }}
cpu: {{ $.Values.proxy.resources.requests.cpu | quote }}
limits:
memory: {{ $.Values.proxy.resources.limits.memory | quote }}
cpu: {{ $.Values.proxy.resources.limits.cpu | quote }}
env:
- name: ZDM_PRIMARY_CLUSTER
value: {{ $.Values.proxy.primaryCluster | upper | quote }}
- name: ZDM_READ_MODE
value: {{ $.Values.proxy.readMode | upper | quote }}
- name: ZDM_LOG_LEVEL
value: {{ $.Values.proxy.logLevel | upper | quote }}
- name: ZDM_PROXY_MAX_CLIENT_CONNECTIONS
value: {{ $.Values.proxy.maxClientConnections | quote }}
- name: ZDM_METRICS_ENABLED
value: {{ $.Values.proxy.metricsEnabled | quote }}
- name: ZDM_PROXY_LISTEN_ADDRESS
valueFrom:
fieldRef:
fieldPath: status.podIP
- name: ZDM_PROXY_LISTEN_PORT
value: "9042"
- name: ZDM_METRICS_ADDRESS
valueFrom:
fieldRef:
fieldPath: status.podIP
- name: ZDM_METRICS_PORT
value: "14001"
- name: ZDM_PROXY_TOPOLOGY_ADDRESSES
value: {{ $service_addresses }}
- name: ZDM_TARGET_SECURE_CONNECT_BUNDLE_PATH
value: /tmp/scb/target.zip
- name: ZDM_ORIGIN_CONTACT_POINTS
valueFrom:
secretKeyRef:
name: zdmproxy
key: origin_contact_points
- name: ZDM_ORIGIN_PORT
valueFrom:
secretKeyRef:
name: zdmproxy
key: origin_port
- name: ZDM_ORIGIN_USERNAME
valueFrom:
secretKeyRef:
name: zdmproxy
key: origin_username
- name: ZDM_ORIGIN_PASSWORD
valueFrom:
secretKeyRef:
name: zdmproxy
key: origin_password
- name: ZDM_TARGET_USERNAME
valueFrom:
secretKeyRef:
name: zdmproxy
key: target_username
- name: ZDM_TARGET_PASSWORD
valueFrom:
secretKeyRef:
name: zdmproxy
key: target_password
volumeMounts:
- name: scb
mountPath: "/tmp/scb"
readOnly: true
volumes:
- name: scb
secret:
secretName: zdmproxy-scb
items:
- key: secure-connect-target.zip
path: target.zip
12 changes: 0 additions & 12 deletions k8s_helm_charts/zdm/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,6 @@ proxy:
# Overrides the image tag whose default is the chart appVersion.
tag: ""

cdm:
resources:
limits:
cpu: 16000m
memory: 32768Mi
requests:
cpu: 16000m
memory: 32768Mi
image:
repository: datastax/cassandra-data-migrator
tag: 2.10.3

nameOverride: ""
fullnameOverride: ""

Expand Down