mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-28 14:10:37 +00:00
fix(helm): let the sandbox provisioner run with several replicas (#6543)
* fix(helm): let the sandbox provisioner run with several replicas
The chart hard-coded replicas: 1 into the provisioner Deployment, gave it
no rollout strategy and no PodDisruptionBudget, and values.yaml had no
provisioner.replicas key, so a multi-replica gateway deployment still sent
every sandbox create/discover/destroy call through one Pod: while that Pod
restarted or its node drained no sandbox could be created anywhere.
- values.yaml provisioner gains replicas (default 1, unchanged), the
gateway's surge-then-drain RollingUpdate strategy and
podDisruptionBudget {enabled: true, minAvailable: 1}.
- provisioner-deployment.yaml renders replicas and strategy from values; at
1 replica the result equals the Kubernetes default strategy.
- New provisioner-pdb.yaml, rendered only while the provisioner is enabled,
the budget is enabled and replicas > 1, selecting on the Deployment's
labels and named <fullname>-provisioner so it never collides with the
gateway budget. No multiInstance override: nothing else in the chart
renders differently per provisioner replica count.
- docker/provisioner/app.py is unchanged. It keeps no state between
requests: the labelled sandbox Pods and Services are the only registry,
create tolerates the 409 a concurrent creator produces, destroy tolerates
404, NodePorts come from the API server and there is no background loop.
The chart README and docker/provisioner/README.md record this, and
tests/test_helm_provisioner_replicas.py pins it at source level next to
the values, template and helm-render assertions.
* docs(changelog): link the provisioner replicas entry to #6543
* fix(helm): preserve percentage PodDisruptionBudget values instead of casting them to 0
Review of #6543: both PDB templates rendered minAvailable through
`| default 1 | int`, and sprig's int turns a percentage string such as
"50%" -- a value Kubernetes accepts -- into 0, a budget that permits every
Pod to be evicted. The gateway budget had the same pre-existing cast.
- New shared helper deer-flow.pdbMinAvailable in _helpers.tpl: a YAML or
--set number renders as an integer, a digit string from --set-string
becomes that integer, a percentage is preserved quoted, and 0, "0%",
more than 100%, a non-integer or anything else fails the render with a
message naming the values key and the enabled switch to use instead.
Unset still falls back to 1.
- gateway-pdb.yaml and provisioner-pdb.yaml render minAvailable through it.
- values.yaml comments, the chart README multi-replica note and the
CHANGELOG entry say integer-or-percentage and that zero is rejected.
- tests/test_helm_provisioner_replicas.py pins the helper and the two
templates at text level and adds render cases for both budgets: "50%"
with three replicas renders "50%", 2 via --set and --set-string renders
2, and abc / 0 / 0% / 150% fail naming the key.
* fix(helm): normalize json.Number before the minAvailable string branches
Review of #6543: Helm 3.18.0 loads YAML numbers as json.Number, whose
kind is "string" but which regexMatch and the other string-typed sprig
functions reject at argument validation ("expected string; got
json.Number"). deer-flow.pdbMinAvailable gated its string branch on
kindIs "string", so the unoverridden chart default minAvailable: 1 made
every gateway.replicas=2 / provisioner.replicas=2 render fail on that
release, inside the README's documented 3.8+ support window.
- The helper keeps the explicit float64/int64/int branch and now converts
everything else with toString into $text before the digit, percentage
and fail branches; regexMatch, atoi, trimSuffix and quote only ever see
that plain string. A json.Number 1 renders 1, a json.Number 1.5 still
fails (the digit regex rejects it) and the hint still prints the raw
value; bools and other kinds still fail. The helper comment records the
Helm 3.18.0 reason.
- tests/test_helm_provisioner_replicas.py pins that the string branch
runs on the toString-normalized variable, that no string-typed function
is applied to $raw and that kindIs "string" is gone, and adds an
explicit render case for both budgets with only the replica count set
(the unoverridden default must render minAvailable: 1), marked as the
Helm 3.18.0 json.Number regression guard.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
a8250eaa4b
commit
7ff13bad45
18
CHANGELOG.md
18
CHANGELOG.md
@ -1185,6 +1185,23 @@ This release closes that milestone with **439 merged pull requests**.
|
||||
Upgrading a release that predates `AUTH_JWT_SECRET` generates a new key and
|
||||
signs every browser session out once; the chart README shows how to seed
|
||||
the previous key into the Secret first to keep sessions. ([#6347])
|
||||
- **deploy:** The Helm chart no longer pins the sandbox provisioner to one Pod.
|
||||
`provisioner.replicas` (default 1, unchanged) sets the replica count, the
|
||||
provisioner Deployment gets the gateway's surge-then-drain rollout strategy
|
||||
(`maxSurge: 1`, `maxUnavailable: 0`), and a `PodDisruptionBudget`
|
||||
(`provisioner.podDisruptionBudget`, `minAvailable: 1`) is rendered while
|
||||
`provisioner.replicas > 1`, so a multi-replica gateway deployment no longer
|
||||
loses sandbox creation whenever its single provisioner Pod restarts or its
|
||||
node drains. The provisioner itself needed no change: the labelled sandbox
|
||||
Pods and Services are its only registry, every handler reads them back from
|
||||
the API server, and create already tolerates the `409 AlreadyExists` a
|
||||
concurrent creator produces. The chart README and the provisioner README
|
||||
record what the replica count rests on and that the budget never renders
|
||||
for a single replica. Both budgets render `minAvailable` through one helper
|
||||
that preserves a percentage such as `"50%"` (the previous `int` cast, also
|
||||
in the pre-existing gateway budget, silently turned it into `0`, a budget
|
||||
that protects nothing) and fails the render for `0`, `"0%"` or any other
|
||||
unsupported value with a message naming the values key. ([#6543])
|
||||
- **persistence:** A second Gateway instance no longer fails startup with
|
||||
`TimeoutError` while another instance runs a PostgreSQL schema migration. The
|
||||
bootstrap advisory lock was taken with a blocking `pg_advisory_lock` on the
|
||||
@ -9318,3 +9335,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#6512]: https://github.com/bytedance/deer-flow/pull/6512
|
||||
[#6519]: https://github.com/bytedance/deer-flow/pull/6519
|
||||
[#6537]: https://github.com/bytedance/deer-flow/pull/6537
|
||||
[#6543]: https://github.com/bytedance/deer-flow/pull/6543
|
||||
|
||||
310
backend/tests/test_helm_provisioner_replicas.py
Normal file
310
backend/tests/test_helm_provisioner_replicas.py
Normal file
@ -0,0 +1,310 @@
|
||||
"""Regression tests for the Helm chart's sandbox-provisioner replica count.
|
||||
|
||||
The chart hard-coded ``replicas: 1`` into the provisioner Deployment, gave it
|
||||
no rollout strategy and no PodDisruptionBudget, and ``values.yaml`` had no
|
||||
``provisioner.replicas`` key, so a multi-replica gateway deployment still sent
|
||||
every sandbox create/discover/destroy call through one Pod: while that Pod
|
||||
restarted or its node drained no sandbox could be created. The provisioner
|
||||
itself keeps no state between requests -- the labelled sandbox Pods and
|
||||
Services are the only registry, every handler reads them back from the API
|
||||
server, and create tolerates the 409 a concurrent creator produces -- so the
|
||||
chart only had to stop pinning it. These tests pin the values keys, the
|
||||
Deployment wiring, the PodDisruptionBudget guard (enabled and replicas > 1),
|
||||
the label agreement between the two templates, the operator notes, and the
|
||||
source-level statelessness the replica claim rests on.
|
||||
|
||||
Review of #6543 found that both budgets rendered ``minAvailable`` through
|
||||
``| int``, which turns a percentage such as ``"50%"`` (a value Kubernetes
|
||||
accepts) into ``0`` -- a budget that protects nothing -- so the tests also pin
|
||||
the shared ``deer-flow.pdbMinAvailable`` helper: integers pass through, digit
|
||||
strings from ``--set-string`` become integers, percentages stay quoted, and
|
||||
``0`` or anything else fails the render naming the values key.
|
||||
|
||||
The ``helm template`` tests skip when helm is not installed; CI's runner has it.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
CHART = REPO_ROOT / "deploy" / "helm" / "deer-flow"
|
||||
VALUES = CHART / "values.yaml"
|
||||
README = CHART / "README.md"
|
||||
DEPLOYMENT_TEMPLATE = CHART / "templates" / "provisioner-deployment.yaml"
|
||||
PDB_TEMPLATE = CHART / "templates" / "provisioner-pdb.yaml"
|
||||
GATEWAY_PDB_TEMPLATE = CHART / "templates" / "gateway-pdb.yaml"
|
||||
HELPERS_TEMPLATE = CHART / "templates" / "_helpers.tpl"
|
||||
PDB_TEMPLATES = {"gateway": GATEWAY_PDB_TEMPLATE, "provisioner": PDB_TEMPLATE}
|
||||
PROVISIONER_APP = REPO_ROOT / "docker" / "provisioner" / "app.py"
|
||||
PROVISIONER_README = REPO_ROOT / "docker" / "provisioner" / "README.md"
|
||||
|
||||
# A selector block as both templates spell it: the shared selector helper plus
|
||||
# the component label, so the budget can only ever count provisioner Pods.
|
||||
SELECTOR_BLOCK = re.compile(r"matchLabels:\n\s+\{\{- include \"deer-flow.selectorLabels\" \. \| nindent 6 \}\}\n\s+app\.kubernetes\.io/component: provisioner\n")
|
||||
|
||||
# Methods that mutate a container in place. A module-level container touched by
|
||||
# one of these from a request handler would be per-Pod state the chart's
|
||||
# replica claim could not survive.
|
||||
MUTATING_METHODS = frozenset({"add", "append", "clear", "discard", "extend", "insert", "pop", "popitem", "remove", "setdefault", "update"})
|
||||
BACKGROUND_WORK = frozenset({"create_task", "ensure_future", "Thread", "Timer", "add_event_handler", "on_event", "repeat_every"})
|
||||
|
||||
|
||||
def _values() -> dict:
|
||||
return yaml.safe_load(VALUES.read_text(encoding="utf-8"))
|
||||
|
||||
|
||||
def _helm(*args: str) -> subprocess.CompletedProcess[str]:
|
||||
helm = shutil.which("helm")
|
||||
if helm is None:
|
||||
pytest.skip("helm is unavailable")
|
||||
return subprocess.run([helm, "template", "deer-flow", str(CHART), *args], capture_output=True, text=True)
|
||||
|
||||
|
||||
def _render(*args: str) -> list[dict]:
|
||||
result = _helm(*args)
|
||||
assert result.returncode == 0, result.stderr
|
||||
return [document for document in yaml.safe_load_all(result.stdout) if isinstance(document, dict)]
|
||||
|
||||
|
||||
def _render_chart(*settings: str) -> list[dict]:
|
||||
return _render(*(arg for setting in settings for arg in ("--set", setting)))
|
||||
|
||||
|
||||
def _render_failure(*args: str) -> str:
|
||||
"""Return helm's stderr for a render that must be refused."""
|
||||
result = _helm(*args)
|
||||
assert result.returncode != 0, "the render must fail instead of producing a budget that protects nothing"
|
||||
return result.stderr
|
||||
|
||||
|
||||
def _min_available_include(component: str) -> str:
|
||||
return f'minAvailable: {{{{ include "deer-flow.pdbMinAvailable" (dict "value" $pdb.minAvailable "key" "{component}.podDisruptionBudget.minAvailable") }}}}'
|
||||
|
||||
|
||||
def _find(documents: list[dict], kind: str, name_suffix: str) -> dict | None:
|
||||
return next((document for document in documents if document.get("kind") == kind and document["metadata"]["name"].endswith(name_suffix)), None)
|
||||
|
||||
|
||||
def _by_kind(documents: list[dict], kind: str, name_suffix: str) -> dict:
|
||||
document = _find(documents, kind, name_suffix)
|
||||
assert document is not None, f"no {kind} ending in {name_suffix}"
|
||||
return document
|
||||
|
||||
|
||||
def _root_name(node: ast.expr) -> str | None:
|
||||
while isinstance(node, (ast.Subscript, ast.Attribute)):
|
||||
node = node.value
|
||||
return node.id if isinstance(node, ast.Name) else None
|
||||
|
||||
|
||||
def _module_level_mutations(tree: ast.Module) -> list[str]:
|
||||
"""Return every statement that writes into a module-level container from anywhere in the module."""
|
||||
module_names = {target.id for node in tree.body if isinstance(node, ast.Assign) for target in node.targets if isinstance(target, ast.Name)}
|
||||
module_names |= {node.target.id for node in tree.body if isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name)}
|
||||
findings: list[str] = []
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, (ast.Assign, ast.AugAssign, ast.AnnAssign)):
|
||||
targets = node.targets if isinstance(node, ast.Assign) else [node.target]
|
||||
for target in targets:
|
||||
if isinstance(target, ast.Subscript) and _root_name(target) in module_names:
|
||||
findings.append(f"line {node.lineno}: item assignment into {_root_name(target)}")
|
||||
elif isinstance(node, ast.Delete):
|
||||
for target in node.targets:
|
||||
if isinstance(target, ast.Subscript) and _root_name(target) in module_names:
|
||||
findings.append(f"line {node.lineno}: del on {_root_name(target)}")
|
||||
elif isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) and node.func.attr in MUTATING_METHODS:
|
||||
if isinstance(node.func.value, ast.Name) and node.func.value.id in module_names:
|
||||
findings.append(f"line {node.lineno}: {node.func.value.id}.{node.func.attr}()")
|
||||
return findings
|
||||
|
||||
|
||||
def test_default_values_keep_one_provisioner_replica_with_a_rollout_budget() -> None:
|
||||
provisioner = _values()["provisioner"]
|
||||
assert provisioner["enabled"] is True
|
||||
assert provisioner["replicas"] == 1, "the conservative default is unchanged; operators opt into more"
|
||||
assert provisioner["strategy"]["type"] == "RollingUpdate"
|
||||
assert provisioner["strategy"]["rollingUpdate"]["maxSurge"] == 1
|
||||
assert provisioner["strategy"]["rollingUpdate"]["maxUnavailable"] == 0, "surge-then-drain keeps one Pod answering sandbox calls through a rollout"
|
||||
assert provisioner["podDisruptionBudget"]["enabled"] is True
|
||||
assert provisioner["podDisruptionBudget"]["minAvailable"] == 1
|
||||
|
||||
|
||||
def test_deployment_template_takes_replicas_and_strategy_from_values() -> None:
|
||||
template = DEPLOYMENT_TEMPLATE.read_text(encoding="utf-8")
|
||||
assert "replicas: {{ .Values.provisioner.replicas }}" in template
|
||||
assert re.search(r"^\s*replicas:\s*\d+\s*$", template, flags=re.MULTILINE) is None, "a literal replica count ignores provisioner.replicas"
|
||||
assert re.search(r"\{\{- with \.Values\.provisioner\.strategy \}\}\n\s+strategy:\n\s+\{\{- toYaml \. \| nindent 4 \}\}\n\s+\{\{- end \}\}", template), "the rollout strategy must come from values like the gateway's"
|
||||
assert SELECTOR_BLOCK.search(template), "the Deployment selector is what the budget has to match"
|
||||
|
||||
|
||||
def test_pdb_template_is_guarded_on_enabled_and_more_than_one_replica() -> None:
|
||||
template = PDB_TEMPLATE.read_text(encoding="utf-8")
|
||||
guard = re.search(r"\{\{- if and \.Values\.provisioner\.enabled \$pdb\.enabled \(gt \(int \.Values\.provisioner\.replicas\) 1\) \}\}", template)
|
||||
assert guard is not None, "render only for an enabled provisioner with an enabled budget and more than one replica"
|
||||
assert "{{- $pdb := .Values.provisioner.podDisruptionBudget | default dict -}}" in template
|
||||
assert "apiVersion: policy/v1" in template
|
||||
assert "kind: PodDisruptionBudget" in template
|
||||
assert 'name: {{ include "deer-flow.fullname" . }}-provisioner' in template, "must not collide with the gateway budget"
|
||||
assert _min_available_include("provisioner") in template, "minAvailable goes through the shared helper so a percentage survives"
|
||||
assert template.count("app.kubernetes.io/component: provisioner") == 2, "once in metadata.labels, once in the selector"
|
||||
assert SELECTOR_BLOCK.search(template), "the selector must match the Deployment's Pod labels exactly"
|
||||
|
||||
|
||||
def test_provisioner_pdb_mirrors_the_gateway_pdb() -> None:
|
||||
"""Same shape as the gateway budget so the two stay reviewable side by side."""
|
||||
gateway = GATEWAY_PDB_TEMPLATE.read_text(encoding="utf-8").replace("gateway", "provisioner")
|
||||
provisioner = PDB_TEMPLATE.read_text(encoding="utf-8")
|
||||
for line in ("apiVersion: policy/v1", "kind: PodDisruptionBudget", 'name: {{ include "deer-flow.fullname" . }}-provisioner', _min_available_include("provisioner")):
|
||||
assert line in gateway and line in provisioner, line
|
||||
|
||||
|
||||
@pytest.mark.parametrize("component", sorted(PDB_TEMPLATES))
|
||||
def test_both_budgets_render_min_available_through_the_shared_helper(component: str) -> None:
|
||||
"""``| int`` turns "50%" into 0; Kubernetes accepts an integer or a percentage, so the chart must preserve both."""
|
||||
template = PDB_TEMPLATES[component].read_text(encoding="utf-8")
|
||||
assert _min_available_include(component) in template
|
||||
assert re.search(r"minAvailable:.*\|\s*int\b", template) is None, "int coerces a percentage string to 0"
|
||||
assert "default 1" not in template, "the fallback lives in the helper so both budgets share it"
|
||||
|
||||
|
||||
def _helper_block() -> str:
|
||||
helpers = HELPERS_TEMPLATE.read_text(encoding="utf-8")
|
||||
start = helpers.index('{{- define "deer-flow.pdbMinAvailable" -}}')
|
||||
end = helpers.find("{{- define ", start + 1)
|
||||
return helpers[start:] if end == -1 else helpers[start:end]
|
||||
|
||||
|
||||
def test_min_available_helper_preserves_percentages_and_fails_on_zero() -> None:
|
||||
helper = _helper_block()
|
||||
assert 'regexMatch "^[0-9]+%$"' in helper, "a percentage is preserved, not cast"
|
||||
assert 'regexMatch "^[0-9]+$"' in helper, "a digit string from --set-string becomes an integer"
|
||||
assert "quote" in helper, "the percentage must stay a YAML string"
|
||||
assert "fail" in helper, "zero and unsupported values refuse to render instead of yielding 0"
|
||||
assert 'kindIs "invalid"' in helper, "an unset value still falls back to 1"
|
||||
assert re.search(r"\|\s*int\s*-?\}\}", helper) is None, "no branch may pipe the raw value through int"
|
||||
|
||||
|
||||
def test_min_available_helper_normalizes_named_numbers_before_string_functions() -> None:
|
||||
"""Helm 3.18.0 loads YAML numbers as ``json.Number``: its kind is "string", so a ``kindIs "string"``
|
||||
gate lets it through, but ``regexMatch`` and the other string-typed functions reject it at argument
|
||||
validation -- and the unoverridden default ``minAvailable: 1`` then fails every multi-replica render."""
|
||||
helper = _helper_block()
|
||||
assert "$text := toString $raw" in helper, "whatever is not a plain Go number is normalized into a plain string first"
|
||||
assert 'kindIs "string"' not in helper, "json.Number passes a kind gate without being a plain string"
|
||||
string_calls = [line for line in helper.splitlines() if re.search(r"\b(regexMatch|atoi|trimSuffix|quote)\b", line)]
|
||||
assert string_calls, "the digit and percentage branches must still exist"
|
||||
for line in string_calls:
|
||||
assert "$raw" not in line, f"string-typed function applied to the raw value: {line.strip()}"
|
||||
assert "$text" in line, f"string-typed function must take the normalized value: {line.strip()}"
|
||||
assert re.search(r"printf .*\(got %v\).*\$raw", helper), "the hint still shows the raw value the operator supplied"
|
||||
|
||||
|
||||
def test_operator_notes_document_the_provisioner_replicas() -> None:
|
||||
values = VALUES.read_text(encoding="utf-8")
|
||||
readme = README.read_text(encoding="utf-8")
|
||||
provisioner_readme = PROVISIONER_README.read_text(encoding="utf-8")
|
||||
for needle in ("provisioner.replicas", "409", "PodDisruptionBudget"):
|
||||
assert needle in readme, needle
|
||||
assert "409" in values, "the comment explains why a concurrent create on another replica is harmless"
|
||||
assert "replica" in provisioner_readme, "the provisioner's own README must say it can run several replicas"
|
||||
assert "app=deer-flow-sandbox" in provisioner_readme or "labels" in provisioner_readme, "and that the labelled K8s objects are the only registry"
|
||||
assert "that one provisioner" not in readme, "the health note must not promise a single provisioner Pod"
|
||||
|
||||
|
||||
def test_provisioner_keeps_no_state_between_requests() -> None:
|
||||
"""The replica claim in values.yaml rests on this; a registry or reaper added here must re-open it."""
|
||||
source = PROVISIONER_APP.read_text(encoding="utf-8")
|
||||
tree = ast.parse(source, filename=str(PROVISIONER_APP))
|
||||
globals_declared = {name for node in ast.walk(tree) if isinstance(node, ast.Global) for name in node.names}
|
||||
assert globals_declared == {"core_v1"}, f"only the K8s client is process-global, found {sorted(globals_declared)}"
|
||||
assert _module_level_mutations(tree) == [], "a module-level container written at request time is per-Pod state"
|
||||
background = sorted({node.attr for node in ast.walk(tree) if isinstance(node, ast.Attribute) and node.attr in BACKGROUND_WORK})
|
||||
assert background == [], f"a background loop would run once per replica: {background}"
|
||||
assert source.count("exc.status != 409") >= 2, "Pod and Service create must both tolerate AlreadyExists from a concurrent creator"
|
||||
assert 'label_selector="app=deer-flow-sandbox"' in source, "the label is the registry every replica lists from"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("replicas", [1, 2, 3])
|
||||
def test_rendered_provisioner_follows_the_replica_count(replicas: int) -> None:
|
||||
values = _values()["provisioner"]
|
||||
documents = _render_chart(f"provisioner.replicas={replicas}")
|
||||
deployment = _by_kind(documents, "Deployment", "-provisioner")
|
||||
assert deployment["spec"]["replicas"] == replicas
|
||||
assert deployment["spec"]["strategy"] == values["strategy"]
|
||||
|
||||
pdb = _find(documents, "PodDisruptionBudget", "-provisioner")
|
||||
assert (pdb is not None) is (replicas > 1), "a budget of 1 on a single replica would block every node drain"
|
||||
assert _find(documents, "PodDisruptionBudget", "-gateway") is None, "the gateway stays at one replica in this render"
|
||||
if pdb is not None:
|
||||
# Also a Helm 3.18.0 json.Number regression guard for the unoverridden YAML default; the
|
||||
# explicit gateway + provisioner case is test_rendered_budget_keeps_the_unoverridden_default.
|
||||
assert pdb["spec"]["minAvailable"] == values["podDisruptionBudget"]["minAvailable"]
|
||||
assert pdb["spec"]["selector"]["matchLabels"] == deployment["spec"]["selector"]["matchLabels"]
|
||||
assert pdb["spec"]["selector"]["matchLabels"] == deployment["spec"]["template"]["metadata"]["labels"]
|
||||
assert pdb["metadata"]["labels"]["app.kubernetes.io/component"] == "provisioner"
|
||||
assert pdb["metadata"]["namespace"] == deployment["metadata"]["namespace"]
|
||||
|
||||
|
||||
def test_rendered_default_matches_the_previous_single_replica_deployment() -> None:
|
||||
documents = _render_chart()
|
||||
deployment = _by_kind(documents, "Deployment", "-provisioner")
|
||||
assert deployment["spec"]["replicas"] == 1
|
||||
assert deployment["spec"]["strategy"] == {"type": "RollingUpdate", "rollingUpdate": {"maxSurge": 1, "maxUnavailable": 0}}
|
||||
assert not any(document.get("kind") == "PodDisruptionBudget" for document in documents)
|
||||
|
||||
|
||||
def test_gateway_and_provisioner_budgets_render_side_by_side() -> None:
|
||||
documents = _render_chart("gateway.replicas=2", "provisioner.replicas=2")
|
||||
names = sorted(document["metadata"]["name"] for document in documents if document.get("kind") == "PodDisruptionBudget")
|
||||
assert len(names) == 2 and names[0].endswith("-gateway") and names[1].endswith("-provisioner")
|
||||
|
||||
|
||||
def test_disabled_provisioner_renders_neither_deployment_nor_budget() -> None:
|
||||
documents = _render_chart("provisioner.enabled=false", "provisioner.replicas=2")
|
||||
assert _find(documents, "Deployment", "-provisioner") is None
|
||||
assert _find(documents, "PodDisruptionBudget", "-provisioner") is None
|
||||
|
||||
|
||||
def test_provisioner_budget_can_be_disabled_independently() -> None:
|
||||
documents = _render_chart("provisioner.replicas=2", "provisioner.podDisruptionBudget.enabled=false")
|
||||
assert _by_kind(documents, "Deployment", "-provisioner")["spec"]["replicas"] == 2
|
||||
assert _find(documents, "PodDisruptionBudget", "-provisioner") is None
|
||||
|
||||
|
||||
@pytest.mark.parametrize("component", sorted(PDB_TEMPLATES))
|
||||
def test_rendered_budget_keeps_the_unoverridden_default(component: str) -> None:
|
||||
"""Helm 3.18.0 json.Number regression guard: with only the replica count set, the chart's own
|
||||
``minAvailable: 1`` must render as the integer 1 -- that release loads YAML numbers as a named
|
||||
string type that string-typed template functions reject unless the helper normalizes it first."""
|
||||
documents = _render("--set", f"{component}.replicas=2")
|
||||
assert _by_kind(documents, "PodDisruptionBudget", f"-{component}")["spec"]["minAvailable"] == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize("component", sorted(PDB_TEMPLATES))
|
||||
def test_rendered_budget_preserves_a_percentage(component: str) -> None:
|
||||
"""With three replicas "50%" must keep two Pods; the old ``int`` coercion rendered 0 and protected none."""
|
||||
documents = _render("--set", f"{component}.replicas=3", "--set-string", f"{component}.podDisruptionBudget.minAvailable=50%")
|
||||
assert _by_kind(documents, "PodDisruptionBudget", f"-{component}")["spec"]["minAvailable"] == "50%"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("component", sorted(PDB_TEMPLATES))
|
||||
@pytest.mark.parametrize("flag", ["--set", "--set-string"])
|
||||
def test_rendered_budget_keeps_an_integer_whichever_way_it_is_set(component: str, flag: str) -> None:
|
||||
documents = _render("--set", f"{component}.replicas=3", flag, f"{component}.podDisruptionBudget.minAvailable=2")
|
||||
assert _by_kind(documents, "PodDisruptionBudget", f"-{component}")["spec"]["minAvailable"] == 2
|
||||
|
||||
|
||||
@pytest.mark.parametrize("component", sorted(PDB_TEMPLATES))
|
||||
@pytest.mark.parametrize(("flag", "value"), [("--set-string", "abc"), ("--set", "0"), ("--set-string", "0%"), ("--set-string", "150%")])
|
||||
def test_rendered_budget_refuses_zero_and_unsupported_values(component: str, flag: str, value: str) -> None:
|
||||
key = f"{component}.podDisruptionBudget.minAvailable"
|
||||
stderr = _render_failure("--set", f"{component}.replicas=3", flag, f"{key}={value}")
|
||||
assert key in stderr, "the message must name the values key the operator has to fix"
|
||||
@ -242,7 +242,8 @@ curl http://localhost:2026/health/ready # readiness: database, checkpointer,
|
||||
Postgres or the Redis stream bridge is unreachable (`stream_bridge: unreachable`),
|
||||
so those pods leave the Service instead of accepting runs they cannot stream.
|
||||
The `provisioner` field reports the provisioner's own `/health` but never
|
||||
changes the status code: every gateway pod shares that one provisioner.
|
||||
changes the status code: every gateway pod reaches the same `provisioner`
|
||||
Service, whichever replica answers.
|
||||
|
||||
Hit the Ingress host (map it in `/etc/hosts` for local clusters) to load the UI.
|
||||
|
||||
@ -318,9 +319,33 @@ kubectl -n deer-flow exec deploy/deer-flow-provisioner -- curl -s localhost:8002
|
||||
override `config:` — a multi-instance gateway that leaves the index under
|
||||
the memory root logs a warning at startup. The index is rebuilt from the
|
||||
Markdown facts on every Pod start, so losing the emptyDir loses nothing.
|
||||
A `PodDisruptionBudget` (`minAvailable: 1`) is rendered automatically for a
|
||||
multi-instance gateway (same rule), and the rollout strategy is
|
||||
surge-then-drain (`maxSurge: 1`, `maxUnavailable: 0`).
|
||||
A `PodDisruptionBudget` (`minAvailable: 1`; an integer or a percentage
|
||||
string such as `"50%"`, while `0` fails the render -- disable the budget
|
||||
instead) is rendered automatically for a multi-instance gateway (same rule),
|
||||
and the rollout strategy is surge-then-drain (`maxSurge: 1`,
|
||||
`maxUnavailable: 0`). The sandbox
|
||||
provisioner is not on the single-instance list: it scales independently
|
||||
(next item).
|
||||
- **Provisioner replicas.** `provisioner.replicas` (default 1) scales the
|
||||
sandbox provisioner. It keeps no state of its own between requests: the
|
||||
sandbox Pods and Services it creates carry the labels
|
||||
(`app=deer-flow-sandbox`, `sandbox-id`) that are its only registry; create,
|
||||
discover, list and destroy all read them back from the API server; create
|
||||
tolerates the `409 AlreadyExists` a concurrent creator on another replica
|
||||
produces (several gateway Pods already run that race against one
|
||||
provisioner) and destroy tolerates `404`; and NodePorts are allocated by the
|
||||
API server, not the provisioner. The `provisioner` Service therefore spreads
|
||||
the gateway's calls over any number of replicas without affinity. At 1
|
||||
replica it is the last single point of failure in a multi-replica gateway
|
||||
deployment: while its Pod restarts or its node drains no sandbox can be
|
||||
created or discovered (running sandboxes are unaffected; the gateway talks
|
||||
to them directly). A `PodDisruptionBudget` (`provisioner.podDisruptionBudget`,
|
||||
`minAvailable: 1`) is rendered only while `provisioner.replicas > 1` -- on a
|
||||
single replica it would block every node drain -- and the rollout strategy
|
||||
is surge-then-drain (`maxSurge: 1`, `maxUnavailable: 0`), both mirroring
|
||||
the gateway. There is no `multiInstance` switch for the provisioner because
|
||||
nothing else in the chart renders differently per replica count; scaling it
|
||||
with `kubectl scale` only leaves it without the budget.
|
||||
- **App secret.** `<release>-app` holds `BETTER_AUTH_SECRET`,
|
||||
`DEER_FLOW_INTERNAL_AUTH_TOKEN`, `AUTH_JWT_SECRET` (the session-cookie
|
||||
signing key) and `PROVISIONER_API_KEY` (the key the gateway presents to the
|
||||
|
||||
@ -93,6 +93,40 @@ imagePullSecrets:
|
||||
{{- end -}}
|
||||
{{- end -}}
|
||||
|
||||
{{/* PodDisruptionBudget minAvailable as Kubernetes accepts it: a positive
|
||||
integer or a percentage string such as "50%". Takes a dict with `value`
|
||||
(the raw values entry; unset falls back to 1) and `key` (the values path,
|
||||
named in the error). `int` on "50%" is 0 -- a budget that protects
|
||||
nothing -- so a YAML number renders as an integer, a digit string from
|
||||
--set-string becomes that integer, a percentage is preserved quoted, and
|
||||
0, "0%", more than 100% or anything else fails the render. Anything that
|
||||
is not a plain Go number is normalized with toString before the string
|
||||
branches: Helm 3.18.0 loads YAML numbers as json.Number, whose kind is
|
||||
"string" but which regexMatch and friends reject at argument validation,
|
||||
so the unoverridden default would otherwise fail every multi-replica
|
||||
render on that release. */}}
|
||||
{{- define "deer-flow.pdbMinAvailable" -}}
|
||||
{{- $raw := .value -}}
|
||||
{{- if kindIs "invalid" $raw -}}{{- $raw = 1 -}}{{- end -}}
|
||||
{{- $hint := printf "%s must be a positive integer or a percentage such as \"50%%\" (got %v); a budget of 0 protects nothing, so set %s to false instead" .key $raw (replace ".minAvailable" ".enabled" .key) -}}
|
||||
{{- if or (kindIs "float64" $raw) (kindIs "int64" $raw) (kindIs "int" $raw) -}}
|
||||
{{- if or (lt (int $raw) 1) (ne (float64 (int $raw)) (float64 $raw)) -}}{{- fail $hint -}}{{- end -}}
|
||||
{{- int $raw -}}
|
||||
{{- else -}}
|
||||
{{- $text := toString $raw -}}
|
||||
{{- if regexMatch "^[0-9]+$" $text -}}
|
||||
{{- if lt (atoi $text) 1 -}}{{- fail $hint -}}{{- end -}}
|
||||
{{- atoi $text -}}
|
||||
{{- else if regexMatch "^[0-9]+%$" $text -}}
|
||||
{{- $percent := atoi (trimSuffix "%" $text) -}}
|
||||
{{- if or (lt $percent 1) (gt $percent 100) -}}{{- fail $hint -}}{{- end -}}
|
||||
{{- $text | quote -}}
|
||||
{{- else -}}
|
||||
{{- fail $hint -}}
|
||||
{{- end -}}
|
||||
{{- end -}}
|
||||
{{- end -}}
|
||||
|
||||
{{/* Name of the postgres StatefulSet/Service. */}}
|
||||
{{- define "deer-flow.postgresFullname" -}}
|
||||
{{- printf "%s-postgres" (include "deer-flow.fullname" .) -}}
|
||||
|
||||
@ -16,7 +16,7 @@ metadata:
|
||||
{{- include "deer-flow.labels" . | nindent 4 }}
|
||||
app.kubernetes.io/component: gateway
|
||||
spec:
|
||||
minAvailable: {{ $pdb.minAvailable | default 1 | int }}
|
||||
minAvailable: {{ include "deer-flow.pdbMinAvailable" (dict "value" $pdb.minAvailable "key" "gateway.podDisruptionBudget.minAvailable") }}
|
||||
selector:
|
||||
matchLabels:
|
||||
{{- include "deer-flow.selectorLabels" . | nindent 6 }}
|
||||
|
||||
@ -8,7 +8,11 @@ metadata:
|
||||
{{- include "deer-flow.labels" . | nindent 4 }}
|
||||
app.kubernetes.io/component: provisioner
|
||||
spec:
|
||||
replicas: 1
|
||||
replicas: {{ .Values.provisioner.replicas }}
|
||||
{{- with .Values.provisioner.strategy }}
|
||||
strategy:
|
||||
{{- toYaml . | nindent 4 }}
|
||||
{{- end }}
|
||||
selector:
|
||||
matchLabels:
|
||||
{{- include "deer-flow.selectorLabels" . | nindent 6 }}
|
||||
|
||||
26
deploy/helm/deer-flow/templates/provisioner-pdb.yaml
Normal file
26
deploy/helm/deer-flow/templates/provisioner-pdb.yaml
Normal file
@ -0,0 +1,26 @@
|
||||
{{- /*
|
||||
PodDisruptionBudget for the sandbox provisioner. Rendered only while the
|
||||
provisioner is enabled and runs more than one replica: with a single
|
||||
replica a budget of 1 would block every node drain, and with several it
|
||||
keeps at least one Pod answering the gateway's sandbox create/discover
|
||||
calls while nodes are drained. Unlike the gateway there is no multiInstance
|
||||
override, because nothing else in the chart renders differently per
|
||||
provisioner replica count; scaling with kubectl alone leaves it unbudgeted.
|
||||
*/ -}}
|
||||
{{- $pdb := .Values.provisioner.podDisruptionBudget | default dict -}}
|
||||
{{- if and .Values.provisioner.enabled $pdb.enabled (gt (int .Values.provisioner.replicas) 1) }}
|
||||
apiVersion: policy/v1
|
||||
kind: PodDisruptionBudget
|
||||
metadata:
|
||||
name: {{ include "deer-flow.fullname" . }}-provisioner
|
||||
namespace: {{ include "deer-flow.namespace" . }}
|
||||
labels:
|
||||
{{- include "deer-flow.labels" . | nindent 4 }}
|
||||
app.kubernetes.io/component: provisioner
|
||||
spec:
|
||||
minAvailable: {{ include "deer-flow.pdbMinAvailable" (dict "value" $pdb.minAvailable "key" "provisioner.podDisruptionBudget.minAvailable") }}
|
||||
selector:
|
||||
matchLabels:
|
||||
{{- include "deer-flow.selectorLabels" . | nindent 6 }}
|
||||
app.kubernetes.io/component: provisioner
|
||||
{{- end }}
|
||||
@ -65,8 +65,10 @@ gateway:
|
||||
maxUnavailable: 0
|
||||
# PodDisruptionBudget, rendered only for a multi-instance gateway (replicas
|
||||
# > 1 or multiInstance: true; a budget of 1 on a single replica would block
|
||||
# node drains). minAvailable falls back to 1 when unset; disable the budget
|
||||
# rather than setting it to 0.
|
||||
# node drains). minAvailable is an integer or a percentage string such as
|
||||
# "50%" (quoted; a plain `int` cast would turn it into 0) and falls back to
|
||||
# 1 when unset. 0 and "0%" fail the render -- a zero budget protects
|
||||
# nothing -- so disable the budget instead.
|
||||
podDisruptionBudget:
|
||||
enabled: true
|
||||
minAvailable: 1
|
||||
@ -146,6 +148,36 @@ provisioner:
|
||||
# `existingSecret` when you supply one (external provisioner); otherwise
|
||||
# its start command defaults the variable to "", so the default `config`
|
||||
# below still loads even when a user-managed Secret lacks the key.
|
||||
# Replica count. 1 is the conservative default, matching the gateway; >1 is
|
||||
# safe as the provisioner stands: it keeps no state of its own between
|
||||
# requests -- the sandbox Pods and Services it labels (`app=deer-flow-sandbox`,
|
||||
# `sandbox-id`) are the only registry, every handler reads them back from
|
||||
# the API server, and create tolerates the 409 AlreadyExists a concurrent
|
||||
# creator on another replica produces (the race several gateway Pods already
|
||||
# run against one provisioner). NodePorts are allocated by the API server,
|
||||
# never by the provisioner. The `provisioner` Service spreads requests over
|
||||
# every replica, so with 1 replica this is the last single point of failure
|
||||
# in a multi-replica gateway deployment: while its Pod restarts or its node
|
||||
# drains no sandbox can be created or discovered. Scaling it changes nothing
|
||||
# else in the chart, so there is no multiInstance switch here.
|
||||
replicas: 1
|
||||
# Rollout strategy. Surge-then-drain keeps one Pod answering sandbox calls
|
||||
# while a peer starts; a replica has nothing to hand over beyond its
|
||||
# in-flight requests, so the default termination grace period is enough.
|
||||
strategy:
|
||||
type: RollingUpdate
|
||||
rollingUpdate:
|
||||
maxSurge: 1
|
||||
maxUnavailable: 0
|
||||
# PodDisruptionBudget, rendered only for replicas > 1 (a budget of 1 on a
|
||||
# single replica would block node drains). minAvailable is an integer or a
|
||||
# percentage string such as "50%" and falls back to 1 when unset; 0 and
|
||||
# "0%" fail the render -- a zero budget protects nothing -- so disable the
|
||||
# budget instead. Scaling with `kubectl scale` instead of `replicas` leaves
|
||||
# the budget unrendered.
|
||||
podDisruptionBudget:
|
||||
enabled: true
|
||||
minAvailable: 1
|
||||
resources:
|
||||
requests:
|
||||
cpu: 50m
|
||||
|
||||
@ -398,6 +398,37 @@ docker exec deer-flow-gateway curl -s $SANDBOX_URL/v1/sandbox
|
||||
- In ClusterIP mode, test from the backend Pod: `curl http://sandbox-XXX-svc.deer-flow.svc.cluster.local:8080/v1/sandbox`
|
||||
- Check `NODE_HOST` for NodePort deployments, or cluster DNS / NetworkPolicy / service mesh rules for ClusterIP deployments
|
||||
|
||||
## Running several replicas
|
||||
|
||||
The provisioner keeps no state of its own between requests, so several
|
||||
replicas can serve the same backend behind one load-balanced Service (the Helm
|
||||
chart's `provisioner.replicas`; compose runs a single container):
|
||||
|
||||
- The sandbox Pods and Services it creates carry the labels it later queries
|
||||
(`app=deer-flow-sandbox`, `sandbox-id`). They are the only registry: create,
|
||||
status, list and delete all read the Kubernetes API, never a process-local
|
||||
table, so any replica can answer for a sandbox another replica created.
|
||||
- `POST /api/sandboxes` is idempotent across replicas. Two creates for the same
|
||||
`sandbox_id` that race on different replicas both succeed: the loser's Pod
|
||||
and Service creates return `409 AlreadyExists`, which the handler treats like
|
||||
its own success before reading the winner's objects back (including
|
||||
`MAX_SHELL_SESSIONS`, so a lower-capacity winner is still reported to the
|
||||
Gateway as `409`). Several Gateway Pods already run this race against a
|
||||
single provisioner.
|
||||
- `DELETE /api/sandboxes/{sandbox_id}` tolerates `404`, so two replicas
|
||||
deleting the same sandbox both return success.
|
||||
- NodePorts are allocated by the API server when the Service is created; the
|
||||
provisioner only reads the assigned port back.
|
||||
- No replica runs a background loop (there is no reaper or cache), and
|
||||
`/api/capabilities` is derived from environment variables every replica of
|
||||
one Deployment shares.
|
||||
|
||||
The one startup write is `_ensure_namespace()`: when `K8S_NAMESPACE` does not
|
||||
exist yet, two replicas starting together can both see `404` and race to create
|
||||
it; the loser's `409` is not caught, so that process exits and is restarted into
|
||||
the now-existing namespace. Under the Helm chart the provisioner runs inside the
|
||||
namespace it manages, so this path never runs there.
|
||||
|
||||
## Security Considerations
|
||||
|
||||
1. **HostPath Volumes**: The provisioner mounts host directories into sandbox Pods by default. Ensure these paths contain only trusted data. For production, prefer PVC-based volumes (set `SKILLS_PVC_NAME` and `USERDATA_PVC_NAME`) to avoid node-specific data loss risks.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user