mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-20 09:45:47 +00:00
* fix(helm): default sandbox Services to ClusterIP (#3929) The K8s sandbox provisioner supports both NodePort and ClusterIP via SANDBOX_SERVICE_TYPE (added in #4016), but the Helm chart never set it, so real-cluster installs inherited the NodePort default. That bound the code-execution sandbox on every node's interfaces - including externally reachable ones on GKE/EKS/AKS - and pinned every sandbox URL to one node IP (SPOF on node reboot/drain/ephemeral-IP). Default the chart to ClusterIP: the provisioner returns a cluster-DNS URL (http://sandbox-<id>-svc.<ns>.svc.cluster.local:8080) so the gateway-> sandbox hop stays inside the cluster network - no node IP, no 30xxx port, no external exposure. The chart always runs the gateway in-cluster, so ClusterIP is always correct there. NodePort remains an opt-in (provisioner.sandboxServiceType: NodePort + nodeHost) for the Docker-Compose/hybrid path where the gateway is not in K8s and cannot resolve .svc.cluster.local; the provisioner code default stays NodePort for that path. - values.yaml: add provisioner.sandboxServiceType ("ClusterIP") - provisioner-deployment.yaml: emit SANDBOX_SERVICE_TYPE; gate the NODE_HOST block on NodePort mode (default "ClusterIP" for upgrade safety) - NOTES.txt + README.md: document ClusterIP default + NodePort opt-in No change to docker/provisioner/app.py (already mode-aware since #4016) or RBAC (services verbs already cover ClusterIP). * test(helm): assert sandbox Service-type gating + CHANGELOG the default flip (#3929) Address review on #4190: - Add scripts/check_chart_sandbox_service.sh: renders the chart for the default (ClusterIP, no NODE_HOST), the NodePort opt-in (both emitted), and NodePort+nodeHost (literal value, not downward API). Locks in the #3929 gating so a regression (e.g. re-adding an unconditional NODE_HOST, or dropping the `default "ClusterIP"` upgrade-safety fallback) fails CI. Wired into .github/workflows/chart.yaml validate-chart job. (#2) - CHANGELOG [Unreleased] -> Changed: note the NodePort->ClusterIP default flip on upgrade + the `sandboxServiceType: NodePort` opt-back-in. (#4) No chart template changes (the gating itself landed in the first commit). --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
co-authored by
Willem Jiang
parent
65afc9b1d2
commit
d57f695769
@@ -4,9 +4,9 @@ name: Publish Helm Chart
|
||||
# container images (see container.yaml). Triggers on the same `v*` tags.
|
||||
#
|
||||
# On pull requests touching the chart or config.example.yaml, `validate-chart`
|
||||
# runs lint + template render + a config_version drift check (the chart's
|
||||
# embedded config_version must not lag config.example.yaml) so a broken or
|
||||
# stale chart fails the PR, not the release.
|
||||
# runs lint + template render + a sandbox Service-type gating check + a
|
||||
# config_version drift check (the chart's embedded config_version must not lag
|
||||
# config.example.yaml) so a broken or stale chart fails the PR, not the release.
|
||||
#
|
||||
# Users then install with:
|
||||
# helm install deer-flow oci://ghcr.io/${{ owner }}/charts/deer-flow --version <ver>
|
||||
@@ -21,6 +21,7 @@ on:
|
||||
- "config.example.yaml"
|
||||
- ".github/workflows/chart.yaml"
|
||||
- "scripts/check_config_version.sh"
|
||||
- "scripts/check_chart_sandbox_service.sh"
|
||||
|
||||
jobs:
|
||||
validate-chart:
|
||||
@@ -42,6 +43,12 @@ jobs:
|
||||
- name: Validate templates render
|
||||
run: helm template deer-flow deploy/helm/deer-flow --include-crds >/dev/null
|
||||
|
||||
# Assert the sandbox Service-type gating: default emits
|
||||
# SANDBOX_SERVICE_TYPE=ClusterIP and no NODE_HOST; the NodePort opt-in
|
||||
# emits both. Guards the #3929 default flip against regressions.
|
||||
- name: Validate sandbox Service-type gating
|
||||
run: bash scripts/check_chart_sandbox_service.sh
|
||||
|
||||
# The chart's `config:` block embeds a config_version that must not fall
|
||||
# behind config.example.yaml. A stale version is silent in-cluster (the
|
||||
# image ships no example to compare against, so _check_config_version
|
||||
|
||||
@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Changed
|
||||
|
||||
- **sandbox:** The Helm chart now defaults per-sandbox Services to `ClusterIP`
|
||||
instead of `NodePort`, so the code-execution sandbox is reachable only inside
|
||||
the cluster via Service DNS (`http://sandbox-<id>-svc.<ns>.svc.cluster.local`)
|
||||
and is no longer bound on every node's interfaces - including the
|
||||
externally-reachable ones on GKE/EKS/AKS. Existing chart installs flip
|
||||
NodePort -> ClusterIP on upgrade. To preserve the old reachability (an
|
||||
external probe hitting the 30xxx port, or the Docker-Compose/hybrid path
|
||||
where the gateway is not in K8s), set `provisioner.sandboxServiceType: NodePort`
|
||||
(with `provisioner.nodeHost` if needed). The provisioner itself is unchanged
|
||||
(mode-aware since #4016). ([#4190])
|
||||
|
||||
### ⚠ Breaking changes
|
||||
|
||||
- **skills:** A directory containing `SKILL.md` is now a runtime package
|
||||
@@ -590,3 +603,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#4095]: https://github.com/bytedance/deer-flow/issues/4095
|
||||
[#4098]: https://github.com/bytedance/deer-flow/pull/4098
|
||||
[#4146]: https://github.com/bytedance/deer-flow/pull/4146
|
||||
[#4190]: https://github.com/bytedance/deer-flow/pull/4190
|
||||
|
||||
@@ -376,17 +376,28 @@ an opt-in root `volumePermissions` initContainer that chowns on every start (the
|
||||
Bitnami pattern) — is not yet wired into this chart; it would introduce a root
|
||||
container, so it's left as an operator decision for now.
|
||||
|
||||
## Sandbox NodePort reachability
|
||||
## Sandbox Service type
|
||||
|
||||
The provisioner returns `http://{NODE_HOST}:{NodePort}` to the gateway so the
|
||||
agent can reach its sandbox. In Docker Compose `NODE_HOST=host.docker.internal`;
|
||||
in Kubernetes `NODE_HOST` **defaults to the provisioner pod's node IP** via the
|
||||
[downward API](https://kubernetes.io/docs/concepts/workloads/pods/downward-api/)
|
||||
(`status.hostIP`). Because a NodePort is exposed on every node, the gateway can
|
||||
reach `<node-IP>:<NodePort>` on most clusters without any configuration.
|
||||
The provisioner exposes each sandbox Pod behind a per-sandbox Service whose type
|
||||
is controlled by `provisioner.sandboxServiceType` (default `ClusterIP`).
|
||||
|
||||
Override `provisioner.nodeHost` only if your CNI or network policy blocks
|
||||
pod->node-IP traffic:
|
||||
**`ClusterIP` (default, recommended).** The provisioner returns a cluster-DNS
|
||||
URL - `http://sandbox-<id>-svc.<namespace>.svc.cluster.local:8080` - so the
|
||||
gateway reaches its sandbox entirely inside the cluster network. No node IP, no
|
||||
high port, and the code-execution surface is **not** bound on every node's
|
||||
interfaces. This is correct for the chart, where the gateway always runs
|
||||
in-cluster.
|
||||
|
||||
**`NodePort` (Docker-Compose/hybrid escape hatch).** Set
|
||||
`provisioner.sandboxServiceType: NodePort` only when the gateway is *not* in K8s
|
||||
(e.g. the compose dev path, where the gateway is a container reaching sandbox
|
||||
Pods on the host's Docker Desktop K8s). The provisioner then returns
|
||||
`http://{NODE_HOST}:{NodePort}`. `NODE_HOST` defaults to the provisioner pod's
|
||||
node IP via the [downward API](https://kubernetes.io/docs/concepts/workloads/pods/downward-api/)
|
||||
(`status.hostIP`); because a NodePort is exposed on every node, the gateway can
|
||||
reach `<node-IP>:<NodePort>` on most clusters without configuration. Override
|
||||
`provisioner.nodeHost` only if your CNI or network policy blocks pod->node-IP
|
||||
traffic:
|
||||
|
||||
```bash
|
||||
kubectl get nodes -o wide # use INTERNAL-IP or EXTERNAL-IP
|
||||
@@ -394,11 +405,13 @@ kubectl get nodes -o wide # use INTERNAL-IP or EXTERNAL-IP
|
||||
|
||||
```yaml
|
||||
provisioner:
|
||||
sandboxServiceType: NodePort
|
||||
nodeHost: 192.168.x.x
|
||||
```
|
||||
|
||||
On multi-node clusters, also switch `persistence.home.accessMode` to
|
||||
`ReadWriteMany`.
|
||||
`ReadWriteMany` (this is orthogonal to the Service type - it governs whether a
|
||||
sandbox Pod can be scheduled on a node other than the gateway's).
|
||||
|
||||
## Lint / dry-run
|
||||
|
||||
|
||||
@@ -39,24 +39,33 @@ Provisioner / sandbox notes:
|
||||
• PVC mode is enabled: sandbox Pods mount the `{{ include "deer-flow.homePVC" . }}`
|
||||
PVC for per-thread user-data.
|
||||
|
||||
⚠️ Sandbox NodePort reachability:
|
||||
The provisioner returns `http://{NODE_HOST}:{NodePort}` to the gateway so
|
||||
the agent can talk to its sandbox. NODE_HOST defaults to the provisioner
|
||||
pod's node IP via the Kubernetes downward API, which routes on most clusters
|
||||
because a NodePort is exposed on every node.
|
||||
|
||||
If your CNI or network policy blocks pod->node-IP traffic, set
|
||||
`provisioner.nodeHost` to an address the gateway can reach that routes to
|
||||
a node IP:
|
||||
• Sandbox reachability: per-sandbox Services are type
|
||||
`{{ .Values.provisioner.sandboxServiceType | default "ClusterIP" }}`.
|
||||
{{- if eq (default "ClusterIP" .Values.provisioner.sandboxServiceType) "NodePort" }}
|
||||
The provisioner returns `http://{NODE_HOST}:{NodePort}` to the gateway.
|
||||
NODE_HOST defaults to the provisioner pod's node IP via the downward API; a
|
||||
NodePort is exposed on every node, so `<node-IP>:<NodePort>` routes from the
|
||||
gateway on most clusters. If your CNI or network policy blocks pod->node-IP
|
||||
traffic, set `provisioner.nodeHost` to an address the gateway can reach:
|
||||
|
||||
kubectl get nodes -o wide
|
||||
|
||||
provisioner:
|
||||
nodeHost: <node-InternalIP-or-ExternalIP>
|
||||
|
||||
On multi-node clusters you additionally need `persistence.home.accessMode:
|
||||
ReadWriteMany` (e.g. NFS) so sandbox Pods on other nodes can mount the
|
||||
shared user-data PVC.
|
||||
⚠️ NodePort binds the code-execution sandbox on every node's interfaces,
|
||||
including externally-reachable ones. Prefer `ClusterIP` (the default) when
|
||||
the gateway runs in-cluster.
|
||||
{{- else }}
|
||||
The provisioner returns a cluster-DNS URL
|
||||
(`http://sandbox-<id>-svc.{{ include "deer-flow.namespace" . }}.svc.cluster.local:{{ .Values.provisioner.sandboxPort }}`)
|
||||
so the gateway reaches its sandbox inside the cluster - no node IP, no
|
||||
external port, no exposure on node interfaces.
|
||||
{{- end }}
|
||||
|
||||
• On multi-node clusters you additionally need
|
||||
`persistence.home.accessMode: ReadWriteMany` (e.g. NFS) so sandbox Pods on
|
||||
other nodes can mount the shared user-data PVC.
|
||||
|
||||
Generated secrets (persisted across upgrades):
|
||||
|
||||
|
||||
@@ -51,11 +51,18 @@ spec:
|
||||
value: {{ .Values.provisioner.sandboxImage | quote }}
|
||||
# In-cluster API access via the mounted ServiceAccount token.
|
||||
# K8S_API_SERVER is intentionally unset (do NOT use host.docker.internal).
|
||||
# NODE_HOST: the address the gateway uses to reach sandbox NodePorts.
|
||||
# When `provisioner.nodeHost` is set, use it; otherwise default to this
|
||||
# pod's node IP via the downward API. A NodePort is exposed on every
|
||||
# node, so <node-IP>:<NodePort> routes from the gateway on most clusters.
|
||||
# Override only when pod->node-IP traffic is blocked by the CNI/policy.
|
||||
# Service type for per-sandbox Services. ClusterIP (default) routes the
|
||||
# gateway->sandbox hop over cluster DNS; NodePort is the compose/hybrid
|
||||
# escape hatch (gateway not in K8s), which needs NODE_HOST below.
|
||||
- name: SANDBOX_SERVICE_TYPE
|
||||
value: {{ .Values.provisioner.sandboxServiceType | default "ClusterIP" | quote }}
|
||||
{{- if eq (default "ClusterIP" .Values.provisioner.sandboxServiceType) "NodePort" }}
|
||||
# NODE_HOST: address the gateway uses to reach sandbox NodePorts
|
||||
# (NodePort mode only). When `provisioner.nodeHost` is set, use it;
|
||||
# otherwise default to this pod's node IP via the downward API. A
|
||||
# NodePort is exposed on every node, so <node-IP>:<NodePort> routes
|
||||
# from the gateway on most clusters. Override only when pod->node-IP
|
||||
# traffic is blocked by the CNI/policy.
|
||||
- name: NODE_HOST
|
||||
{{- if .Values.provisioner.nodeHost }}
|
||||
value: {{ .Values.provisioner.nodeHost | quote }}
|
||||
@@ -64,6 +71,7 @@ spec:
|
||||
fieldRef:
|
||||
fieldPath: status.hostIP
|
||||
{{- end }}
|
||||
{{- end }}
|
||||
# PVC mode — sandbox Pods mount the same PVCs the gateway uses.
|
||||
{{- if .Values.persistence.home.enabled }}
|
||||
- name: USERDATA_PVC_NAME
|
||||
|
||||
@@ -94,10 +94,17 @@ provisioner:
|
||||
memory: 512Mi
|
||||
# -- AIO-compatible sandbox container image used for sandbox Pods.
|
||||
sandboxImage: "enterprise-public-cn-beijing.cr.volces.com/vefaas-public/all-in-one-sandbox:latest"
|
||||
# -- Host the gateway uses to reach sandbox NodePorts. Empty (default) falls
|
||||
# back to the provisioner pod's node IP via the Kubernetes downward API,
|
||||
# which routes on most clusters because a NodePort is exposed on every
|
||||
# node. Set explicitly only when pod->node-IP traffic is blocked by your
|
||||
# -- Service type for per-sandbox Services. `ClusterIP` (default) exposes
|
||||
# sandboxes only inside the cluster via Service DNS - correct for the
|
||||
# in-cluster gateway and avoids binding the code-execution surface on
|
||||
# every node's interfaces. `NodePort` is the Docker-Compose/hybrid escape
|
||||
# hatch (used when the gateway is not in K8s); pair with `nodeHost`.
|
||||
sandboxServiceType: "ClusterIP"
|
||||
# -- Host the gateway uses to reach sandbox NodePorts. Only consulted when
|
||||
# `sandboxServiceType: NodePort`. Empty (default) falls back to the
|
||||
# provisioner pod's node IP via the Kubernetes downward API, which routes
|
||||
# on most clusters because a NodePort is exposed on every node. Set
|
||||
# explicitly only when pod->node-IP traffic is blocked by your
|
||||
# CNI/network policy. In Docker Compose this is `host.docker.internal`.
|
||||
nodeHost: ""
|
||||
# -- Sandbox container port (must match the sandboxImage's listening port).
|
||||
|
||||
Executable
+89
@@ -0,0 +1,89 @@
|
||||
#!/usr/bin/env bash
|
||||
# Assert the Helm chart emits the sandbox Service-type env correctly:
|
||||
# - default (sandboxServiceType unset -> ClusterIP):
|
||||
# SANDBOX_SERVICE_TYPE=ClusterIP, NODE_HOST absent
|
||||
# - provisioner.sandboxServiceType=NodePort:
|
||||
# SANDBOX_SERVICE_TYPE=NodePort, NODE_HOST present
|
||||
# - NodePort + provisioner.nodeHost=<ip>:
|
||||
# NODE_HOST takes the literal value (not the downward API)
|
||||
#
|
||||
# Locks in the gating added for #3929 so a future change - e.g. re-adding an
|
||||
# unconditional NODE_HOST, or dropping the `default "ClusterIP"` fallback that
|
||||
# keeps a stale values.yaml from sending an empty string the provisioner
|
||||
# rejects - fails CI rather than silently regressing the sandbox exposure
|
||||
# surface. The provisioner itself is mode-aware since #4016; this guards the
|
||||
# chart wiring that selects the mode.
|
||||
#
|
||||
# Usage:
|
||||
# scripts/check_chart_sandbox_service.sh
|
||||
#
|
||||
# Called by .github/workflows/chart.yaml (validate-chart, on PRs + v* tags).
|
||||
# Requires `helm` (ubuntu-latest ships helm 3 preinstalled).
|
||||
#
|
||||
# Exit status is 0 when all assertions pass, 1 otherwise.
|
||||
|
||||
set -uo pipefail
|
||||
|
||||
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
CHART="$ROOT/deploy/helm/deer-flow"
|
||||
|
||||
if ! command -v helm >/dev/null 2>&1; then
|
||||
echo "::error::helm is required to run this check" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
TMP="$(mktemp -d)"
|
||||
trap 'rm -rf "$TMP"' EXIT
|
||||
|
||||
if ! helm template deer-flow "$CHART" --include-crds >"$TMP/default.yaml"; then
|
||||
echo "::error::default chart render failed" >&2
|
||||
exit 1
|
||||
fi
|
||||
if ! helm template deer-flow "$CHART" --include-crds \
|
||||
--set provisioner.sandboxServiceType=NodePort >"$TMP/nodeport.yaml"; then
|
||||
echo "::error::NodePort chart render failed" >&2
|
||||
exit 1
|
||||
fi
|
||||
if ! helm template deer-flow "$CHART" --include-crds \
|
||||
--set provisioner.sandboxServiceType=NodePort \
|
||||
--set provisioner.nodeHost=192.168.1.10 >"$TMP/nodeport-host.yaml"; then
|
||||
echo "::error::NodePort+nodeHost chart render failed" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# An env-var list item renders as ` - name: <NAME>`. Matching the item (not the
|
||||
# comments that merely mention the name) is what makes the NODE_HOST-absent
|
||||
# assertion meaningful.
|
||||
has_env() { grep -qE "^[[:space:]]*- name: $1\$" "$2"; }
|
||||
env_value() { grep -A1 -E "^[[:space:]]*- name: $1\$" "$2" | grep -oE 'value: "[^"]*"' | head -1; }
|
||||
|
||||
errors=0
|
||||
check() { # check <0|1> <desc> (0 == pass)
|
||||
if [ "$1" -eq 0 ]; then
|
||||
echo " PASS $2"
|
||||
else
|
||||
echo " FAIL $2"
|
||||
errors=$((errors + 1))
|
||||
fi
|
||||
}
|
||||
|
||||
echo "## Default render (provisioner.sandboxServiceType unset -> ClusterIP)"
|
||||
has_env SANDBOX_SERVICE_TYPE "$TMP/default.yaml"; check $? "SANDBOX_SERVICE_TYPE env present"
|
||||
[ "$(env_value SANDBOX_SERVICE_TYPE "$TMP/default.yaml")" = 'value: "ClusterIP"' ]; check $? "SANDBOX_SERVICE_TYPE == ClusterIP"
|
||||
if has_env NODE_HOST "$TMP/default.yaml"; then check 1 "NODE_HOST env absent"; else check 0 "NODE_HOST env absent"; fi
|
||||
|
||||
echo "## NodePort opt-in (provisioner.sandboxServiceType=NodePort)"
|
||||
has_env SANDBOX_SERVICE_TYPE "$TMP/nodeport.yaml"; check $? "SANDBOX_SERVICE_TYPE env present"
|
||||
[ "$(env_value SANDBOX_SERVICE_TYPE "$TMP/nodeport.yaml")" = 'value: "NodePort"' ]; check $? "SANDBOX_SERVICE_TYPE == NodePort"
|
||||
has_env NODE_HOST "$TMP/nodeport.yaml"; check $? "NODE_HOST env present"
|
||||
|
||||
echo "## NodePort + provisioner.nodeHost=192.168.1.10"
|
||||
[ "$(env_value NODE_HOST "$TMP/nodeport-host.yaml")" = 'value: "192.168.1.10"' ]; check $? "NODE_HOST == 192.168.1.10 (literal, not downward API)"
|
||||
|
||||
echo
|
||||
if [ "$errors" -eq 0 ]; then
|
||||
echo "All sandbox Service-type render assertions passed."
|
||||
exit 0
|
||||
fi
|
||||
echo "::error::$errors assertion(s) failed (see above)" >&2
|
||||
exit 1
|
||||
Reference in New Issue
Block a user