From 00f39c849e477c8c66a941c8ba254d791263b1e7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 15 Jul 2026 05:12:52 -0400 Subject: [PATCH] docs(modbus-tests): document profile-cycling gotcha; sweep #2 was a false alarm The 2026-07 integration sweep flagged the Modbus non-standard profiles as 8F/5F/3F/8F and guessed 'pymodbus 4.x drift'. Re-investigation shows that was wrong on both counts: - The image runs the PINNED pymodbus 3.13.0 (verified via import pymodbus; __version__). The '...removed in v4' log is 3.13.0's forward-deprecation warning, misread as running 4.x. - The failures were a fixture-cycling artifact: each profile is a separate profile-gated compose service sharing :5020, and a plain 'docker compose down' (no --profile) does NOT stop the running profile container. It kept holding :5020; the next '--profile up' container silently failed to bind (compose still says 'Started'), so every later profile run hit the STALE sim. Proven by a raw Modbus probe: the 'mitsubishi' sim served DL205's 0xCAFE at HR0. After force-removing all modbus containers and bringing up each profile cleanly (verifying it actually published :5020), all four pass: dl205 16/16, mitsubishi 13/13, s7_1500 10/10, exception_injection 17/17. No OtOpcUa or test code change needed. Added a profile-cycling gotcha + reliable-cycle recipe to Docker/README.md and corrected the sweep record. Integration-sweep follow-up #2. --- archreview/plans/INTEGRATION-SWEEP-STATUS.md | 41 ++++++++++++++----- .../Docker/README.md | 25 +++++++++++ 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/archreview/plans/INTEGRATION-SWEEP-STATUS.md b/archreview/plans/INTEGRATION-SWEEP-STATUS.md index 2cbaf4e4..6e3b3607 100644 --- a/archreview/plans/INTEGRATION-SWEEP-STATUS.md +++ b/archreview/plans/INTEGRATION-SWEEP-STATUS.md @@ -23,7 +23,7 @@ need sudo to create new ones — I deployed FOCAS to `~/otopcua-focas` on the ho | **Modbus** (profile=standard) | ✅ 6/6 | — | | **Host** | ⚠️ **105/117** (7F, 5 skip) | 7 heavy 2-node deploy/failover E2E time out (20–45 s) under amd64-emulated SQL; 5 LDAP-backed skip | | **TwinCAT** | ⏭️ 13 skip | no TC3 XAR fixture exists (expected clean-skip) | -| **Modbus** (dl205 / mitsubishi / s7_1500 / exception_injection) | ❌ 8F/5F/3F/8F | sim-behavior drift — see below | +| **Modbus** (dl205 / mitsubishi / s7_1500 / exception_injection) | ✅ 16/13/10/17 (was 8F/5F/3F/8F) | **NOT drift** — fixture-cycling artifact, re-verified GREEN 2026-07-15 — see below | | **AbCip** | ✅ 10P/3skip (was 6F/1P/3skip) | static-init-order bug FIXED 2026-07-15 (`326b22a7`) — see below | | **FOCAS** | ❌ 9F/1P | tests assume a localhost-colocated mock — see below | | **AbLegacy** | ❌ 4F | no AbLegacy sim fixture deployed to the host | @@ -31,21 +31,36 @@ need sudo to create new ones — I deployed FOCAS to `~/otopcua-focas` on the ho ### Headline conclusion **No OtOpcUa production regression.** Every failure is **pre-existing test/fixture infrastructure rot** in -long-deferred suites. The drivers are fine: 4 suites fully green, Modbus-standard green, Host 105/117 green, +long-deferred suites — or, in the Modbus case, a **fixture-cycling false alarm** (see below). The drivers are +fine: 4 suites fully green, **all Modbus profiles green** (standard + dl205/mitsubishi/s7_1500/exception once +correctly bound — the sweep's non-standard "failures" were the stale-container artifact), Host 105/117 green, and all driver **unit** suites + the live `/run` gates pass. These integration suites were DEFERRED in round 2 -and never run, so they've bit-rotted in several distinct ways. +and never run, so they'd bit-rotted in several distinct ways. + +**Follow-up progress (2026-07-15):** #1 AbCip (PR #446), #5 OpcUaServer (PR #447), and #2 Modbus (PR #448 — false +alarm, no code) all closed. Remaining: #3 FOCAS, #4 AbLegacy, #6 Host — each needs a design/infra decision. --- ## Per-suite failure detail (evidence) -- **Modbus non-standard profiles** — NOT stale fixtures (all `profiles/*.json` register maps byte-match the - repo) and NOT an OtOpcUa regression (standard profile + all Modbus unit suites green). The sim container runs - **pymodbus 4.x-dev** (logs: *"ModbusServerContext is deprecated … removed in v4"*, `pymodbus.simulator - --json_file /fixtures/.json`). Failures are `StatusCode` assertion mismatches on BCD decode / coil - mapping / DL205 addressing — a pymodbus-version behavioral drift vs when the tests were authored. - **Fix:** pin pymodbus in `tests/.../Modbus.IntegrationTests/Docker/Dockerfile` to the version the tests - target, or update expected values for 4.x. +- **Modbus non-standard profiles** — ✅ **RE-VERIFIED GREEN 2026-07-15 (PR #448). The sweep's 8F/5F/3F/8F was a + false alarm — a fixture-cycling artifact, NOT pymodbus drift and NOT an OtOpcUa regression.** Correcting the + sweep's original diagnosis: + - The image genuinely runs **pymodbus 3.13.0** (`docker run --entrypoint python ... -c 'import pymodbus; + print(pymodbus.__version__)'` → `3.13.0`; the Dockerfile already pins `pymodbus[simulator]==3.13.0`). The + *"…removed in v4"* log line is 3.13.0's **forward-deprecation warning**, not evidence it runs 4.x — the + original sweep note misread it. + - Root cause: each Modbus profile is a **separate profile-gated compose service** sharing `:5020`. A plain + `docker compose down` (no `--profile`) does **not** stop the running profile container, so it keeps holding + `:5020`; the next `--profile … up` container silently fails to bind (compose still says "Started"). Every + later profile run then hit the **stale** container. Proven by a raw Modbus probe: the "mitsubishi" sim + served DL205's `0xCAFE` at HR0 (DL205's V0 marker) and Illegal-Data-Address for mitsubishi-only addresses, + while `docker ps` showed the mitsubishi container up with **no** port mapping and DL205 still holding `:5020`. + - After force-removing all modbus containers and bringing up each profile cleanly (verifying it actually + published `:5020`), all four pass: **dl205 16/16, mitsubishi 13/13, s7_1500 10/10, exception_injection + 17/17** (the injector needs `--force-recreate` — it uses a different container name). **No code change** — + added a profile-cycling gotcha + reliable-cycle recipe to `Docker/README.md`. - **AbCip** — **static-init-order bug in the test harness** (always-fail, blocks the whole suite). `tests/.../AbCip.IntegrationTests/AbServerProfile.cs`: `KnownProfiles.All { get; } = [ControlLogix, …]` is @@ -93,7 +108,11 @@ and never run, so they've bit-rotted in several distinct ways. ✅ **DONE 2026-07-15** (commit `326b22a7`, PR #446). Live-verified against the controllogix fixture: suite went **6F/1P/3skip → 10 pass / 0 fail / 3 skip** (skips = emulator-mode tests needing a different fixture mode). -2. **Modbus non-standard profiles** — pin pymodbus in the fixture Dockerfile (or update expected values for 4.x). +2. ~~**Modbus non-standard profiles** — pin pymodbus in the fixture Dockerfile.~~ + ✅ **RESOLVED 2026-07-15 (PR #448) — false alarm, no code change.** Image already runs the pinned + pymodbus 3.13.0; the 8F/5F/3F/8F was a fixture-cycling artifact (stale profile container holding `:5020`). + Re-verified all four profiles GREEN (16/13/10/17). Added a profile-cycling gotcha + reliable-cycle recipe + to `Docker/README.md`. 3. **FOCAS** — reconcile the 127.0.0.1-mock topology with the remote-fixture model. 4. **AbLegacy** — deploy an AbLegacy sim fixture (none on host). 5. ~~**OpcUaServer** — supply cert-store StorePath in the harness config.~~ diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/Docker/README.md b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/Docker/README.md index 0986415c..306fbc52 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/Docker/README.md +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/Docker/README.md @@ -48,6 +48,31 @@ service + starting another. The integration tests discriminate by a separate `MODBUS_SIM_PROFILE` env var so they skip correctly when the wrong profile is live. +> **⚠️ Profile-cycling gotcha (bit us in the 2026-07 sweep — see +> `archreview/plans/INTEGRATION-SWEEP-STATUS.md`).** Each profile is a +> *separate compose service* selected by `--profile`, so a plain +> `docker compose down` (no `--profile`) does **not** stop the +> currently-running profile container — it keeps holding `:5020`. Start +> another profile and the new container silently fails to bind the port +> (compose still reports "Started"); every test then hits the **stale** +> profile's data and fails with wrong values / Illegal-Data-Address. The +> symptom is deceptive: reads succeed but return the previous profile's +> seed (e.g. Mitsubishi's `D0` reads DL205's `0xCAFE`). Always tear the +> old container down **by name / with its profile** before bringing up the +> next, and verify the new container actually published `:5020`: +> +> ```bash +> # reliable cycle (force-remove ALL modbus containers, then bring one up) +> docker rm -f $(docker ps -aq --filter name=otopcua-modbus --filter name=otopcua-pymodbus) +> docker compose --profile up -d --force-recreate +> docker ps --filter name=otopcua --format '{{.Names}} {{.Ports}}' # MUST show 0.0.0.0:5020->5020 +> ``` +> +> The `exception_injection` service uses a different container name +> (`otopcua-modbus-exception-injector`, not `otopcua-pymodbus-*`), so a +> name-filtered `docker rm` that only matches `otopcua-pymodbus` will miss +> it — the `--force-recreate` above covers that case. + ### Profile coverage matrix The two general-purpose profiles cover disjoint test sets. A full pass