Skip to content

Add Azure Linux to regular CI #115415

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

Merged
merged 9 commits into from
May 12, 2025
Merged

Add Azure Linux to regular CI #115415

merged 9 commits into from
May 12, 2025

Conversation

richlander
Copy link
Member

@richlander richlander commented May 8, 2025

Plan:

  • Upgrade Azure Linux to regular libraries CI.
  • Ensure Azure Linux and Ubuntu are both present in inner and outerloop
  • Ensure one raw VM is present in both inner and outerloop (but not the same one; partially blocked on Re-enable Azure Linux 3 testing #114276)
  • Debian can be moved to outer loop since it is no longer our default container OS
  • Outer loop should bias to latest OS version, like Ubuntu interim releases or Debian pre-release (which may require updates in servicing, for example Ubuntu 25.x to 26.04).
  • Will repeat on other branches (once this merges).

High-level summary:

  • Establishes clear tiers for functional tests for Linux.
  • Tier 1: Azure Linux, Debian, Ubuntu (due to being in inner and outer loop)
  • Tier 2: Alpine and CentOS Stream (due to being in inner loop)
  • Tier 3: Fedora and OpenSUSE (due to being in extra platforms)
  • Tiers represent risk/responsibility for the project as a whole.
  • Partners are encouraged to augment testing in their own infra, as needed.

Notes:

  • Ubuntu could be argued to be Tier 0 because we're testing (in effect) two LTS versions across inner and outer loop and it is also tested by CoreCLR test legs, resulting in the highest coverage.
  • Debian could be argued to be Tier 1.5 since it is in inner loop via CoreCLR and outer loop by Libraries resulting in lesser coverage.

Relevant test legs:

@jkotas

@Copilot Copilot AI review requested due to automatic review settings May 8, 2025 22:38
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 8, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds Azure Linux support to the regular CI pipelines by updating the job configuration in the libraries pipeline.

  • Added Azure Linux configuration to the conditional extra platforms build block.
  • Ensured consistency with the upgrade requirements per the referenced issue.

@richlander
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop-linux

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@richlander
Copy link
Member Author

@akoeplinger @ViktorHofer

@jkotas jkotas added area-Infrastructure and removed area-Infrastructure-coreclr needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 9, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

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

I'm good with this if @bartonjs and @vcsjones are.

Copy link
Member

@bartonjs bartonjs left a comment

Choose a reason for hiding this comment

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

CentOS Stream 10 or Debian 13 are important to keep on in inner loop because those two distros have OpenSSL 3.5 (latest and greatest). Deb-13 was taken out, but since CentOS Stream 10 is still there, and Ubuntu is still there, it's fine by me.

@richlander
Copy link
Member Author

@jeffhandley -- can you validate that the XSLT failure is OK? Sounds like it is not crypto related but it could be something else. Have we seen that failure elsewhere?

@jkotas
Copy link
Member

jkotas commented May 9, 2025

can you validate that the XSLT failure is OK?

The test is complaining about unexpected data and time formatting on Azure Linux:

e.g. 1:45 PM vs. 13:45

This failure needs to be addressed before this PR gets merged. We have not seen this failure anywhere else.

@vcsjones
Copy link
Member

vcsjones commented May 9, 2025

I can consistently reproduce it on AZL 3.

It appears fixed by #114200 which is not yet merged.

@richlander
Copy link
Member Author

This appears to be the relevant code: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.Xml/tests/Xslt/TestFiles/TestData/XsltApiV2/bug93189.xsl

We need to validate that this works correctly on Azure Linux 3.

@jeffhandley

@tarekgh
Copy link
Member

tarekgh commented May 9, 2025

I disabled the failing test case to unblock the PR. The issue is the date day is formatted with dd instead of d and time formatted with HH instead of hh. I'll wait to see if the test pass and will log issue track fixing the issue.

@jkotas
Copy link
Member

jkotas commented May 10, 2025

Set environment variables
ENV LANG en_US.UTF-8
ENV LANGUAGE en_US:en
ENV LC_ALL en_US.UTF-8

Do you see it as a temporary workaround or as an actual fix? I thought that we want the libraries tests to be robust against environment locale settings.

@tarekgh
Copy link
Member

tarekgh commented May 10, 2025

Do you see it as a temporary workaround or as an actual fix? I thought that we want the libraries tests to be robust against environment locale settings.

Agree. I want first to validate my understanding, but the test needs to change too to work reliably regardless of the machine configuration.

@jkotas
Copy link
Member

jkotas commented May 10, 2025

So dotnet/dotnet-buildtools-prereqs-docker#1427 should be reverted, and this test disabled and eventually fixed instead.

@tarekgh
Copy link
Member

tarekgh commented May 10, 2025

So dotnet/dotnet-buildtools-prereqs-docker#1427 should be reverted, and this test disabled and eventually fixed instead.

Looks the test not failing on other images which indicate the default locale was set there. So, I don't think reverting dotnet/dotnet-buildtools-prereqs-docker#1427 is needed. The test needs to change anyway though.

@richlander
Copy link
Member Author

richlander commented May 10, 2025

Relevant ENVs, for reference:

Set environment variables
ENV LANG en_US.UTF-8
ENV LANGUAGE en_US:en
ENV LC_ALL en_US.UTF-8

Here's how the images are configured (that recent PR has not yet come into effect):

rich@vancouver:~$ docker run --rm mcr.microsoft.com/dotnet-buildtools/prereqs:azurelinux-3.0-helix-amd64 locale
LANG=en_US.UTF-8
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_PAPER="en_US.UTF-8"
LC_NAME="en_US.UTF-8"
LC_ADDRESS="en_US.UTF-8"
LC_TELEPHONE="en_US.UTF-8"
LC_MEASUREMENT="en_US.UTF-8"
LC_IDENTIFICATION="en_US.UTF-8"
LC_ALL=en_US.UTF-8
rich@vancouver:~$ docker run --rm mcr.microsoft.com/dotnet-buildtools/prereqs:debian-13-helix-amd64 locale
LANG=en_US.utf8
LANGUAGE=
LC_CTYPE="en_US.utf8"
LC_NUMERIC="en_US.utf8"
LC_TIME="en_US.utf8"
LC_COLLATE="en_US.utf8"
LC_MONETARY="en_US.utf8"
LC_MESSAGES="en_US.utf8"
LC_PAPER="en_US.utf8"
LC_NAME="en_US.utf8"
LC_ADDRESS="en_US.utf8"
LC_TELEPHONE="en_US.utf8"
LC_MEASUREMENT="en_US.utf8"
LC_IDENTIFICATION="en_US.utf8"
LC_ALL=
rich@vancouver:~$ docker run --rm mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-25.04-helix-amd64 locale
LANG=en_US.utf8
LANGUAGE=
LC_CTYPE="en_US.utf8"
LC_NUMERIC="en_US.utf8"
LC_TIME="en_US.utf8"
LC_COLLATE="en_US.utf8"
LC_MONETARY="en_US.utf8"
LC_MESSAGES="en_US.utf8"
LC_PAPER="en_US.utf8"
LC_NAME="en_US.utf8"
LC_ADDRESS="en_US.utf8"
LC_TELEPHONE="en_US.utf8"
LC_MEASUREMENT="en_US.utf8"
LC_IDENTIFICATION="en_US.utf8"
LC_ALL=
$ docker run --rm mcr.microsoft.com/dotnet-buildtools/prereqs:centos-stream-10-helix-amd64 locale
LANG=en_US.utf8
LC_CTYPE="en_US.utf8"
LC_NUMERIC="en_US.utf8"
LC_TIME="en_US.utf8"
LC_COLLATE="en_US.utf8"
LC_MONETARY="en_US.utf8"
LC_MESSAGES="en_US.utf8"
LC_PAPER="en_US.utf8"
LC_NAME="en_US.utf8"
LC_ADDRESS="en_US.utf8"
LC_TELEPHONE="en_US.utf8"
LC_MEASUREMENT="en_US.utf8"
LC_IDENTIFICATION="en_US.utf8"
LC_ALL=

Upon seeing this (which I should have done initially), It doesn't seem like the PR we made is correct. The only interesting difference is that LC_ALL isn't set to a value on Debian and Ubuntu.

Revert: dotnet/dotnet-buildtools-prereqs-docker#1428

@jkotas
Copy link
Member

jkotas commented May 10, 2025

I don't think reverting dotnet/dotnet-buildtools-prereqs-docker#1427 is needed.

I disagree. We want our default test environment to be as close as possible to the default environment used by customers.

We know that Azure Linux is non-standard in number of ways. We want to cover as much of these differences as possible in our tests. It is the reason why we are beefing up the Azure Linux testing.

@tarekgh
Copy link
Member

tarekgh commented May 10, 2025

Upon seeing this (which I should have done initially), It doesn't seem like the PR we made is correct.

Not really, setting the environment variables I sent is enough and not necessary setting all the rest.

I disagree. We want our default test environment to be as close as possible to the default environment used by customers.

I don't mind either way. The failing test is checking against hardcoded precalculated values depending on specific settings. If we think testing with default image settings is beneficiary for whole tests, that will be fine.

@richlander
Copy link
Member Author

richlander commented May 10, 2025

Slightly wrong data. The images got published just as I pulled them.

Before and after PR1427:

rich@vancouver:~$ docker run --rm mcr.microsoft.com/dotnet-buildtools/prereqs@sha256:5ded7a1ff99093aa48ecea8649aa75aadb007c98105d664d79878fa0a7020e10 locale
LANG=
LC_CTYPE="POSIX"
LC_NUMERIC="POSIX"
LC_TIME="POSIX"
LC_COLLATE="POSIX"
LC_MONETARY="POSIX"
LC_MESSAGES="POSIX"
LC_PAPER="POSIX"
LC_NAME="POSIX"
LC_ADDRESS="POSIX"
LC_TELEPHONE="POSIX"
LC_MEASUREMENT="POSIX"
LC_IDENTIFICATION="POSIX"
LC_ALL=
rich@vancouver:~$ docker run --rm mcr.microsoft.com/dotnet-buildtools/prereqs@sha256:a9b9de612f54c6e34ad273ab85aee01ff0cdb456bd6c7a4d3475c26b2068f4ac locale
LANG=en_US.UTF-8
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_PAPER="en_US.UTF-8"
LC_NAME="en_US.UTF-8"
LC_ADDRESS="en_US.UTF-8"
LC_TELEPHONE="en_US.UTF-8"
LC_MEASUREMENT="en_US.UTF-8"
LC_IDENTIFICATION="en_US.UTF-8"
LC_ALL=en_US.UTF-8

This aligns with the initial investigation by @tarekgh that language info is not set.

The philosophy that we take the images as-is sounds good. We can make a tracking issue for AL4 if we think this should be changed.

I'll re-kickoff CI when the new image gets published to ensure we get a clean run.

@tarekgh
Copy link
Member

tarekgh commented May 10, 2025

The philosophy that we take the images as-is sounds good. We can make a tracking issue for AL4 if we think this should be changed.

Sounds good to me. We need two issues one for the image and one for the test :-)

I'll go ahead and revert the disabling test case too here.

@richlander
Copy link
Member Author

Sorry ... what are the issues?

I thought that keeping the test disabled would be the correct action. We cannot merge this PR otherwise.

@tarekgh
Copy link
Member

tarekgh commented May 10, 2025

ok:

  • we'll not change the locale setting on the image. will continue having the default posix settings
  • We'll disable the failing test to unblock this PR
  • We'll create issue track fixing and enabling the test.

@richlander
Copy link
Member Author

richlander commented May 10, 2025

More context. Looks like AL3 closely matches Fedora, which we also support (obviously).

rich@vancouver:~$ docker run --rm mcr.microsoft.com/azurelinux/base/core:3.0 locale
LANG=
LC_CTYPE="POSIX"
LC_NUMERIC="POSIX"
LC_TIME="POSIX"
LC_COLLATE="POSIX"
LC_MONETARY="POSIX"
LC_MESSAGES="POSIX"
LC_PAPER="POSIX"
LC_NAME="POSIX"
LC_ADDRESS="POSIX"
LC_TELEPHONE="POSIX"
LC_MEASUREMENT="POSIX"
LC_IDENTIFICATION="POSIX"
LC_ALL=
rich@vancouver:~$ docker run --rm fedora locale
LANG=
LC_CTYPE="POSIX"
LC_NUMERIC="POSIX"
LC_TIME="POSIX"
LC_COLLATE="POSIX"
LC_MONETARY="POSIX"
LC_MESSAGES="POSIX"
LC_PAPER="POSIX"
LC_NAME="POSIX"
LC_ADDRESS="POSIX"
LC_TELEPHONE="POSIX"
LC_MEASUREMENT="POSIX"
LC_IDENTIFICATION="POSIX"
LC_ALL=

This locks in the idea that we should be testing the distros unmodified.

@richlander
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop-linux

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@richlander
Copy link
Member Author

Respin per rebuild: dotnet/versions@911f03a

@richlander
Copy link
Member Author

richlander commented May 10, 2025

Looks like the tests are clean.

  • BA is green with no unique failures reported
  • Azure Linux outerloop failures appear to match failures in CentOS and Alpine

This is a big change with significant implications. I'll wait for someone else to "OK" merging (or just merge).

I'll port this change to release branches next.

@@ -3165,7 +3165,7 @@ public void ValidCases_ExternalURI(object param0, object param1, object param2,
//[Variation("No Import/Include, NullResolver", Pri = 0, Params = new object[] { "Bug382198.xsl", "fruits.xml", "bug382198.txt", "NullResolver", true })]
[InlineData("Bug382198.xsl", "fruits.xml", "bug382198.txt", "NullResolver", true, "IXPathNavigable")]
[InlineData("Bug382198.xsl", "fruits.xml", "bug382198.txt", "NullResolver", true, "XmlReader")]
[InlineData("bug93189.xsl", "bug93189.xml", "bug93189.xml", "NullResolver", true, "XmlReader")]
// [InlineData("bug93189.xsl", "bug93189.xml", "bug93189.xml", "NullResolver", true, "XmlReader")]
Copy link
Member

Choose a reason for hiding this comment

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

This should have a link to an issue that tracks fixing the test.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I have created #115455

@richlander richlander merged commit d2c5618 into dotnet:main May 12, 2025
83 of 85 checks passed
@richlander richlander deleted the al3-ci branch May 12, 2025 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants