Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: linux musl detection #502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

YOU54F
Copy link
Member

@YOU54F YOU54F commented May 29, 2024

Rationale

pact-reference has introduced musl and arm64 based ffi libraries for linux

Tracking Issue

Issues Resolved

fixes #498
fixes #496
fixes #500
fixes #374
fixes #387

Backwards Compatibility

Linux glibc based hosts take precedence, so if any error occurs during musl detection. I do not anticipate breaking changes for users

Implementation notes

.NET notes

Conditions for execution

musl detection will run if

  • if linux
  • if /lib/ld-musl-(x86_64|aarch64).so.1 exists
  • if ldd bin/sh | grep musl is true (musl lib is loaded, rather than glibc)

will continue on error, reverting back to glibc based libaries.

Supported musl targets

should work for multiple musl based distroes if

  • /lib/ld-musl-(x86_64|aarch64).so.1 exists
  • ldd is available (available by default in alpine images)

Tested on Alpine ARM64 / AMD64.

Caveats

  • .NET does not run under QEMU affecting the ability to test multi-arch from a single system
  • .NET restore can take a long time when running under containers.
    • Workaround: Set DOTNET_NUGET_SIGNATURE_VERIFICATION to false

Compatibility

Operating System

Due to using a shared native library instead of C# for the main Pact logic only certain OSs are supported:

OS Arch Support
Windows x86 ❌ No
Windows x64 ✔️ Yes
Linux (libc) x86 ❌ No
Linux (libc) x64 ✔️ Yes
Linux (musl) x64 ✔️ Yes (Tier 2)*
Linux (libc) ARM ✔️ Yes (Tier 3)*
Linux (musl) ARM ✔️ Yes (Tier 3)*
OSX x64 ✔️ Yes
OSX ARM (M1/M2) ✔️ Yes

Support

  • Tier 1
    • Established
    • Full CI/CD support.
    • Users should not encounter issues
    • Full reproducible examples running in CI, should be provided by users raising issues
    • If using musl targets, users should attempt the same test on a libc target (such as debian)
  • Tier 2
    • Recently introduced
    • Full CI/CD support.
    • Users may encounter issues
    • Full reproducible examples running in CI, should be provided by users raising issues
    • If using musl targets, users should attempt the same test on a libc target (such as debian)
  • Tier 3
    • Recently introduced, No/limited CI/CD support.
    • Users may encounter issues
    • Full reproducible examples which can be run by maintainers locally, should be provided by users raising issues

.cirrus.yml Outdated Show resolved Hide resolved
@YOU54F
Copy link
Member Author

YOU54F commented May 29, 2024

There is some flakiness in the tests, I've had to rerun a couple, I've not ran enough jobs from master to see if this is something that has been introduced by this change, or an issue in the ffi lib or the test setup.

[xUnit.net 00:00:01.24]     PactNet.Tests.Verifier.Messaging.MessagingProviderTests.HandleMessage_NoDescription_ReturnsBadRequest [FAIL]
[xUnit.net 00:00:01.24]       PactNet.Exceptions.PactFailureException : Unable to start the internal messaging server
[xUnit.net 00:00:01.24]       ---- System.Net.HttpListenerException : Address in use
[xUnit.net 00:00:01.24]       Stack Trace:
[xUnit.net 00:00:01.24]         /home/runner/work/pact-net/pact-net/src/PactNet/Verifier/Messaging/MessagingProvider.cs(85,0): at PactNet.Verifier.Messaging.MessagingProvider.Start(JsonSerializerOptions settings)
[xUnit.net 00:00:01.24]         /home/runner/work/pact-net/pact-net/tests/PactNet.Tests/Verifier/Messaging/MessagingProviderTests.cs(45,0): at PactNet.Tests.Verifier.Messaging.MessagingProviderTests..ctor(ITestOutputHelper output)
[xUnit.net 00:00:01.24]            at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
[xUnit.net 00:00:01.24]            at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
[xUnit.net 00:00:01.24]         ----- Inner Stack Trace -----
[xUnit.net 00:00:01.24]            at System.Net.HttpEndPointManager.GetEPListener(String host, Int32 port, HttpListener listener, Boolean secure)
[xUnit.net 00:00:01.24]            at System.Net.HttpEndPointManager.AddPrefixInternal(String p, HttpListener listener)
[xUnit.net 00:00:01.24]            at System.Net.HttpEndPointManager.AddListener(HttpListener listener)
[xUnit.net 00:00:01.24]            at System.Net.HttpListener.Start()
[xUnit.net 00:00:01.24]         /home/runner/work/pact-net/pact-net/src/PactNet/Verifier/Messaging/MessagingProvider.cs(75,0): at PactNet.Verifier.Messaging.MessagingProvider.Start(JsonSerializerOptions settings)
[xUnit.net 00:00:01.24]       Output:
[xUnit.net 00:00:01.24]         Starting messaging provider at http://localhost:49152/pact-messages/
dbug: Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware[1]
      Request matched endpoint 'Provider.Orders.OrdersController.GetByIdAsync (Provider)'
info: Microsoft.AspNetCore.Routing.EndpointMiddleware[0]
      Executing endpoint 'Provider.Orders.OrdersController.GetByIdAsync (Provider)'

@areckoundarjian
Copy link

Hello! Just wondering if there is any information on when this will be merged / released. Many thanks :)

@dileepbg
Copy link

@YOU54F can you please let me know when this PR will be merged and released.?

@YOU54F
Copy link
Member Author

YOU54F commented Jun 18, 2024

hey its beyond my control as i am the contributor not the maintainer

@adamrodger
Copy link
Contributor

It's been school holidays so I've been on vacation and I'm a bit behind. I'll review this properly tomorrow when I'm not on my phone, but my initial observations are:

  • I'm not keen on the idea of different CI being defined for different platforms instead of a single CI definition. Things like the pass/fail annotation on GitHub commits are important, and if that has a tick but the other CI is failing then that's really bad. That's not to mention the maintenance burden.

  • I'm not comfortable introducing the entire new concept of "tiered" support. I think something is either supported or not, and users will quite rightly not accept the response of "oh sorry, that's tier 2 so it's broken" as if that in any way changes the response they'd expect to their bug report.

  • the delta on the csproj is big and appears to have embedded scripting...? That needs very careful consideration during a proper review.

@YOU54F
Copy link
Member Author

YOU54F commented Jun 18, 2024

we dont linux arm64 gh machines yet and dotnet wont run under qemu

tiered targets are literally the premise of the rust.

if we dont have ci, then a user cant raise a reproducible bug for that platform so it requires someone with that combo to reproduce.

people can argue than it should be supported but if we cant test it, it gets tricky, so we cant ask for all the good stuff reproducers and the like.

i dont get your argument. its open source software; no warranty implied or given

@YOU54F
Copy link
Member Author

YOU54F commented Jun 18, 2024

hope you had a nice hol,

yeah the csproj change looks huge but it’s actually quite small. we need to wrap a code block which shifts the change down and shows a large delta.

happy to annotate or walk you through the changes.

@YOU54F
Copy link
Member Author

YOU54F commented Jun 18, 2024

There is some flakiness in the tests, I've had to rerun a couple, I've not ran enough jobs from master to see if this is something that has been introduced by this change, or an issue in the ffi lib or the test setup.

[xUnit.net 00:00:01.24]     PactNet.Tests.Verifier.Messaging.MessagingProviderTests.HandleMessage_NoDescription_ReturnsBadRequest [FAIL]
[xUnit.net 00:00:01.24]       PactNet.Exceptions.PactFailureException : Unable to start the internal messaging server
[xUnit.net 00:00:01.24]       ---- System.Net.HttpListenerException : Address in use
[xUnit.net 00:00:01.24]       Stack Trace:
[xUnit.net 00:00:01.24]         /home/runner/work/pact-net/pact-net/src/PactNet/Verifier/Messaging/MessagingProvider.cs(85,0): at PactNet.Verifier.Messaging.MessagingProvider.Start(JsonSerializerOptions settings)
[xUnit.net 00:00:01.24]         /home/runner/work/pact-net/pact-net/tests/PactNet.Tests/Verifier/Messaging/MessagingProviderTests.cs(45,0): at PactNet.Tests.Verifier.Messaging.MessagingProviderTests..ctor(ITestOutputHelper output)
[xUnit.net 00:00:01.24]            at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
[xUnit.net 00:00:01.24]            at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
[xUnit.net 00:00:01.24]         ----- Inner Stack Trace -----
[xUnit.net 00:00:01.24]            at System.Net.HttpEndPointManager.GetEPListener(String host, Int32 port, HttpListener listener, Boolean secure)
[xUnit.net 00:00:01.24]            at System.Net.HttpEndPointManager.AddPrefixInternal(String p, HttpListener listener)
[xUnit.net 00:00:01.24]            at System.Net.HttpEndPointManager.AddListener(HttpListener listener)
[xUnit.net 00:00:01.24]            at System.Net.HttpListener.Start()
[xUnit.net 00:00:01.24]         /home/runner/work/pact-net/pact-net/src/PactNet/Verifier/Messaging/MessagingProvider.cs(75,0): at PactNet.Verifier.Messaging.MessagingProvider.Start(JsonSerializerOptions settings)
[xUnit.net 00:00:01.24]       Output:
[xUnit.net 00:00:01.24]         Starting messaging provider at http://localhost:49152/pact-messages/
dbug: Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware[1]
      Request matched endpoint 'Provider.Orders.OrdersController.GetByIdAsync (Provider)'
info: Microsoft.AspNetCore.Routing.EndpointMiddleware[0]
      Executing endpoint 'Provider.Orders.OrdersController.GetByIdAsync (Provider)'

i was also to repro this against master and locally, but haven’t ascertained the cause yet

@adamrodger
Copy link
Contributor

In terms of the technical change itself, that looks OK, and thanks for linking into some of the MS detection stuff so we can be sure we're being consistent. It needs some tidying up but I'll do that afterwards.

The two points above are still concerns though. Rust might have the concept of "tiered" support, but PactNet doesn't 😀 A user raising a bug still rightly wants their bug fixed. We can't really hide behind "sorry, it's open source". That doesn't stop me being tagged on issues and doesn't stop people repeatedly asking for updates.

In terms of the CI, it appears GitLab Actions plans to support ARM runners (source) so we can add proper support there later. It's probably worth splitting this into two separate changes instead of trying to tackle both musl and ARM at the same time, because x64 musl and ARM currently have different implications for support and CI. ARM can be added later once GHA supports it (like they do with Mac ARM runners now) and once we have a good answer for how to run it locally to reproduce issues, but x64 musl shouldn't have to wait for ARM support to be mature.

One additional thing that I'd like to see investigated first is what happens when you attempt to run on different versions of Alpine. We're just seeing Alpine 3.20 images starting to come through now, but 3.19 will be supported for a long time to come yet. For example, the official .Net images ship with both Alpine 3.19 and 3.20 variants at the moment. What happens if you try to use the library on 3.19 and 3.20? Do they both work or does one break?

## Rationale

pact-reference has introduced musl and arm64 based ffi libraries for linux

- pact-foundation/pact-reference#416

Tracking Issue

- pact-foundation/roadmap#30

## Issues Resolved

fixes pact-foundation#498
fixes pact-foundation#496
fixes pact-foundation#500
fixes pact-foundation#374
fixes pact-foundation#387

## Backwards Compatibility

Linux glibc based hosts take precedence, so if any error occurs during musl
detection. I do not anticipate breaking changes for users

## Implementation notes

### .NET notes

- Docs
  - [Uses MSBuild Exec task](https://learn.microsoft.com/en-us/visualstudio/msbuild/exec-task?view=vs-2022)
- MSBuild Blog Posts
  - [Cross-Platform Build Events in .NET Core using MSBuild](https://jeremybytes.blogspot.com/2020/05/cross-platform-build-events-in-net-core.html)
  - [MSBuild 101: Using the exit code from a command](https://www.creepingcoder.com/2020/06/01/msbuild-101-using-the-exit-code-from-a-command/)
- Stack OverFlow
  - [Set PropertyGroup property to Exec output](https://stackoverflow.com/questions/76583824/set-propertygroup-property-to-exec-output)
- .NET runtime musl detection code
  - https://github.com/dotnet/runtime/blob/a50ba0669353893ca8ade8568b0a7d210b5a425f/src/mono/llvm/llvm-init.proj\#L7
  - https://github.com/dotnet/runtime/blob/a50ba0669353893ca8ade8568b0a7d210b5a425f/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Unix.cs\#L78t

### Conditions for execution

musl detection will run if

- if linux
- if /lib/ld-musl-(x86_64|aarch64).so.1 exists
- if ldd bin/sh | grep musl is true (musl lib is loaded, rather than glibc)

will continue on error, reverting back to glibc based libaries.

### Supported musl targets

should work for multiple musl based distroes if

- /lib/ld-musl-(x86_64|aarch64).so.1 exists
- ldd is available (available by default in alpine images)

Tested on Alpine ARM64 / AMD64.
@YOU54F YOU54F force-pushed the feat/linux_arm64_plus_musl_container_ci branch from ce5e411 to a41c667 Compare September 24, 2024 15:50
@YOU54F
Copy link
Member Author

YOU54F commented Sep 24, 2024

It's probably worth splitting this into two separate changes instead of trying to tackle both musl and ARM at the same time, because x64 musl and ARM currently have different implications for support and CI

Have split out linux-arm64 (libc) support into a separate PR. #521

Would you like x64 musl and arm64 musl changes split as well?

One additional thing that I'd like to see investigated first is what happens when you attempt to run on different versions of Alpine. We're just seeing Alpine 3.20 images starting to come through now, but 3.19 will be supported for a long time to come yet. For example, the official .Net images ship with both Alpine 3.19 and 3.20 variants at the moment. What happens if you try to use the library on 3.19 and 3.20? Do they both work or does one break?

Fair shout, have added 3.19 / 3.20 variants of the alpine containers. Tags taken from https://github.com/dotnet/dotnet-docker/blob/main/README.sdk.md#linux-amd64-tags

I've personally tested back to Alpine 3.15 with the shared library, although I imagine most users will be on much later versions

@YOU54F YOU54F changed the title feat: linux arm64 support & musl detection feat: linux musl detection Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants