Skip to content

update helm deploy yamls #62

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
JacksonMei merged 1 commit into from
Jan 29, 2026
Merged

update helm deploy yamls #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions deploy/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ dependencies:
version: "0.0.1"
repository: "file://./api-service"
condition: api-service.enabled
- name: api-service
version: "0.0.1"
repository: "file://./api-service"
condition: api-service-k8s.enabled
alias: api-service-k8s
Comment on lines +30 to +34
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 29, 2026

Choose a reason for hiding this comment

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

medium

Duplicating the api-service dependency introduces a maintenance risk. If the version of the api-service subchart changes, it will need to be updated in two places, which is error-prone. While this is a common Helm pattern for deploying multiple instances of a subchart, it's important to be aware of this potential for inconsistency.


# Keywords for Helm repository search
keywords:
Expand Down
14 changes: 14 additions & 0 deletions deploy/api-service/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,17 @@ Selector labels
{{- printf "http://controller.%s.svc.cluster.local:8080" .Values.metadata.namespace -}}
{{ end }}
{{ end }}

{{- define "api-service.scheduleType" -}}
{{ if .Values.scheduleType }}
{{- .Values.scheduleType -}}
{{ else }}
{{- printf "k8s" .Values.metadata.namespace -}}
{{ end }}
{{ end }}

{{- define "api-service.qps" -}}
{{- if .Values.qps }}{{ printf "%v" .Values.qps }}{{ else }}{{ "10" }}{{ end }}
{{- end }}
Comment on lines +54 to +64
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 29, 2026

Choose a reason for hiding this comment

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

medium

The logic in these new helper templates can be significantly simplified and made more idiomatic. The api-service.scheduleType helper uses printf incorrectly, and both helpers can be made more concise by using the default function.

{{- define "api-service.scheduleType" -}}
    {{- .Values.scheduleType | default "k8s" -}}
{{ end }}

{{- define "api-service.qps" -}}
{{- .Values.qps | default "10" -}}
{{- end }}



4 changes: 4 additions & 0 deletions deploy/api-service/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ spec:
- {{ include "api-service.backendAddr" . }}
- --schedule-addr
- {{ include "api-service.scheduleAddr" . }}
- --schedule-type
- {{ include "api-service.scheduleType" . }}
- --qps
- {{ include "api-service.qps" . | quote }}
ports:
- name: http
containerPort: {{ .Values.service.port }}
Expand Down
2 changes: 1 addition & 1 deletion deploy/controller/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,5 @@ Create the name of the service account to use
Sandbox namespace
*/}}
{{- define "controller.sandboxNamespace" -}}
{{- .Values.sandboxNamespace | default "aenvsandbox" }}
{{- .Values.sandboxNamespace | default "aenv-sandbox" }}
{{- end }}
10 changes: 8 additions & 2 deletions deploy/controller/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@ spec:
args:
- --namespace
- {{ include "controller.sandboxNamespace" . }}
- --leader-elect
- {{ .Values.leaderElect | quote }}
{{- if .Values.leaderElect }}
- --leader-elect=true
{{- else }}
- --leader-elect=false
{{- end }}
Comment on lines +51 to +55
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 29, 2026

Choose a reason for hiding this comment

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

medium

This if/else block for the --leader-elect flag is a bit verbose. You can achieve the same result more concisely by embedding the value directly into the argument string.

            - --leader-elect={{ .Values.leaderElect }}

ports:
- name: http
containerPort: {{ .Values.service.port }}
Expand All @@ -67,6 +70,9 @@ spec:
periodSeconds: 5
successThreshold: 1
timeoutSeconds: 1
env:
- name: SERVICE_DOMAIN_SUFFIX
value: {{ .Values.serviceDomainSuffix | quote }}
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 29, 2026

Choose a reason for hiding this comment

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

high

The template uses .Values.serviceDomainSuffix, but this value is not defined in deploy/controller/values.yaml. If this value is not provided during Helm installation, template rendering will fail. It's crucial to provide a default value. The best practice is to add serviceDomainSuffix: "" to your values.yaml. As a defensive measure in the template, you can use the default function to prevent errors.

              value: {{ .Values.serviceDomainSuffix | default "" | quote }}

resources:
{{ toYaml .Values.resources | nindent 12 }}
{{- if .Values.podTemplates }}
Expand Down
2 changes: 1 addition & 1 deletion deploy/controller/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ name: controller

metadata:
namespace: aenv
sandboxNamespace: aenvsandbox
sandboxNamespace: aenv-sandbox

# Namespace configuration
namespace:
Expand Down
2 changes: 1 addition & 1 deletion deploy/controller/values/prod.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

metadata:
namespace: aenv
sandboxNamespace: aenvsandbox
sandboxNamespace: aenv-sandbox
storageAddr: ""
leaderElect: true

Expand Down
34 changes: 16 additions & 18 deletions deploy/envhub/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,38 +35,30 @@ spec:
image: {{ .Values.image }}
imagePullPolicy: {{.Values.imagePullPolicy }}
env:
- name: ACI_ACCESS
- name: OSS_ENDPOINT
valueFrom:
secretKeyRef:
name: envhub-secret
key: ACI_ACCESS
- name: ACI_SECRET
name: oss-secret
key: oss-endpoint
- name: OSS_BUCKET
valueFrom:
secretKeyRef:
name: envhub-secret
key: ACI_SECRET
{{- if .Values.oss.endpoint }}
- name: OSS_ENDPOINT
value: {{ .Values.oss.endpoint | quote }}
{{- end }}
{{- if .Values.oss.bucket }}
- name: OSS_BUCKET
value: {{ .Values.oss.bucket | quote }}
{{- end }}
name: oss-secret
key: oss-bucket
- name: OSS_KEY_PREFIX
value: {{ .Values.oss.keyPrefix | quote }}
- name: OSS_REGION
value: {{ .Values.oss.region | quote }}
- name: OSS_ACCESS_KEY_ID
valueFrom:
secretKeyRef:
name: envhub-secret
key: OSS_ACCESS_KEY_ID
name: oss-secret
key: oss-ak
- name: OSS_ACCESS_KEY_SECRET
valueFrom:
secretKeyRef:
name: envhub-secret
key: OSS_ACCESS_KEY_SECRET
name: oss-secret
key: oss-sk

args:
- --storage-backend
Expand All @@ -79,6 +71,12 @@ spec:
- {{ .Values.service.port | quote }}
- --metrics-port
- {{ .Values.service.metricsPort | quote }}
{{if .Values.deploy.templateId}}
- --template-id={{ .Values.deploy.templateId }}
{{end}}
{{if .Values.deploy.callbackURL}}
- --callback-url={{ .Values.deploy.callbackURL }}
{{end}}
Comment on lines +74 to +79
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 29, 2026

Choose a reason for hiding this comment

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

medium

There are a couple of improvements to be made here for style and whitespace control. It's a Helm convention to include a space after if (i.e., {{ if ... }}). Additionally, using {{- and -}} helps control newlines, preventing extra blank lines in the rendered YAML when the conditions are false.

            {{- if .Values.deploy.templateId }}
            - --template-id={{ .Values.deploy.templateId }}
            {{- end }}
            {{- if .Values.deploy.callbackURL }}
            - --callback-url={{ .Values.deploy.callbackURL }}
            {{- end }}

ports:
- name: http
containerPort: {{ .Values.service.port }}
Expand Down
61 changes: 54 additions & 7 deletions deploy/envhub/templates/secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,60 @@
# 创建Secret
# 为 envhub namespace 创建 OSS secret
apiVersion: v1
kind: Secret
metadata:
name: envhub-secret
name: oss-secret
namespace: {{ .Values.metadata.namespace }}
type: Opaque
data:
# base64编码的值
ACI_ACCESS: ""
ACI_SECRET: ""
OSS_ACCESS_KEY_ID: ""
OSS_ACCESS_KEY_SECRET: ""
# OSS credentials (base64 encoded)
{{- if .Values.oss.accessKeyId }}
oss-ak: {{ .Values.oss.accessKeyId | b64enc | quote }}
{{- else }}
oss-ak: ""
{{- end }}
{{- if .Values.oss.accessKeySecret }}
oss-sk: {{ .Values.oss.accessKeySecret | b64enc | quote }}
{{- else }}
oss-sk: ""
{{- end }}
{{- if .Values.oss.endpoint }}
oss-endpoint: {{ .Values.oss.endpoint | b64enc | quote }}
{{- else }}
oss-endpoint: ""
{{- end }}
{{- if .Values.oss.bucket }}
oss-bucket: {{ .Values.oss.bucket | b64enc | quote }}
{{- else }}
oss-bucket: ""
{{- end }}
---
# 为 aenv-sandbox namespace 创建相同的 OSS secret
# 用于 service pod 访问 OSS
apiVersion: v1
kind: Secret
metadata:
name: oss-secret
namespace: {{ .Values.global.sandboxNamespace }}
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 29, 2026

Choose a reason for hiding this comment

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

high

There is an inconsistency in how sandboxNamespace is accessed across subcharts. This chart (envhub) uses a global value (.Values.global.sandboxNamespace), while the controller chart uses a local value (.Values.sandboxNamespace). This can lead to configuration errors and makes the umbrella chart difficult to manage. For shared configuration like this, you should standardize on using a single global value defined in the parent chart's values.yaml and accessed via .Values.global.sandboxNamespace in all subcharts.

type: Opaque
data:
# OSS credentials (base64 encoded)
{{- if .Values.oss.accessKeyId }}
oss-ak: {{ .Values.oss.accessKeyId | b64enc | quote }}
{{- else }}
oss-ak: ""
{{- end }}
{{- if .Values.oss.accessKeySecret }}
oss-sk: {{ .Values.oss.accessKeySecret | b64enc | quote }}
{{- else }}
oss-sk: ""
{{- end }}
{{- if .Values.oss.endpoint }}
oss-endpoint: {{ .Values.oss.endpoint | b64enc | quote }}
{{- else }}
oss-endpoint: ""
{{- end }}
{{- if .Values.oss.bucket }}
oss-bucket: {{ .Values.oss.bucket | b64enc | quote }}
{{- else }}
oss-bucket: ""
{{- end }}
Comment on lines +1 to +60
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 29, 2026

Choose a reason for hiding this comment

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

medium

This file defines the same oss-secret twice, once for each namespace. This leads to significant code duplication and is a maintenance concern. If the secret's structure changes, you'll have to update it in two places.

A better approach is to define a helper template in _helpers.tpl for the secret data and then include it in both secret definitions. This follows the Don't Repeat Yourself (DRY) principle.

For example, in deploy/envhub/templates/_helpers.tpl:

{{- define "envhub.ossSecretData" -}}
# OSS credentials (base64 encoded)
...
{{- end -}}

And then in secret.yaml:

...
data:
{{ include "envhub.ossSecretData" . | nindent 2 }}
---
...
data:
{{ include "envhub.ossSecretData" . | nindent 2 }}

8 changes: 6 additions & 2 deletions deploy/envhub/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ podSecurityContext: {}

securityContext: {}

deploy:
templateId: ""
callbackURL: ""

service:
type: LoadBalancer
port: 8083
Expand Down Expand Up @@ -67,5 +71,5 @@ oss:
bucket: "" # OSS bucket name, e.g., "your-bucket-name"
keyPrefix: "" # Key prefix for OSS objects
region: "" # OSS region， e.g. "cn-hangzhou"
# Access credentials are loaded from secret (OSS_ACCESS_KEY_ID and OSS_ACCESS_KEY_SECRET)
# or from environment variables
accessKeyId: "" # OSS Access Key ID (will be base64 encoded in secret)
accessKeySecret: "" # OSS Access Key Secret (will be base64 encoded in secret)
2 changes: 1 addition & 1 deletion deploy/redis/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ persistence:
storageClassName: ""
accessModes:
- ReadWriteOnce
size: 10Gi
size: 5Gi

# Redis 配置（无密码模式）
redis:
Expand Down
Loading