Skip to content

Commit 5ea2b41

Browse files
simonjbeaumontLukasa
authored andcommitted
tests: Remove asserts on the value of local Vsock CID (#2975)
### Motivation: We have some tests for our Vsock support that only run when we detect the system supports Vsock. These weren't running in CI but, now we've migrated CI to GitHub Actions, they can. In prototyping this though, I discovered that one of our tests was too strict, and will fail for VMs running on Hyper-V, which is the hypervisor used by GitHub actions. Specifically, we have support for getting the local context ID by a channel option. This is implemented by an ioctl, but the semantics of the return value differ between the various Vsock transport kernel modules: - `virtio_transport`: returns the guest CID, which is a number greater than `VMADDR_CID_HOST`, but less than `VMADDR_CID_ANY`, and returns `VMADDR_CID_ANY` as an error. - `vsock_loopback`: returns `VMADDR_CID_LOCAL` always. - `vmci_transport`: returns the guest CID if the guest and host transport are active; `VMADDR_CID_HOST` if only the host transport is active; and `VMCI_INVALID_ID` otherwise, which happens to be the same value as `VMADDR_CID_ANY`. - `hyperv_transport`: returns `VMADDR_CID_ANY` always. For this reason, we should probably remove any attempts to interpret the validity of the value that comes back from the driver and users will need to know what to do with it. ### Modifications: - tests: Only run Vsock echo tests on Linux, when `vsock_loopback` is loaded. - tests: Remove asserts on the value of local Vsock CID. - ci: Add pipeline that runs Vsock tests. ### Result: - Vsock tests will no longer run unless `vsock_loopback` kernel module is loaded. - Vsock tests will no longer fail in Hyper-V VMs. - Vsock tests will now run in CI. (cherry picked from commit 64eb8bf)
1 parent 70b2d6e commit 5ea2b41

File tree

6 files changed

+29
-20
lines changed

6 files changed

+29
-20
lines changed

.github/workflows/pull_request.yml

+18
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,21 @@ jobs:
4141
with:
4242
name: "Integration tests"
4343
matrix_linux_command: "apt-get update -y -q && apt-get install -y -q lsof dnsutils netcat-openbsd net-tools curl jq && ./scripts/integration_tests.sh"
44+
45+
vsock-tests:
46+
name: Vsock tests
47+
runs-on: ubuntu-latest
48+
steps:
49+
- name: Checkout repository
50+
uses: actions/checkout@v4
51+
with:
52+
persist-credentials: false
53+
- name: Load vsock_loopback kernel module
54+
run: sudo modprobe vsock_loopback
55+
- name: Build package tests
56+
run: swift build --build-tests
57+
- name: Run Vsock tests
58+
shell: bash # explicitly choose bash, which ensures -o pipefail
59+
run: swift test --filter "(?i)vsock" | tee test.out
60+
- name: Check for skipped tests
61+
run: test -r test.out && ! grep -i skipped test.out

Sources/NIOPosix/VsockAddress.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ extension VsockAddress.ContextID {
217217
/// - On Darwin, the `ioctl()` request operates on a socket.
218218
/// - On Linux, the `ioctl()` request operates on the `/dev/vsock` device.
219219
///
220-
/// - Note: On Linux, ``local`` may be a better choice.
220+
/// - Note: The semantics of this `ioctl` vary between vsock transports on Linux; ``local`` may be more suitable.
221221
static func getLocalContextID(_ socketFD: NIOBSDSocket.Handle) throws -> Self {
222222
#if canImport(Darwin)
223223
let request = CNIODarwin_IOCTL_VM_SOCKETS_GET_LOCAL_CID

Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -1141,7 +1141,7 @@ final class AsyncChannelBootstrapTests: XCTestCase {
11411141
// MARK: VSock
11421142

11431143
func testVSock() async throws {
1144-
try XCTSkipUnless(System.supportsVsock, "No vsock transport available")
1144+
try XCTSkipUnless(System.supportsVsockLoopback, "No vsock loopback transport available")
11451145
let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 3)
11461146
defer {
11471147
try! eventLoopGroup.syncShutdownGracefully()

Tests/NIOPosixTests/EchoServerClientTest.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ class EchoServerClientTest: XCTestCase {
234234
}
235235

236236
func testEchoVsock() throws {
237-
try XCTSkipUnless(System.supportsVsock, "No vsock transport available")
237+
try XCTSkipUnless(System.supportsVsockLoopback, "No vsock loopback transport available")
238238
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
239239
defer {
240240
XCTAssertNoThrow(try group.syncShutdownGracefully())

Tests/NIOPosixTests/TestUtils.swift

+4-13
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,10 @@ extension System {
3030
}
3131
}
3232

33-
static var supportsVsock: Bool {
34-
#if canImport(Darwin) || os(Linux) || os(Android)
35-
guard let socket = try? Socket(protocolFamily: .vsock, type: .stream) else { return false }
36-
XCTAssertNoThrow(try socket.close())
37-
#if !canImport(Darwin)
38-
do {
39-
let fd = try Posix.open(file: "/dev/vsock", oFlag: O_RDONLY | O_CLOEXEC)
40-
try Posix.close(descriptor: fd)
41-
} catch {
42-
return false
43-
}
44-
#endif
45-
return true
33+
static var supportsVsockLoopback: Bool {
34+
#if os(Linux) || os(Android)
35+
guard let modules = try? String(contentsOf: URL(fileURLWithPath: "/proc/modules")) else { return false }
36+
return modules.split(separator: "\n").compactMap({ $0.split(separator: " ").first }).contains("vsock_loopback")
4637
#else
4738
return false
4839
#endif

Tests/NIOPosixTests/VsockAddressTest.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ class VsockAddressTest: XCTestCase {
5858
}
5959

6060
func testGetLocalCID() throws {
61-
try XCTSkipUnless(System.supportsVsock)
61+
try XCTSkipUnless(System.supportsVsockLoopback, "No vsock loopback transport available")
6262

6363
let socket = try ServerSocket(protocolFamily: .vsock, setNonBlocking: true)
6464
defer { try? socket.close() }
6565

66-
// Check the local CID is valid: higher than reserved values, but not VMADDR_CID_ANY.
66+
// Check we can get the local CID using the static property on ContextID.
6767
let localCID = try socket.withUnsafeHandle(VsockAddress.ContextID.getLocalContextID)
68-
XCTAssertNotEqual(localCID, .any)
69-
XCTAssertGreaterThan(localCID.rawValue, VsockAddress.ContextID.host.rawValue)
68+
69+
// Check the local CID from the socket matches.
7070
XCTAssertEqual(try socket.getLocalVsockContextID(), localCID)
7171

7272
// Check the local CID from the channel option matches.

0 commit comments

Comments
 (0)