fix(config): give values_sim.yml an entry for every pool the simulator serves [HOLD — waits on smart-router#262] - #98
Conversation
…r serves The simulator binds listeners for 14 pools. This file carried 9 of them, so four pools the running process actually serves had no router-side entry here at all: eth-duo-sim, eth-best-sim, eth-priority-sim and eth-precedence-sim. The guard test could not see the gap because each missing pool was named in POOLS_WITH_NO_ROUTER_HERE, the allow-list that excuses an absence. Three of those four excuses had stopped being true — canonical deploys routers for eth-duo-sim, eth-priority-sim and eth-precedence-sim today — so the file drifted while the test stayed green. Add the four entries, ports and names taken from TOPOLOGY, and shrink the allow-list to the one pool that is genuinely unwired. eth-best-sim gets an entry but cannot be deployed through the chart: its whole purpose is --qos-selection-mode=best, and chart smart-router-5.15.0 has no key that emits that flag, with no extraArgs passthrough. Rendering the chart against canonical's own values produces qos-selection-mode zero times. Only the k3d cluster passes router arguments straight through, and its row ships disabled, so nothing runs it today. The entry is here so this file matches what the simulator serves; the comment says why copying it into a deploy repo would not work yet. Also correct two docstrings that described the old state: topology.py called eth-duo-sim k3d-only when canonical runs it, and said priority and precedence had no router wired. Verified: full suite 1021 passed.
|
Related to smart-router#262 ( This repo's copy of Recording the dependency so it is not lost: Companion: Magma-Devs/smart-router-automation#677. |
VicSheCodes
changed the title
|
Held — not for this release. Converted to draft so it cannot be merged by accident. Gate: smart-router#262 ( To be precise about what is and is not gated, because only one of the four pools really is:
Nothing in this file deploys anything — it is read only by Copilot review requested; any comments will be addressed while it waits, so it is ready to merge the moment the gate clears. |
There was a problem hiding this comment.
Pull request overview
This PR brings config/values_sim.yml back into alignment with the simulator’s declared topology by ensuring every simulator-served pool that has (or should have) a router entry is represented in the values file, and by tightening the guarding test so missing pools can’t be silently excused.
Changes:
- Added router entries in
config/values_sim.ymlforeth-duo-sim,eth-best-sim,eth-priority-sim, andeth-precedence-sim, with ports/provider names aligned toTOPOLOGY. - Reduced
POOLS_WITH_NO_ROUTER_HEREin the guard test to onlyln-sim, so newly-missing pools fail the suite rather than being allow-listed. - Updated
provider_simulator/topology.pydocumentation to reflect the current deployment reality (canonical vs k3d constraints) for these pools.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/test_values_sim_matches_topology.py | Tightens the exception list and updates the module docstring to reflect that only ln-sim is intentionally uncovered by values_sim.yml. |
| provider_simulator/topology.py | Updates the pool/router relationship documentation to match the current wiring and deployment constraints. |
| config/values_sim.yml | Adds missing router entries so the values file covers all simulator pools that are intended to have routers configured. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
What was wrong
The simulator binds listeners for 14 pools.
config/values_sim.ymlcarried 9. Four pools the running process actually serves had no entry here at all:eth-duo-simeth-priority-simeth-precedence-simeth-best-simThe guard test could not see the gap, because every missing pool was named in
POOLS_WITH_NO_ROUTER_HERE— the allow-list that excuses an absence. Three of those four excuses had stopped being true. The file drifted for months while the test stayed green.What this changes
TOPOLOGY, not hand-typed.POOLS_WITH_NO_ROUTER_HEREtoln-sim, the one pool that genuinely has no router anywhere.topology.pycalledeth-duo-simk3d-only when canonical runs it, and said priority and precedence had no router wired.The one pool that cannot be deployed
eth-best-simgets an entry, but copying it into a deploy repo would not give you a working router. Its whole purpose is--qos-selection-mode=best, and chartsmart-router-5.15.0has no key that emits that flag and noextraArgspassthrough — rendering the chart against canonical's own values producesqos-selection-modezero times. Only the k3d cluster passes router arguments straight through, and its row shipsenabled: false, so nothing runs it today. The entry carries a comment saying exactly this.Verification
Full suite: 1021 passed. The guard test now covers all 13 pools that have routers, including every new port.
Why this PR is a DRAFT [HOLD — waits on smart-router#262]
Parked on purpose so it cannot be merged by mistake. One of the pools it declares (
eth-best-sim) only means something once Magma-Devs/smart-router#262 lands the--qos-selection-modeflag. This PR merges together with the automation-repo half of the same sync — see Magma-Devs/smart-router-automation#684 for the full state of that half and the decision to make at #262 time.