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

Add linux mariner to rolling builds for libraries tests #57322

Merged
merged 3 commits into from
Aug 17, 2021
Merged

Conversation

safern
Copy link
Member

@safern safern commented Aug 12, 2021

Fixes: #54478

@ghost
Copy link

ghost commented Aug 12, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes: #54478

Author: safern
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@safern
Copy link
Member Author

safern commented Aug 12, 2021

Sample build including this queue here: https://dev.azure.com/dnceng/public/_build/results?buildId=1291084&view=results

@safern
Copy link
Member Author

safern commented Aug 13, 2021

@bartonjs it seems like this crypto test is failing on Linux Mariner:

I guess OpenSsl is present but for some reason the runtime property is returning false, what is your advise? Just disable it for this distro?

Copy link
Member

@danmoseley danmoseley left a comment

Choose a reason for hiding this comment

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

Thanks

@bartonjs
Copy link
Member

what is your advise? Just disable it for this distro?

Seems like a reasonable starting place. OpenSSL on Mariner must be built with one or both of nopoly1305 or nochacha. When things get more complicated we'll need to figure out how to better determine that in the tests (hopefully via a route other than just duplicating the product logic).

            else if (PlatformDetection.IsAndroid)
            {
                // Android with API Level 28 is the minimum API Level support for ChaChaPoly1305.
                expectedIsSupported = OperatingSystem.IsAndroidVersionAtLeast(28);
            }
+           else if (PlatformDetection.IsMariner)
+           {
+               // OpenSSL is present, and a high enough version,
+               // but the distro build options turned off ChaCha/Poly.
+           }
            else if (PlatformDetection.OpenSslPresentOnSystem &&
                (PlatformDetection.IsOSX || PlatformDetection.IsOpenSslSupported))
            {

Or something like that.

@danmoseley
Copy link
Member

danmoseley commented Aug 13, 2021

I wonder whether there's some assert we can add in the tests so that if Mariner changes its configuration, we can enable the tests again.

A minor thing though -- seems completely reasonable as stated above.

@bartonjs
Copy link
Member

I wonder whether there's some assert we can add in the tests so that if Mariner changes its configuration, we can enable the tests again.

Well, the diff I proposed would leave Mariner running the test, but in the "expect: false" case, then the test would fail with "actual: true" and we'd be in what I called "when things get more complicated". So we already have that assert/action-signal 😄.

For this feature, the mainline tests only run when the platform says it's supported, this particular test just checks that the platform says things are (or aren't) supported as we expect them to be.

@safern
Copy link
Member Author

safern commented Aug 13, 2021

Sounds good, I'll adjust accordingly.

@safern
Copy link
Member Author

safern commented Aug 13, 2021

@safern
Copy link
Member Author

safern commented Aug 14, 2021

Looks like some tests are failing on Mono. I will open issues and disable them.

cc: @steveisok @akoeplinger

@safern
Copy link
Member Author

safern commented Aug 16, 2021

@safern
Copy link
Member Author

safern commented Aug 17, 2021

The linux mariner build is now green. Merging as failures are unrelated.

@safern safern merged commit 380cfc9 into main Aug 17, 2021
@safern safern deleted the AddLinuxMariner branch August 17, 2021 01:01
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Mariner Linux distro to CI validation
5 participants