Skip to content

feat(helm): Add API server deployment. #1818

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.

Sign up for GitHub

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

Merged
junhaoliao merged 12 commits into from
Jan 5, 2026
Merged

feat(helm): Add API server deployment. #1818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7636aee
feat(helm): Add api-server deployment to the chart.
junhaoliao Dec 19, 2025
62bd3ef
add the missing charts...
junhaoliao Dec 19, 2025
e5088bf
Merge branch 'main' into helm-api-server
jackluo923 Dec 19, 2025
b31f5f7
Merge branch 'main' into helm-api-server
jackluo923 Dec 19, 2025
6484c22
Merge branch 'main' into helm-api-server
junhaoliao Dec 19, 2025
7b80cfd
chore(helm): Bump chart version to 0.1.2-dev.12
junhaoliao Dec 19, 2025
8fd3c51
Merge branch 'main' into helm-api-server
hoophalab Dec 20, 2025
8d0a9af
Merge branch 'main' into helm-api-server
junhaoliao Dec 20, 2025
9e2639e
fix(helm): Update api-server health check port to use named port "api…
junhaoliao Dec 20, 2025
88f6097
Merge branch 'main' into helm-api-server
junhaoliao Dec 29, 2025
26b5055
fix(helm): update termination grace period for api-server deployments…
junhaoliao Jan 5, 2026
a9a342b
fix(helm): reorder config volume definition in api-server deployment:…
junhaoliao Jan 5, 2026
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
2 changes: 1 addition & 1 deletion tools/deployment/package-helm/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: "v2"
name: "clp"
version: "0.1.2-dev.11"
version: "0.1.2-dev.12"
description: "A Helm chart for CLP's (Compressed Log Processor) package deployment"
type: "application"
appVersion: "0.7.1-dev"
Expand Down
103 changes: 103 additions & 0 deletions tools/deployment/package-helm/templates/api-server-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
{{- if .Values.clpConfig.api_server }}
apiVersion: "apps/v1"
kind: "Deployment"
metadata:
name: {{ include "clp.fullname" . }}-api-server
labels:
{{- include "clp.labels" . | nindent 4 }}
app.kubernetes.io/component: "api-server"
spec:
replicas: 1
selector:
matchLabels:
{{- include "clp.selectorLabels" . | nindent 6 }}
app.kubernetes.io/component: "api-server"
template:
metadata:
labels:
{{- include "clp.labels" . | nindent 8 }}
app.kubernetes.io/component: "api-server"
spec:
serviceAccountName: {{ include "clp.fullname" . }}-job-watcher
terminationGracePeriodSeconds: 60
securityContext:
runAsUser: {{ .Values.securityContext.firstParty.uid }}
runAsGroup: {{ .Values.securityContext.firstParty.gid }}
fsGroup: {{ .Values.securityContext.firstParty.gid }}
initContainers:
- {{- include "clp.waitFor" (dict
"root" .
"type" "job"
"name" "db-table-creator"
) | nindent 10 }}
- {{- include "clp.waitFor" (dict
"root" .
"type" "job"
"name" "results-cache-indices-creator"
) | nindent 10 }}
containers:
- name: "api-server"
image: "{{ include "clp.image.ref" . }}"
imagePullPolicy: "{{ .Values.image.clpPackage.pullPolicy }}"
env:
- name: "CLP_DB_PASS"
valueFrom:
secretKeyRef:
name: {{ include "clp.fullname" . }}-database
key: "password"
- name: "CLP_DB_USER"
valueFrom:
secretKeyRef:
name: {{ include "clp.fullname" . }}-database
key: "username"
- name: "CLP_LOGS_DIR"
value: "/var/log/api_server"
- name: "RUST_LOG"
value: "INFO"
Comment on lines +42 to +56
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider making RUST_LOG configurable.

The RUST_LOG environment variable is hardcoded to "INFO". For debugging or troubleshooting, operators may need to adjust the log level without modifying the chart.

🔎 Suggested enhancement

In values.yaml, add:

api_server:
  log_level: "INFO"

Then update the template:

           - name: "RUST_LOG"
-            value: "INFO"
+            value: {{ .Values.clpConfig.api_server.log_level | quote }}

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 46-46: too many spaces inside braces

(braces)

[error] 46-46: too many spaces inside braces

(braces)

[error] 51-51: too many spaces inside braces

(braces)

[error] 51-51: too many spaces inside braces

(braces)

🤖 Prompt for AI Agents 
In @tools/deployment/package-helm/templates/api-server-deployment.yaml around
lines 42-56, Make RUST_LOG configurable by adding an api_server.log_level entry
in values.yaml (default "INFO") and update the deployment template to replace
the hardcoded RUST_LOG value with a Helm value lookup that uses
api_server.log_level with a fallback to "INFO" when unset; reference the
RUST_LOG env var in the template change and ensure chart documentation/README
notes the new api_server.log_level option.

ports:
- name: "api-server"
containerPort: 3001
Comment on lines +38 to +59
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Resource limits and requests remain unaddressed.

The past review comment about adding resource requests and limits is still valid. Without these, the scheduler cannot make optimal placement decisions, and the pod could consume excessive cluster resources.

Consider exposing resource configuration in values.yaml (e.g., resources.apiServer.requests and resources.apiServer.limits) and templating them here for production readiness.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 46-46: too many spaces inside braces

(braces)

[error] 46-46: too many spaces inside braces

(braces)

[error] 51-51: too many spaces inside braces

(braces)

[error] 51-51: too many spaces inside braces

(braces)

🤖 Prompt for AI Agents 
In @tools/deployment/package-helm/templates/api-server-deployment.yaml around
lines 38-59, Add configurable resource requests/limits to the api-server
container spec: update the container with a templated resources block that reads
values from values.yaml keys like resources.apiServer.requests and
resources.apiServer.limits so the "api-server" container can receive CPU/memory
requests and limits; expose these keys in values.yaml
(resources.apiServer.requests and resources.apiServer.limits) with sensible
defaults and reference them in the template using the Helm templating helpers
(e.g., using .Values.resources.apiServer.* and include defaulting via default or
required functions) so the scheduler can make placement decisions and avoid
unbounded resource use.

volumeMounts:
- name: {{ include "clp.volumeName" (dict
"component_category" "api-server"
"name" "logs"
) | quote }}
mountPath: "/var/log/api_server"
- name: "config"
mountPath: "/etc/clp-config.yaml"
subPath: "clp-config.yaml"
readOnly: true
- name: {{ include "clp.volumeName" (dict
"component_category" "shared-data"
"name" "streams"
) | quote }}
mountPath: "/var/data/streams"
Comment on lines +60 to +74
Copy link
Contributor

@hoophalab hoophalab Dec 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thinking about this again: this volume mount probably won't work on a true cluster (not minikube/kind) right? They need to be PVs on a nfs?

And even if the user specifies s3 in archive output, the pod would probably fail to start because this mount is unconditional?

Copy link
Member Author

@junhaoliao junhaoliao Dec 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

good points. those are valid concerns and we do plan to address them:

  1. streams ... need to be PVs on a nfs

    If everything is running on a single node (i.e., the same host), this isn't an issue because all pods can access the same files via the local filesystem. However, once we scale query workers across multiple nodes, or place query workers on different nodes from the API server (and similarly the Web UI), each node gets its own PV backed by the same hostPath. If that hostPath is not on shared storage, files created by a pod on one node will not be accessible to pods on other nodes. I believe this is what you mean by "won't work". While we recommend S3 in a distributed compute environment, if users insist to use "fs" storage, we plan to ask users to set up distributed storage and create (FUSE) mounts at those host locations, so files can be shared. For example, SeadweedFs can be used. (I plan to submit those user guides to docs(helm): Add Kubernetes Helm user guides and dev guides; Restructure user documentation. #1827 but haven't pushed all of them. will try to get them all polished and pushed tonight. sorry)

  2. even if the user specifies s3 in archive output, the pod would probably fail to start because this mount is unconditional

    You're right to question this. The plan is to add the conditional template in feat(helm): Add S3 storage support for archives, streams, and log inputs. #1825 . We will review feat(helm): Add S3 storage support for archives, streams, and log inputs. #1825 once feat(helm): Add garbage collector deployment with retention period configuration. #1817 and feat(helm): Add API server deployment. #1818 are merged.

Copy link
Contributor

@hoophalab hoophalab Dec 22, 2025

Choose a reason for hiding this comment

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

xD It's probably almost impossible to configure seaweedfs on every node in a serious use case, but we could always ask them to use s3

Copy link
Member Author

@junhaoliao junhaoliao Dec 29, 2025

Choose a reason for hiding this comment

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

The concerns of the ability to deploy on a multi-node cluster are fairly valid. Some changes will be made in #1829 to address such concerns

command: [
"/opt/clp/bin/api_server",
"--host", "0.0.0.0",
"--port", "3001",
"--config", "/etc/clp-config.yaml"
]
readinessProbe:
{{- include "clp.readinessProbeTimings" . | nindent 12 }}
httpGet: &api-server-health-check
path: "/health"
port: "api-server"
livenessProbe:
{{- include "clp.livenessProbeTimings" . | nindent 12 }}
httpGet: *api-server-health-check
volumes:
- {{- include "clp.pvcVolume" (dict
"root" .
"component_category" "api-server"
"name" "logs"
) | nindent 10 }}
Comment on lines +90 to +94
Copy link
Member

@kirkrodrigues kirkrodrigues Jan 4, 2026

Choose a reason for hiding this comment

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

For consistency with other helm files, can we move this below the config mount?

Copy link
Member Author

@junhaoliao junhaoliao Jan 5, 2026

Choose a reason for hiding this comment

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

i suppose you mean listing the (non-templated) config mount as the last item, for consistency with other Helm files, and i made the change accordingly

- {{- include "clp.pvcVolume" (dict
"root" .
"component_category" "shared-data"
"name" "streams"
) | nindent 10 }}
- name: "config"
configMap:
name: {{ include "clp.fullname" . }}-config
{{- end }}
11 changes: 11 additions & 0 deletions tools/deployment/package-helm/templates/api-server-logs-pv.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{{- if .Values.clpConfig.api_server }}
{{- include "clp.createLocalPv" (dict
"root" .
"component_category" "api-server"
"name" "logs"
"nodeRole" "control-plane"
"capacity" "5Gi"
"accessModes" (list "ReadWriteOnce")
"hostPath" (printf "%s/api_server" .Values.clpConfig.logs_directory)
) }}
{{- end }}
9 changes: 9 additions & 0 deletions tools/deployment/package-helm/templates/api-server-logs-pvc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{{- if .Values.clpConfig.api_server }}
{{- include "clp.createPvc" (dict
"root" .
"component_category" "api-server"
"name" "logs"
"capacity" "5Gi"
"accessModes" (list "ReadWriteOnce")
) }}
{{- end }}
18 changes: 18 additions & 0 deletions tools/deployment/package-helm/templates/api-server-service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{{- if .Values.clpConfig.api_server }}
apiVersion: "v1"
kind: "Service"
metadata:
name: {{ include "clp.fullname" . }}-api-server
labels:
{{- include "clp.labels" . | nindent 4 }}
app.kubernetes.io/component: "api-server"
spec:
type: "NodePort"
selector:
{{- include "clp.selectorLabels" . | nindent 4 }}
app.kubernetes.io/component: "api-server"
ports:
- port: 3001
targetPort: "api-server"
nodePort: {{ .Values.clpConfig.api_server.port }}
{{- end }}
11 changes: 11 additions & 0 deletions tools/deployment/package-helm/templates/configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ metadata:
{{- include "clp.labels" . | nindent 4 }}
data:
clp-config.yaml: |
{{- with .Values.clpConfig.api_server }}
api_server:
default_max_num_query_results: {{ .default_max_num_query_results | int }}
host: "localhost"
port: 3001
query_job_polling:
initial_backoff_ms: {{ .query_job_polling.initial_backoff_ms | int }}
max_backoff_ms: {{ .query_job_polling.max_backoff_ms | int }}
{{- else }}
api_server: null
{{- end }}
archive_output:
compression_level: {{ .Values.clpConfig.archive_output.compression_level }}
storage:
Expand Down
5 changes: 4 additions & 1 deletion tools/deployment/package-helm/test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ mkdir -p "$CLP_HOME/var/"{data,log}/{database,queue,redis,results_cache} \
"$CLP_HOME/var/data/"{archives,streams} \
"$CLP_HOME/var/log/"{compression_scheduler,compression_worker,user} \
"$CLP_HOME/var/log/"{query_scheduler,query_worker,reducer} \
"$CLP_HOME/var/log/garbage_collector" \
"$CLP_HOME/var/log/"{api_server,garbage_collector} \
"$CLP_HOME/var/tmp" \
"$CLP_HOME/samples"

Expand Down Expand Up @@ -94,6 +94,9 @@ cat <<EOF | kind create cluster --name clp-test --config=-
- containerPort: 30000
hostPort: 30000
protocol: TCP
- containerPort: 30301
hostPort: 30301
protocol: TCP
EOF

helm uninstall test --ignore-not-found
Expand Down
8 changes: 8 additions & 0 deletions tools/deployment/package-helm/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ clpConfig:
results_metadata_collection_name: "results-metadata"
rate_limit: 1000

# API server config
api_server:
port: 30301
default_max_num_query_results: 1000
query_job_polling:
initial_backoff_ms: 100
max_backoff_ms: 5000

# Where archives should be output to
archive_output:
storage:
Expand Down
Loading