feat(chaos): chaos_service CRUD, Service Discovery agent listing, and full load-test authoring parity (Locust/K6/JMeter) - #656
Conversation
11ebdea to
11dcc9d
Compare
11dcc9d to
977f8d1
Compare
priyanshuagarwal-harness
changed the title
0f90b53 to
00a4e75
Compare
00a4e75 to
cfc82f7
Compare
priyanshuagarwal-harness
changed the title
priyanshuagarwal-harness
changed the title
067025f to
245203f
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } else if (toolType === "Locust") { | ||
| toolBlock = buildLocustToolConfig(b, { scriptSource, script, targetUrl }); | ||
| } else { | ||
| const supplied = b.tool_config as Record<string, unknown> | undefined; |
There was a problem hiding this comment.
Important (schema vs create): bodySchema advertises tool_config as a general escape hatch for Locust/K6/JMeter, but create only honors tool_config on the JMeter branch (Locust/K6 silently ignore it). Agents following the schema can believe they authored nested config when only scalars were used.
Either reject tool_config for Locust/K6 on create, or honor it like update’s escape hatch — and tighten the field description accordingly.
Architecture review (Sunil standards checklist) — request changesChecked against Passes
Fails / gaps (inline comments filed)
Nits (not blocking alone): execute Please address the blocking/important inlines before merge. Happy to re-review after. |
| (toolBlock as Record<string, unknown>).variables = b.variables; | ||
| } | ||
|
|
||
| const body: Record<string, unknown> = { |
There was a problem hiding this comment.
Architecture note (migration): create builds a fresh toolConfig body and never accepts legacy flat inputs[] / scriptContent. Intentional, but old callers get silent omission of tunables rather than a loud error (Sunil: fail loudly / don’t silent-drop). If any legacy key is present, throw with a migration hint to the new scalars/toolConfig shape.
| } | ||
| const out: JMeterThreshold = { metric, operator, value: t.value as number }; | ||
| if (stat) out.stat = stat; | ||
| if (t.abort_on_fail === true) out.abortOnFail = true; |
There was a problem hiding this comment.
Nit / Sunil falsy-drop pattern: abort_on_fail: false and send_to_engines: false are omitted rather than sent as false (=== true gates). Examples/docs often show explicit false. Prefer != null and always project the boolean when provided, matching the “body builders must not silent-drop falsy values” architecture lesson.
…s of chaos services
532426a to
7eb25a7
Compare
| (toolBlock as Record<string, unknown>).variables = b.variables; | ||
| } | ||
|
|
||
| const body: Record<string, unknown> = { |
There was a problem hiding this comment.
Optional follow-up (Sunil fail-loudly): legacy flat inputs[] / scriptContent are still silently ignored on create (tests assert they stay undefined). A loud migration error when those keys are present would save agents from “tunables vanished” debugging. Non-blocking if you’re intentionally hard-cutting.
| } | ||
| const out: JMeterThreshold = { metric, operator, value: t.value as number }; | ||
| if (stat) out.stat = stat; | ||
| if (t.abort_on_fail === true) out.abortOnFail = true; |
There was a problem hiding this comment.
Nit (still open / Sunil falsy-drop): abort_on_fail: false and send_to_engines: false are still omitted via === true gates. Prefer projecting the boolean whenever the caller provided it (!= null) so explicit false survives round-trips.
| { name: "properties", type: "array", required: false, description: descLoadtestProperties }, | ||
| { name: "thresholds", type: "array", required: false, description: descLoadtestThresholds }, | ||
| { name: "variables", type: "array", required: false, description: "Custom template.Variable entries stored under toolConfig.<tool>.variables." }, | ||
| { name: "tool_config", type: "object", required: false, description: "Pass-through toolConfig object -- escape hatch for advanced/back-compat use (e.g. JMeter .zip bundles). Not required for Locust/K6/JMeter; prefer the scalar fields (script/script_image/properties/env_vars/thresholds/worker_count)." }, |
There was a problem hiding this comment.
Still open (docs/schema ≠ create): create prose now says JMeter tool_config is the advanced escape hatch, but this bodySchema description still reads as a general Locust/K6/JMeter pass-through (“Not required for Locust/K6/JMeter”). Create still only honors tool_config on the JMeter branch — Locust/K6 silently ignore it.
Please tighten this field description to “JMeter-only on create” (or reject tool_config for Locust/K6) so agents don’t think nested authoring worked.
| export const descCreateChaosService = `Onboard (create) a chaos service. This is a GUIDED, ORDERED workflow — do NOT skip ahead or invent identifiers; each step depends on the selection made in the previous one. Do NOT advance to the next step until the current step's selection is made, and NEVER call harness_create(chaos_service) until every step below is resolved and the user has confirmed. | ||
|
|
||
| STEP 1 — Select a Discovery Agent (REQUIRED FIRST; gate: do not continue without a chosen agent). | ||
| Call harness_list(resource_type='discovered_agent', org_id, project_id). Show each agent's name, identity, serviceCount, and last-discovery status. Ask the user to pick one. |
There was a problem hiding this comment.
Nit (still open): STEP 1 still asks agents to show “last-discovery status”, but compactDiscoveredAgent only keeps identity/name/.../serviceCount/networkMapCount/installationType/createdAt/updatedAt. If there is a real discovery-status field on the SD agent payload, keep it here; otherwise drop “last-discovery status” from STEP 1 so agents don’t invent one.
Re-review (head
|
2 participants
Summary
Adds MCP registry coverage for Chaos Service Management and the Service Discovery agent prerequisite flow, and brings
chaos_loadtestcreate/update up to parity with the Harness UI's Load Test Studio across all three tools (Locust/K6/JMeter).New resources
chaos_servicediscovered_agentagent_idwhen creating/updating a chaos servicechaos_k8s_infrastructurealso gains acreateoperation (registration body).chaos_serviceenvironment_ids,infrastructure_ids,tags,include_all_scope,search,probe_ids,onboarding_id, sort/pagination.externalServiceId,agentId,environmentId,infrastructureId, probe associations with inputs).list_experiment_runs,list_load_tests(viaserviceReferences).discovered_agent-> (optional)discovered_namespace->discovered_service-> (optional)chaos_probe->harness_create(chaos_service).chaosServiceListExtractfor the v3 list envelope{ data, pagination }.chaos_loadtest— schema migration + authoring paritytoolConfig.<tool>.tunables/toolConfig.<tool>.variables(perLocustSpec/K6Spec/JMeterSpecin loadTestManager), replacing the old flatinputs[]. Added theupdateoperation.script_image+script_entrypoint+load_args+image_pull_secretnow build identicaltoolConfig.<tool>.script.{image,entrypoint,loadArgs,imagePullSecret}blocks for Locust, K6, and JMeter (JMeter's image branch previously lackedloadArgs/imagePullSecret).properties[](JMeter-J/-Gruntime property overrides) andthresholds[](pass/fail criteria evaluated against.jtlrun results:metric/stat/operator/value/abort_on_fail).cleanup_policy(delete/retain) andresources(per-pod CPU/memorylimits/requests, Kubernetes only) now flow through to both the wire body and the synthesizedspec.yamlmanifest, matching what the Load Test Studio UI writes.descCreateLoadtestrewritten as a step-by-step guided flow (target type -> infra -> service -> tool type -> script/image mode -> load config -> Advanced Options) with per-tool YAML deltas, replacing stale/contradictory "JMeter has no scalar shortcuts" guidance.extractors.ts) updated to read the new nestedtoolConfig.<tool>.{tunables,script,variables}shape instead ofinputs[].Tests & docs
tests/registry/chaos-service.test.ts— list filter mapping,list_experiment_runs/list_load_testsexecute actions.tests/registry/chaos-loadtest.test.tssubstantially expanded/refactored for the new toolConfig shape: Locust/K6/JMeter script + Custom Image coverage (incl. private-registryimagePullSecret,load_argsvalidation), JMeterproperties/thresholds/env_vars, andcleanupPolicy/resourcespass-through + YAML emission.chaos_k8s_infrastructurecreate and new execute actions documented.Type of Change
Checklist
pnpm testpassespnpm typecheckpassespnpm buildpassespnpm standards:checkpassespnpm docs:checkpasses (resource count changed 223 -> 225; README updated)Coding Standards (registry-driven MCP model)
server.registerTool()calls — only toolset definitions insrc/registry/toolsets/.chaos_service/discovered_agentregistered under the existingchaostoolset.operationPolicyset on every new/changed endpoint.src/registry/extractors.ts(chaosServiceListExtract, reusedsdPageExtractfor agents).identifierFieldsandscopedeclared on new resources.console.log()insrc/.Test plan
pnpm test tests/registry/chaos-service.test.tspnpm test tests/registry/chaos-loadtest.test.tsharness_list(resource_type="discovered_agent")in a project with SD agents deployed.harness_list(resource_type="chaos_service")with environment/infrastructure filters.harness_create(resource_type="chaos_loadtest")for Locust/K6/JMeter in both script and Custom Image mode, incl.cleanup_policy="retain"andresourceslimits/requests.harness_execute(resource_type="chaos_service", action="list_experiment_runs"/"list_load_tests", identity=...).