toki
b4c6550eab
- Add node store implementation for edge app - Add adapters factory for node app - Update edge and node transport layers - Update domain rules for edge and node - Add bin scripts for edge and node - Update configs and documentation - Add agent-task node_centralized_mgmt directory
264 lines
11 KiB
Text
264 lines
11 KiB
Text
<!-- task=node_centralized_mgmt plan=0 tag=REFACTOR -->
|
|
|
|
# Code Review Reference - REFACTOR
|
|
|
|
## 개요
|
|
|
|
date=2026-05-03
|
|
task=node_centralized_mgmt, plan=0, tag=REFACTOR
|
|
|
|
## 이 파일을 읽는 리뷰 에이전트에게
|
|
|
|
각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요.
|
|
리뷰 완료 후 반드시 아래 순서로 아카이브하세요.
|
|
|
|
1. `CODE_REVIEW.md` → `code_review_0.log` (N = 기존 code_review_*.log 수)
|
|
2. `PLAN.md` → `plan_0.log` (M = 기존 plan_*.log 수)
|
|
3. PASS인 경우 `complete.log` 작성 후 종료. WARN/FAIL인 경우 새 `PLAN.md` + `CODE_REVIEW.md` 스텁 작성.
|
|
|
|
---
|
|
|
|
## 구현 항목별 완료 여부
|
|
|
|
| 항목 | 완료 여부 |
|
|
|------|---------|
|
|
| [REFACTOR-1] proto: RegisterRequest / RegisterResponse / NodeConfigPayload 추가 | [x] |
|
|
| [REFACTOR-2] packages/config: NodeConfig 단순화 + EdgeConfig에 노드 정의 추가 | [x] |
|
|
| [REFACTOR-3] edge: NodeStore 신설 + NodeEntry 갱신 | [x] |
|
|
| [REFACTOR-4] edge: transport 핸드셰이크 변경 + bootstrap 재배선 | [x] |
|
|
| [REFACTOR-5] node: transport 핸드셰이크 변경 + DialEdge 시그니처 변경 | [x] |
|
|
| [REFACTOR-6] node: adapter factory + bootstrap 재구성 | [x] |
|
|
| [REFACTOR-7] 테스트 전체 갱신 | [x] |
|
|
|
|
## 계획 대비 변경 사항
|
|
|
|
- 실행 환경에 `make`가 없어 `make proto`는 실패했다. Makefile의 `proto` 타깃과 동일한 `protoc` 명령을 `PATH="/config/go/bin:$PATH"`로 직접 실행해 생성물을 갱신했다.
|
|
- edge 등록 처리에서 동일 node ID가 이미 registry에 있으면 `node already connected`로 거절하도록 했다. 중앙 관리 전환의 중복 접속 방지 목적에 맞춘 보강이다.
|
|
- node bootstrap에서 edge가 내려준 `workspace_root` 디렉터리를 생성하고, OnStop에서 SQLite store를 닫도록 했다. 런타임 시작 실패와 리소스 누수를 줄이기 위한 보강이다.
|
|
|
|
## 주요 설계 결정
|
|
|
|
- edge의 `NodeStore`는 config 기반 사전 등록 정보를 token/ID로 보관하고, runtime `Registry`는 현재 연결된 `NodeEntry{node_id, alias, client}`만 보관한다.
|
|
- node는 `DialEdge(edge_addr, token)`에서 `RegisterRequest`를 먼저 보내고, `RegisterResponse`가 accepted일 때만 `Session`과 중앙 설정 payload를 사용한다.
|
|
- `Session`은 registration 이후 생성되는 `node.Node`를 받을 수 있도록 mutex 보호 `SetHandler`를 제공한다.
|
|
- `adapters.BuildFromPayload`는 mock adapter를 항상 등록하고, edge payload에 포함된 enabled adapter만 추가 등록한다.
|
|
|
|
## 리뷰어를 위한 체크포인트
|
|
|
|
- `RegisterRequest`/`RegisterResponse`가 proto에 정확히 추가되었는지, `make proto` 후 컴파일 오류가 없는지 확인
|
|
- `CapabilityRequest`/`CapabilityResponse`가 proto에서 삭제되지 않고 유지되었는지 확인
|
|
- edge `onNodeConnected`에서 더 이상 `CapabilityRequest`를 전송하지 않는지 확인
|
|
- node `DialEdge`가 연결 직후 `RegisterRequest`를 전송하고 응답을 검증하는지 확인
|
|
- `Session.SetHandler`가 mutex로 보호되는지 확인
|
|
- `node.Node.OnCapabilityRequest`가 삭제되었는지, `Handler` 인터페이스에도 없는지 확인
|
|
- `adapters.BuildFromPayload`가 mock을 항상 등록하고 타입별 변환이 정확한지 확인
|
|
- `node.New`가 `*config.NodeConfig` 대신 `nodeID string`을 직접 받는지 확인
|
|
- `NodeStore.FindByToken`이 등록되지 않은 token에 대해 false를 반환하는지 확인
|
|
- `configs/node.yaml`에 edge_addr + token 외 항목이 없는지 확인
|
|
- 모든 테스트가 새 handshake 기반으로 갱신되었는지 확인
|
|
|
|
## 검증 결과
|
|
|
|
_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._
|
|
|
|
### REFACTOR-1 중간 검증
|
|
```
|
|
$ make proto
|
|
/usr/bin/bash: line 1: make: command not found
|
|
|
|
$ PATH="/config/go/bin:$PATH" protoc --go_out=. --go_opt=module=iop --proto_path=. proto/iop/runtime.proto proto/iop/node.proto proto/iop/control.proto proto/iop/job.proto
|
|
(no output)
|
|
|
|
$ go build ./...
|
|
(no output)
|
|
```
|
|
|
|
### REFACTOR-2 중간 검증
|
|
```
|
|
$ go build ./packages/config/...
|
|
(no output)
|
|
```
|
|
|
|
### REFACTOR-3 중간 검증
|
|
```
|
|
$ go test ./apps/edge/internal/node/...
|
|
ok iop/apps/edge/internal/node 0.003s
|
|
```
|
|
|
|
### REFACTOR-4 중간 검증
|
|
```
|
|
$ go build ./apps/edge/...
|
|
(no output)
|
|
```
|
|
|
|
### REFACTOR-5 중간 검증
|
|
```
|
|
$ go build ./apps/node/internal/transport/...
|
|
(no output)
|
|
```
|
|
|
|
### REFACTOR-6 중간 검증
|
|
```
|
|
$ go build ./apps/node/...
|
|
(no output)
|
|
```
|
|
|
|
### REFACTOR-7 중간 검증
|
|
```
|
|
$ go test ./apps/edge/internal/node/...
|
|
ok iop/apps/edge/internal/node 0.003s
|
|
|
|
$ go test ./apps/node/internal/node/...
|
|
ok iop/apps/node/internal/node 0.005s
|
|
|
|
$ go test ./apps/node/internal/adapters/...
|
|
ok iop/apps/node/internal/adapters 0.003s
|
|
? iop/apps/node/internal/adapters/cli [no test files]
|
|
? iop/apps/node/internal/adapters/mock [no test files]
|
|
? iop/apps/node/internal/adapters/ollama [no test files]
|
|
? iop/apps/node/internal/adapters/vllm [no test files]
|
|
|
|
$ go test ./apps/edge/internal/transport/...
|
|
ok iop/apps/edge/internal/transport 0.004s
|
|
|
|
$ go test ./apps/node/internal/transport/...
|
|
ok iop/apps/node/internal/transport 0.004s
|
|
```
|
|
|
|
### 최종 검증 (리뷰어 재실행)
|
|
```
|
|
$ go test -count=1 ./...
|
|
ok iop/apps/edge/internal/node 0.003s
|
|
ok iop/apps/edge/internal/transport 0.005s
|
|
ok iop/apps/node/internal/adapters 0.003s
|
|
ok iop/apps/node/internal/node 0.005s
|
|
ok iop/apps/node/internal/transport 0.005s
|
|
```
|
|
|
|
### 최종 검증
|
|
```
|
|
$ make proto && go build ./... && go test ./...
|
|
/usr/bin/bash: line 1: make: command not found
|
|
|
|
$ PATH="/config/go/bin:$PATH" protoc --go_out=. --go_opt=module=iop --proto_path=. proto/iop/runtime.proto proto/iop/node.proto proto/iop/control.proto proto/iop/job.proto
|
|
(no output)
|
|
|
|
$ go build ./...
|
|
(no output)
|
|
|
|
$ go test ./...
|
|
? iop/apps/control-plane/cmd/control-plane [no test files]
|
|
? iop/apps/edge/cmd/edge [no test files]
|
|
? iop/apps/edge/internal/bootstrap [no test files]
|
|
ok iop/apps/edge/internal/node (cached)
|
|
ok iop/apps/edge/internal/transport (cached)
|
|
? iop/apps/node/cmd/node [no test files]
|
|
ok iop/apps/node/internal/adapters (cached)
|
|
? iop/apps/node/internal/adapters/cli [no test files]
|
|
? iop/apps/node/internal/adapters/mock [no test files]
|
|
? iop/apps/node/internal/adapters/ollama [no test files]
|
|
? iop/apps/node/internal/adapters/vllm [no test files]
|
|
? iop/apps/node/internal/bootstrap [no test files]
|
|
ok iop/apps/node/internal/node (cached)
|
|
? iop/apps/node/internal/router [no test files]
|
|
? iop/apps/node/internal/runtime [no test files]
|
|
? iop/apps/node/internal/store [no test files]
|
|
ok iop/apps/node/internal/transport (cached)
|
|
? iop/apps/worker/cmd/worker [no test files]
|
|
? iop/packages/auth [no test files]
|
|
? iop/packages/config [no test files]
|
|
? iop/packages/jobs [no test files]
|
|
? iop/packages/metadata [no test files]
|
|
? iop/packages/observability [no test files]
|
|
? iop/packages/policy [no test files]
|
|
? iop/packages/version [no test files]
|
|
? iop/proto/gen/iop [no test files]
|
|
|
|
$ go test -count=1 ./...
|
|
? iop/apps/control-plane/cmd/control-plane [no test files]
|
|
? iop/apps/edge/cmd/edge [no test files]
|
|
? iop/apps/edge/internal/bootstrap [no test files]
|
|
ok iop/apps/edge/internal/node 0.003s
|
|
ok iop/apps/edge/internal/transport 0.005s
|
|
? iop/apps/node/cmd/node [no test files]
|
|
ok iop/apps/node/internal/adapters 0.003s
|
|
? iop/apps/node/internal/adapters/cli [no test files]
|
|
? iop/apps/node/internal/adapters/mock [no test files]
|
|
? iop/apps/node/internal/adapters/ollama [no test files]
|
|
? iop/apps/node/internal/adapters/vllm [no test files]
|
|
? iop/apps/node/internal/bootstrap [no test files]
|
|
ok iop/apps/node/internal/node 0.005s
|
|
? iop/apps/node/internal/router [no test files]
|
|
? iop/apps/node/internal/runtime [no test files]
|
|
? iop/apps/node/internal/store [no test files]
|
|
ok iop/apps/node/internal/transport 0.005s
|
|
? iop/apps/worker/cmd/worker [no test files]
|
|
? iop/packages/auth [no test files]
|
|
? iop/packages/config [no test files]
|
|
? iop/packages/jobs [no test files]
|
|
? iop/packages/metadata [no test files]
|
|
? iop/packages/observability [no test files]
|
|
? iop/packages/policy [no test files]
|
|
? iop/packages/version [no test files]
|
|
? iop/proto/gen/iop [no test files]
|
|
```
|
|
|
|
---
|
|
|
|
## 코드리뷰 결과
|
|
|
|
### 종합 판정: FAIL
|
|
|
|
### 차원별 평가
|
|
|
|
| 차원 | 판정 | 비고 |
|
|
|------|------|------|
|
|
| Correctness | Fail | CLI 어댑터 args/env `[]string` → structpb 오류 무시 |
|
|
| Completeness | Pass | 계획 7개 항목 전부 구현 |
|
|
| Test coverage | Warn | CLI 어댑터 config 직렬화 경로 미검증 |
|
|
| API contract | Pass | Handler 인터페이스, DialEdge 시그니처, 모든 호출부 일치 |
|
|
| Code quality | Pass | 불필요한 주석 없음, 잔여 TODO 없음 |
|
|
| Plan deviation | Pass | 계획 대비 변경은 모두 합리적 보강 |
|
|
| Verification trust | Pass | 검증 출력이 실제 코드와 일치 |
|
|
|
|
### 발견된 문제
|
|
|
|
- **Required** `apps/edge/internal/transport/server.go:160-161`
|
|
`buildConfigPayload`에서 CLI 프로필의 `Args`/`Env` 필드를 `[]string` 타입 그대로 `structpb.NewStruct`에 전달한다.
|
|
`structpb.NewValue`는 `[]interface{}`만 처리하고 `[]string`은 `proto: invalid type: []string` 오류를 반환한다.
|
|
`st, _ := structpb.NewStruct(settings)`로 오류를 무시하므로 `st`가 `nil`이 되어 CLI 어댑터 설정(command, args, env)이 전부 소실된다. 노드에서 CLI 어댑터가 빈 profile 맵으로 초기화되어 모든 실행 요청이 "unknown profile" 오류로 실패한다.
|
|
|
|
수정:
|
|
```go
|
|
// buildConfigPayload 내 CLI 처리 부분
|
|
toAnySlice := func(ss []string) []interface{} {
|
|
out := make([]interface{}, len(ss))
|
|
for i, s := range ss {
|
|
out[i] = s
|
|
}
|
|
return out
|
|
}
|
|
profiles[name] = map[string]any{
|
|
"command": p.Command,
|
|
"args": toAnySlice(p.Args),
|
|
"env": toAnySlice(p.Env),
|
|
}
|
|
```
|
|
아울러 `buildConfigPayload`가 오류를 반환할 수 있도록 시그니처를 `func buildConfigPayload(rec *edgenode.NodeRecord) (*iop.NodeConfigPayload, error)`로 변경하고, `onNodeConnected`에서 오류 발생 시 `RegisterResponse{Accepted: false, Reason: ...}`를 응답한다.
|
|
|
|
- **Suggested** `apps/edge/internal/node/store.go:LoadFromConfig`
|
|
중복 token은 검사하지만 중복 alias는 검사하지 않는다. Alias가 동일하면 `"node-" + alias`로 파생되는 ID가 충돌하고, `byID` 맵에서 후자가 전자를 덮어쓴다. 운영자가 실수로 동일 alias를 두 번 등록하면 한 노드가 silent하게 소실된다.
|
|
|
|
수정: `seen` 맵을 alias에도 적용한다.
|
|
```go
|
|
seenAlias := make(map[string]bool)
|
|
// ...
|
|
if seenAlias[d.Alias] {
|
|
return nil, fmt.Errorf("node[%d] alias=%q: duplicate alias", i, d.Alias)
|
|
}
|
|
seenAlias[d.Alias] = true
|
|
```
|
|
|
|
### 다음 단계
|
|
|
|
FAIL: Required 이슈 수정을 위한 새 PLAN.md를 작성한다.
|