已合并
[bugfix] deployer NodePort 冲突检测：CRD 重命名 Service 误判导致扩缩容失败 #776
杨安创建于 11 天前
[bugfix] deployer NodePort 冲突检测：CRD 重命名 Service 误判导致扩缩容失败 #776
已合并
共 3 个文件变更+61-2
| @@ -180,6 +180,8 @@ Node selector 字段均为 JSON 对象。自定义标签会与 deployer 根据 ` | |||
| 180 | 180 | ||
| 181 | 非 TTY（脚本/CI）场景按 `N` 处理。建议 NodePort 范围：`30000-32767`。 | 181 | 非 TTY（脚本/CI）场景按 `N` 处理。建议 NodePort 范围：`30000-32767`。 |
| 182 | 182 | ||
| 183 | +同一 namespace 下由本服务自己占用的端口不算冲突，因此重复部署与 `--update_instance_num` 扩缩容不会被误判。InferServiceSet 模式下 CRD 会把 Service 重命名为 `{service_name}-{InferServiceSet 名}-{索引}-{role}`，该命名同样识别为本服务自有端口。 | ||
| 184 | + | ||
| 183 | 说明：交互 remap 只修改本次部署使用的 `output_yamls`，**不会**自动回写 `user_config.json`。若需要把新端口持久化到配置里，请手动同步修改 `motor_deploy_config` 中对应的 `*_node_port` 字段。 | 185 | 说明：交互 remap 只修改本次部署使用的 `output_yamls`，**不会**自动回写 `user_config.json`。若需要把新端口持久化到配置里，请手动同步修改 `motor_deploy_config` 中对应的 `*_node_port` 字段。 |
| 184 | 186 | ||
| 185 | ### env.json | 187 | ### env.json |
| @@ -20,6 +20,7 @@ from __future__ import annotations | |||
| 20 | 20 | ||
| 21 | import json | 21 | import json |
| 22 | import os | 22 | import os |
| 23 | +import re | ||
| 23 | import subprocess | 24 | import subprocess |
| 24 | import sys | 25 | import sys |
| 25 | from dataclasses import dataclass | 26 | from dataclasses import dataclass |
| @@ -48,6 +49,10 @@ class PlannedNodePort: | |||
| 48 | service_namespace: str | 49 | service_namespace: str |
| 49 | purpose: str | 50 | purpose: str |
| 50 | container_port: int | None | 51 | container_port: int | None |
| 52 | + # InferServiceSet context: the CRD renames Services, so the cluster name | ||
| 53 | + # differs from the name declared in yaml (see _is_self_owned). | ||
| 54 | + infer_set_name: str | None = None | ||
| 55 | + role_name: str | None = None | ||
| 51 | 56 | ||
| 52 | 57 | ||
| 53 | 58 | ||
| @@ -95,6 +100,8 @@ def _planned_from_service_spec( | |||
| 95 | svc_name: str, | 100 | svc_name: str, |
| 96 | svc_ns: str, | 101 | svc_ns: str, |
| 97 | svc_spec: dict, | 102 | svc_spec: dict, |
| 103 | + infer_set_name: str | None = None, | ||
| 104 | + role_name: str | None = None, | ||
| 98 | ) -> list[PlannedNodePort]: | 105 | ) -> list[PlannedNodePort]: |
| 99 | planned: list[PlannedNodePort] = [] | 106 | planned: list[PlannedNodePort] = [] |
| 100 | for port_entry in (svc_spec or {}).get("ports") or []: | 107 | for port_entry in (svc_spec or {}).get("ports") or []: |
| @@ -126,6 +133,8 @@ def _planned_from_service_spec( | |||
| 126 | service_namespace=svc_ns, | 133 | service_namespace=svc_ns, |
| 127 | purpose=_purpose_for(svc_name, container_port_i), | 134 | purpose=_purpose_for(svc_name, container_port_i), |
| 128 | container_port=container_port_i, | 135 | container_port=container_port_i, |
| 136 | + infer_set_name=infer_set_name, | ||
| 137 | + role_name=role_name, | ||
| 129 | ) | 138 | ) |
| 130 | ) | 139 | ) |
| 131 | return planned | 140 | return planned |
| @@ -145,6 +154,7 @@ def _collect_from_infer_service_set(doc: dict, yaml_path: str) -> list[PlannedNo | |||
| 145 | """Collect nodePorts nested under InferServiceSet roles[].services[].""" | 154 | """Collect nodePorts nested under InferServiceSet roles[].services[].""" |
| 146 | meta = doc.get("metadata") or {} | 155 | meta = doc.get("metadata") or {} |
| 147 | svc_ns = meta.get("namespace") or "" | 156 | svc_ns = meta.get("namespace") or "" |
| 157 | + infer_set_name = meta.get("name") or "" | ||
| 148 | planned: list[PlannedNodePort] = [] | 158 | planned: list[PlannedNodePort] = [] |
| 149 | template = (doc.get("spec") or {}).get("template") or {} | 159 | template = (doc.get("spec") or {}).get("template") or {} |
| 150 | if not isinstance(template, dict): | 160 | if not isinstance(template, dict): |
| @@ -182,6 +192,8 @@ def _collect_from_infer_service_set(doc: dict, yaml_path: str) -> list[PlannedNo | |||
| 182 | service_namespace=svc_ns, | 192 | service_namespace=svc_ns, |
| 183 | purpose="Controller observability", | 193 | purpose="Controller observability", |
| 184 | container_port=container_port_i, | 194 | container_port=container_port_i, |
| 195 | + infer_set_name=infer_set_name, | ||
| 196 | + role_name=role_name, | ||
| 185 | ) | 197 | ) |
| 186 | ) | 198 | ) |
| 187 | continue | 199 | continue |
| @@ -191,6 +203,8 @@ def _collect_from_infer_service_set(doc: dict, yaml_path: str) -> list[PlannedNo | |||
| 191 | svc_name=svc_name, | 203 | svc_name=svc_name, |
| 192 | svc_ns=svc_ns, | 204 | svc_ns=svc_ns, |
| 193 | svc_spec=svc.get("spec") or {}, | 205 | svc_spec=svc.get("spec") or {}, |
| 206 | + infer_set_name=infer_set_name, | ||
| 207 | + role_name=role_name, | ||
| 194 | ) | 208 | ) |
| 195 | ) | 209 | ) |
| 196 | return planned | 210 | return planned |
| @@ -301,10 +315,26 @@ def collect_cluster_nodeports() -> dict[int, ClusterNodePort]: | |||
| 301 | return used | 315 | return used |
| 302 | 316 | ||
| 303 | 317 | ||
| 318 | +def _matches_crd_service_name(planned: PlannedNodePort, owner_name: str) -> bool: | ||
| 319 | + """InferServiceSet creates Services as {svc}-{inferSetName}-{index}-{role}.""" | ||
| 320 | + if not planned.infer_set_name or not planned.role_name: | ||
| 321 | + return False | ||
| 322 | + pattern = ( | ||
| 323 | + rf"^{re.escape(planned.service_name)}" | ||
| 324 | + rf"-{re.escape(planned.infer_set_name)}" | ||
| 325 | + rf"-\d+-{re.escape(planned.role_name)}$" | ||
| 326 | + ) | ||
| 327 | + return re.match(pattern, owner_name, re.IGNORECASE) is not None | ||
| 328 | + | ||
| 329 | + | ||
| 304 | def _is_self_owned(planned: PlannedNodePort, owner: ClusterNodePort, job_id: str) -> bool: | 330 | def _is_self_owned(planned: PlannedNodePort, owner: ClusterNodePort, job_id: str) -> bool: |
| 305 | - """Treat same job namespace + same service name as our own (re-deploy).""" | 331 | + """Treat same job namespace + same service as our own (re-deploy / scaling).""" |
| 306 | planned_ns = planned.service_namespace or job_id | 332 | planned_ns = planned.service_namespace or job_id |
| 307 | - return owner.namespace == planned_ns and owner.service_name == planned.service_name | 333 | + if owner.namespace != planned_ns: |
| 334 | + return False | ||
| 335 | + if owner.service_name == planned.service_name: | ||
| 336 | + return True | ||
| 337 | + return _matches_crd_service_name(planned, owner.service_name) | ||
| 308 | 338 | ||
| 309 | 339 | ||
| 310 | def find_conflicts( | 340 | def find_conflicts( |
| @@ -356,6 +356,33 @@ def test_resolve_dry_run_does_not_clear_existing_warning_files(tmp_path: Path, m | |||
| 356 | assert warn_path.read_text(encoding="utf-8") == "keep-me\n" | 356 | assert warn_path.read_text(encoding="utf-8") == "keep-me\n" |
| 357 | 357 | ||
| 358 | 358 | ||
| 359 | +def test_resolve_scaling_keeps_ports_owned_by_own_crd_services(tmp_path: Path): | ||
| 360 | + """Scaling an existing InferServiceSet deploy: CRD-renamed Services are ours, not conflicts.""" | ||
| 361 | + yaml_path = tmp_path / "infer.yaml" | ||
| 362 | + _write_infer_service_set_yaml(yaml_path, "mindie-b") | ||
| 363 | + cluster = { | ||
| 364 | + 31015: ClusterNodePort(31015, "mindie-b", "mindie-motor-coordinator-infer-vllm-0-coordinator"), | ||
| 365 | + 31017: ClusterNodePort(31017, "mindie-b", "mindie-motor-coordinator-obs-vllm-0-coordinator"), | ||
| 366 | + # Non-zero index: InferServiceSet replicas > 1 must still be recognised as ours. | ||
| 367 | + 31027: ClusterNodePort(31027, "mindie-b", "mindie-motor-observability-vllm-1-controller"), | ||
| 368 | + } | ||
| 369 | + | ||
| 370 | + with ( | ||
| 371 | + patch("lib.nodeport_allocator.collect_cluster_nodeports", return_value=cluster), | ||
| 372 | + patch("lib.nodeport_allocator.sys.stdin") as stdin, | ||
| 373 | + patch("builtins.input", return_value="N") as prompt, | ||
| 374 | + ): | ||
| 375 | + stdin.isatty.return_value = True | ||
| 376 | + remapping = resolve_and_rewrite_nodeports([str(yaml_path)], "mindie-b") | ||
| 377 | + | ||
| 378 | + assert remapping == {} | ||
| 379 | + prompt.assert_not_called() | ||
| 380 | + text = yaml_path.read_text(encoding="utf-8") | ||
| 381 | + assert "nodePort: 31015" in text | ||
| 382 | + assert "nodePort: 31017" in text | ||
| 383 | + assert "nodePort: 31027" in text | ||
| 384 | + | ||
| 385 | + | ||
| 359 | def test_is_observability_service_name_avoids_robust_false_positive(): | 386 | def test_is_observability_service_name_avoids_robust_false_positive(): |
| 360 | assert is_observability_service_name("mindie-motor-coordinator-obs") | 387 | assert is_observability_service_name("mindie-motor-coordinator-obs") |
| 361 | assert is_observability_service_name("mindie-motor-observability") | 388 | assert is_observability_service_name("mindie-motor-observability") |