Skip to content

Add wpa_supplicant mount for CNET tests in SDKContainer - #368

Merged
rquidute merged 2 commits into
project-chip:v2.16-developfrom
khodya:cnet-tests
Sep 14, 2026
Merged

rquidute merged 2 commits into
project-chip:v2.16-developfrom
khodya:cnet-tests

Conversation

@khodya

@khodya khodya commented Sep 9, 2026

Copy link
Copy Markdown

Summary

Adds a /var/run/wpa_supplicant volume mount to the chip-cert-bins (SDK)
container that the Test Harness starts automatically, so CNET (Network
Commissioning cluster) test cases can manage Wi-Fi networks on the host
without requiring testers to start the container manually with an extra -v
flag.

Background

CNET Wi-Fi network commissioning tests talk to the host's wpa_supplicant
over its control socket. SDKContainer.run_parameters["volumes"] (used by
container_manager.create_container(...) when the TH engine/UI boots the
container) already mounts /var/run/dbus/system_bus_socket, PAA certs,
credentials, and the python_testing tree — but not /var/run/wpa_supplicant.
Without it, CNET tests run through the automated TH engine can't reach
wpa_supplicant, forcing manual docker run ... -v /var/run/wpa_supplicant:/var/run/wpa_supplicant ... workarounds outside the
TH.

Changes

  • test_collections/matter/sdk_tests/support/sdk_container.py:
    • Added LOCAL_WPA_SUPPLICANT_PATH / DOCKER_WPA_SUPPLICANT_PATH constants
      (/var/run/wpa_supplicant), following the existing mount-constant pattern.
    • Added the corresponding entry to SDKContainer.run_parameters["volumes"]
      (mode: rw, matching the D-Bus socket mount).

Test plan

  • Start the SDK container via the TH engine and confirm
    /var/run/wpa_supplicant is bind-mounted (docker inspect <container> | grep wpa_supplicant).
  • Run a CNET test case through the TH and confirm it can add/update/remove
    Wi-Fi networks without the manual volume workaround.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: b6bde68e-9aa7-4c88-8a11-1f756409aa6a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 677bb672-da62-4238-a8e0-5c71e4da5b5a

📥 Commits

Reviewing files that changed from the base of the PR and between d7c2bb2 and 9b2fe31.

📒 Files selected for processing (1)
  • test_collections/matter/sdk_tests/support/sdk_container.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The SDK container configuration now defines host and container paths for the WPA supplicant socket. It mounts the host socket into the container with read-write access for CNET Wi-Fi tests.

Suggested reviewers: greens

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 9b2fe

SDK test containers now receive the host WPA supplicant control socket needed for CNET Wi-Fi commissioning tests. No concrete current-head merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description clearly explains the added WPA supplicant volume mount and its purpose for CNET Wi-Fi tests.
Title check ✅ Passed The title clearly and concisely identifies the main change: adding the wpa_supplicant mount for CNET tests in SDKContainer.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greens greens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly my area of expertise, but would this still work on a host that's not running supplicant or has no wifi interfaces?

@cecille

cecille commented Sep 9, 2026

Copy link
Copy Markdown

Are there pi's that don't have wifi interfaces?

@cecille cecille left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've discussed this change already offline. I'm not a docker expert, but this looks correct to me.

@greens

greens commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

@cecille unsure, but would this break, for example, any developer attempting to run locally on a desktop machine, server, or mini-pc without one?

I understand the Pi is the target platform, but I think broad compatibility is still a good goal.

@oxesoft

oxesoft commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

@khodya In case this is expected for 1.7 TE2, please, change the target branch to v2.16-develop.

@khodya
khodya changed the base branch from main to v2.16-develop September 11, 2026 17:15
@mergify

mergify Bot commented Sep 11, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@oxesoft

oxesoft commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

This mount is added unconditionally to the class-level run_parameters["volumes"] dict, so it applies to every SDKContainer.start() call — i.e. every test run, not just CNET/Wi-Fi tests. On hosts without wpa_supplicant (no Wi-Fi hardware, wired-only mini-PCs/servers, or non-Linux Docker hosts), this could break SDK container creation entirely for all tests, not just CNET ones:

  • Docker Desktop for Mac only shares /Users, /Volumes, /private, /tmp, /var/folders by default — /var/run isn't on that list, so binding it would likely fail with "Mounts denied."
  • Rootless Docker on Linux runs the daemon unprivileged and typically can't auto-create a directory under root-owned /var/run.

Suggest gating the mount on the host path actually existing, checked at container-start time rather than baked into the static class attribute, e.g.:

def _optional_volumes() -> dict:
    volumes = {}
    if LOCAL_WPA_SUPPLICANT_PATH.is_dir():
        volumes[LOCAL_WPA_SUPPLICANT_PATH] = {
            "bind": DOCKER_WPA_SUPPLICANT_PATH,
            "mode": "rw",
        }
    return volumes

and merge it into volumes inside start() before calling container_manager.create_container(...). That way hosts without wpa_supplicant behave exactly as they did before this PR (CNET Wi-Fi tests just don't work, same as requiring the manual -v flag today), instead of risking a failure to start the SDK container for every test.

@oxesoft

oxesoft commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

@khodya Could you please address the comment above ASAP? We are about to kick off 1.7 TE2 and the related documentation was already integrated.

The mount was previously baked into the class-level run_parameters,
so it applied to every SDK container start and could break container
creation on hosts without wpa_supplicant/Wi-Fi hardware (Docker
Desktop for Mac, rootless Docker, wired-only mini-PCs/servers).

Now the mount is added conditionally at start() time, only when
/var/run/wpa_supplicant exists on the host, so unaffected hosts keep
their previous behavior.
@khodya

khodya commented Sep 14, 2026

Copy link
Copy Markdown
Author

@oxesoft fixed, thanks!

@rquidute
rquidute merged commit ad32993 into project-chip:v2.16-develop Sep 14, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants