[release/2.1] Update branding to 2.1.28 #31547

dougbu · 2021-04-05T21:15:00Z

update branding to 2.1.28
bump $(MicrosoftNETCoreAppPackageVersion)
- version flows into $(MicrosoftNETCoreApp21PackageVersion) automatically
- cherry-picked from [release/2.1] Correct and streamline internal builds #30729 internal branch
get SharedFx.UnitTests working after rebranding
- skip SharedFxTests.BaselineTest(...) between rebranding and baseline updates
  - previous runtime is not available in this window
- reduce retries in RetryHelper
  - download will usually fail repeatedly if it fails even once
- previous maximum duration (105 min.) was greater than the job timeout
avoid NETSDK1023 warnings
- eng/targets/CSharp.Common.props adds packages removed from project
- cherry-picked from [release/2.1] Correct and streamline internal builds #30729 internal branch
  nit: remove unused GetTotalRetriesUsed() method
use Ubuntu 18.04 build agents

- version flows into `$(MicrosoftNETCoreApp21PackageVersion)` automatically - cherry-picked from #30729 internal branch

TanayParikh

Overall LGTM, not sure if other changes are required to make this official.

TanayParikh · 2021-04-05T21:20:35Z

eng/PatchConfig.props

@@ -100,4 +100,8 @@ Later on, this will be checked using this condition:
      @aspnet/signalr-protocol-msgpack;
    </PackagesInPatch>
  </PropertyGroup>
+  <PropertyGroup Condition=" '$(VersionPrefix)' == '2.1.28' ">


Above changes are for 2.1.26 and this is .28 is that intentional?

Ah so this is the AspNetCore package version https://github.com/dotnet/aspnetcore/pull/31547/files#diff-e4bed5b736f205989dd4fdb6d78acfe9126577983d325d378ce91794d74e63c8R5

Yes, my changes in build/dependencies.props are just to use the packages that are already public in rolling and PR builds. We'll move to 2.1.27 when updating the baselines on Patch Tuesday.

dougbu · 2021-04-05T23:06:39Z

Thanks @TanayParikh I'll get this in tomorrow, when the branches are slated to open.

dougbu · 2021-04-06T01:03:40Z

@wtgodbe @JunTaoLuo not sure if the commits I just added are needed for 3.1 or 5.0. Decided they made sense here because the Docker images we were using were very old.

But, this bulks up the PR. Let me know if you'd prefer I did some parts in separate PRs.

dougbu · 2021-04-06T17:55:40Z

Holding for a actions/runner-images#3120 fix (or at least a workaround). Leaving this alone to make it easier for Azure / GitHub Actions team to see the problem.

Will open another PR without Ubuntu upgrade in a bit…

dougbu · 2021-04-07T21:03:11Z

/fyi this may take a bit more time because

https://status.dev.azure.com/_event/235501110
Saw one test that likely won't be fixed by the environment variable workaround. Expect to need another bit from my [release/2.1] Correct and streamline internal builds #30729 internal branch.

dougbu · 2021-04-08T00:26:25Z

build/tools/docker/ubuntu.18.04/Dockerfile

@@ -29,7 +30,8 @@ RUN apt-get update && \
        lldb-3.6 \
        wget && \


If this long list can be pared down because the Ubuntu 18.04 image contains them already, I'll do that in #30729

dougbu · 2021-04-08T00:28:43Z

src/Templating/test/Templates.Test/CdnScriptTagTests.cs

@@ -96,6 +96,36 @@ public async Task FallbackSrcContent_Matches_CDNContent(ScriptTag scriptTag)
            Assert.Equal(RemoveLineEndings(cdnContent), RemoveLineEndings(fallbackSrcContent));
        }

+        class RetryHandler : DelegatingHandler


Surprisingly, it's the tests in src/Identity/test/Identity.Test/CdnScriptTaghelperTests.cs that have been flaky in this PR. The RetryHelper should be helpful in any case.

- skip `SharedFxTests.BaselineTest(...)` between rebranding and baseline updates - previous runtime is not available in this window - reduce retries in `RetryHelper` - download will usually fail repeatedly if it fails even once - previous maximum duration (105 min.) was greater than the job timeout - avoid NETSDK1023 warnings - eng/targets/CSharp.Common.props adds packages removed from project - create archives based on one that actually exists between rebranding and baseline updates - mostly cherry-picked from #30729 internal branch nit: remove unused `GetTotalRetriesUsed()` method

- set locale consistently on all platforms - default locale on newer agents is unloved `C.UTF-8`

- run `locale-gen` to avoid "setlocale: LC_ALL: cannot change locale" and similar - needs `locales` package - move to still-supported Ubuntu 18.04 image - image needs slightly-newer `libicu60`, `clang` and `lldb` packages - rename Dockerfile to match - cherry-picked from #30729 internal branch for Ubuntu image update nit: set locale-related environment variables to match Python default

- avoid MSB4011 warnings (about repeated Directory.Build.targets imports) - place `$DOTNET_HOME` beside repo root, not under it - cherry-picked from #30729 internal branch for image update nits: - use current Docker image for 2.1 alpine - set `$LANGUAGE` with other locale-related environment variables

- minor change shouldn't affect submodule build but best to keep up

- apply efe9b95 to similar template test

dougbu · 2021-04-08T06:52:11Z

/fyi I've been testing internal builds and addressing failures there. Test failures in PR validation builds appear to be flaky tests

extra debug information for IdentityUI_ScriptTags_SubresourceIntegrityCheck tests didn't help much since SSL failure throws and IdentityUI_ScriptTags_SubresourceIntegrityCheck.RetryHandler doesn't catch the Exception
Sockets.FunctionalTest/netcoreapp2.1 test group fails occasionally during teardown

dougbu · 2021-04-08T20:50:13Z

Still blocked on Microsoft.AspNetCore.Identity.Test.CdnScriptTagTests.IdentityUI_ScriptTags_SubresourceIntegrityCheck failures.


Error message
System.Net.Http.HttpRequestException : An error occurred while sending the request.
---- System.Net.WebException : The request was aborted: Could not create SSL/TLS secure channel.


Stack trace
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Identity.Test.CdnScriptTagTests.RetryHandler.<SendAsync>d__2.MoveNext() in /_/src/Identity/test/Identity.Test/CdnScriptTaghelperTests.cs:line 81
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Net.Http.HttpClient.<FinishSendAsyncUnbuffered>d__59.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Net.Http.HttpClient.<FinishGetStreamAsync>d__33.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Identity.Test.CdnScriptTagTests.<IdentityUI_ScriptTags_SubresourceIntegrityCheck>d__2.MoveNext() in /_/src/Identity/test/Identity.Test/CdnScriptTaghelperTests.cs:line 52
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
----- Inner Stack Trace -----
   at System.Net.HttpWebRequest.EndGetResponse(IAsyncResult asyncResult)
   at System.Net.Http.HttpClientHandler.GetResponseCallback(IAsyncResult ar)

I'm working to duplicate @javiercn's 3368ea6 / #13646 fix but can't get https://github.com/dotnet/aspnetcore/blob/release/3.1/src/Identity/test/Identity.Test/RetryHandler.cs#L57 to compile in this branch. Suspect updating https://github.com/dotnet/aspnetcore/blob/release/2.1/eng/targets/CSharp.Common.props#L4 to 8.0 or changing https://github.com/dotnet/aspnetcore/blob/release/2.1/src/Identity/test/Identity.Test/Microsoft.AspNetCore.Identity.Test.csproj to override that default would be sufficient. If increasing the C# language version is a breaking change, I'd do the second. Thoughts @dotnet/aspnet-build❔

dougbu · 2021-04-08T20:59:32Z

/btw https://dev.azure.com/dnceng/internal/_build/results?buildId=1077719 confirmed our official builds (where we don't test 'natch) will succeed once this is in.

@javiercn

- based on @javiercn's 3368ea6 / #13646 fix - avoid `TimeSpan` arithmetic and did not match refactoring done in 'release/3.1' - c# 8.0 is not supported in this branch

Pilchie · 2021-04-08T23:04:23Z

@karelz @scalablecory - any ideas on the failure above in Details.

It's coming from here, and only seems to be failing in 2.1.

@dougbu I don't think you can update to compile with C# 8, because that requires a .NET SDK of 3.0 or later. What is the error you are seeing?

dougbu · 2021-04-08T23:17:08Z

@dougbu I don't think you can update to compile with C# 8, because that requires a .NET SDK of 3.0 or later. What is the error you are seeing?

In local runs with C# 7.2, I saw

CdnScriptTaghelperTests.cs(105,51): error CS0019: Operator '*' cannot be applied to operands of type 'TimeSpan' and 'int' [...\src\Identity\test\Identity.Test\Microsoft.AspNetCore.Identity.Test.csproj]

To work around this, I ended up multiplying an int by 2 and creating a TimeSpan each time through the loop. See
https://github.com/dotnet/aspnetcore/pull/31547/files#diff-3b3c01f3096f00f0578c36dc3234ed37af00a870920fe3d79eafe8292f4cb47fR114
and
https://github.com/dotnet/aspnetcore/pull/31547/files#diff-3b3c01f3096f00f0578c36dc3234ed37af00a870920fe3d79eafe8292f4cb47fR139-R140

dougbu · 2021-04-08T23:24:55Z

only seems to be failing in 2.1

Successful test results don't show much information but could at least see success of IdentityUI_ScriptTags_SubresourceIntegrityCheck tests at https://dev.azure.com/dnceng/public/_build/results?buildId=1078781&view=ms.vss-test-web.build-test-results-tab. That's for 'main', where the test is a [Theory] and somewhat refactored but otherwise much like we have here.

Pilchie · 2021-04-08T23:33:11Z

To be pendantic, that's not a C# compiler change, that's about the set of operators on System.TimeSpan. A static TimeSpan operator*(TimeSpan left, int right) have been added after .NET Core 2.1.

dougbu · 2021-04-08T23:38:29Z

Thanks for the additional context @Pilchie; I appreciate the pedantry.

Also asked whether this might be specific to the BuildPool.Server.Amd64.VS2017.Open queue in https://teams.microsoft.com/l/message/19:[email protected]/1617924854323?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=4d73664c-9f2f-450d-82a5-c2f02756606d&parentMessageId=1617924854323&teamName=.NET%20Core%20Eng%20Services%20Partners&channelName=First%20Responders&createdTime=1617924854323

dougbu · 2021-04-09T01:31:17Z

@karelz @scalablecory @Pilchie

Problem might be specific to .NET Framework runs and look relatively new. I saw it succeed on Monday in this branch (https://dev.azure.com/dnceng/public/_build/results?buildId=1073196) but it looked flaky in my internal builds for #30729 last week (https://dev.azure.com/dnceng/internal/_build?definitionId=21&branchFilter=150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570%2C150570).

I'm going to try skipping on .NET Framework. But, would very much like to know why this test went South.

- failures appear specific to that platform - leave src/Templating/test/Templates.Test/CdnScriptTagTests.cs unchanged - test project does not target .NET Framework

dougbu · 2021-04-09T01:47:11Z

src/Identity/test/Identity.Test/CdnScriptTaghelperTests.cs

-        [Fact]
+        // Because this test runs on .NET Core and seems reliable there, likely not worth running on .NET Framework.
+        [ConditionalFact]
+        [FrameworkSkipCondition(RuntimeFrameworks.CLR, SkipReason = "At least flaky on .NET Framework.")]


@Pilchie @dotnet/aspnet-build thoughts about filing a follow-up issue❔ I'm leaning toward no because the test is about external resources. Also realized the test runs fine on Linux and macOS.

I think we should at least file an issue about figuring out why the test failed

If that's in dotnet/runtime, so be it

@wtgodbe I agree and wasn't really questioning that. My question was more whether we needed an issue about removing the [FrameworkSkipCondition(...)]. Hearing none 😃

@karelz @scalablecory where should I file an issue about the .NET Framework HttpClient class and the underlying SSL libraries❔

@dougbu I believe we do not run dotnet/runtime tests on .NET Framework anymore. Before we stopped to run them, we saw quite some flakiness on .NET Framework in networking. I wonder if this is similar ...
If you're able to isolate it to HttpClient/SslStream repro, we can take a look -- I would start email thread with our team as a starting point.

dougbu · 2021-04-09T04:41:09Z

CI is green now 🎉 Just waiting for a response on adding an issue tracking restoring the skipped .NET Framework test. (Would link to the issue in the new SkipReason.)

Just checked and MusicStore.csproj targets only .NET Core in 'release/3.1' and 'release/5.0', which might explain why we haven't seen failures in those branches.

dougbu added 2 commits April 5, 2021 14:03

Update branding to 2.1.28

f6b2202

Bump $(MicrosoftNETCoreAppPackageVersion)

b354b4d

- version flows into `$(MicrosoftNETCoreApp21PackageVersion)` automatically - cherry-picked from #30729 internal branch

dougbu added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework tell-mode Indicates a PR which is being merged during tell-mode labels Apr 5, 2021

dougbu requested a review from a team April 5, 2021 21:15

dougbu added this to the 2.1.28 milestone Apr 5, 2021

TanayParikh approved these changes Apr 5, 2021

View reviewed changes

wtgodbe approved these changes Apr 6, 2021

View reviewed changes

dougbu mentioned this pull request Apr 6, 2021

Ubuntu 18.04 agents have unexpected default locale actions/runner-images#3120

Closed

7 tasks

dougbu mentioned this pull request Apr 6, 2021

[release/2.1] Update branding to 2.1.28 II #31569

Closed

JunTaoLuo approved these changes Apr 6, 2021

View reviewed changes

dougbu force-pushed the dougbu/branding.2.1.28 branch from 471a5ce to fde8f9a Compare April 7, 2021 20:47

dougbu force-pushed the dougbu/branding.2.1.28 branch from fde8f9a to f0cf5f3 Compare April 8, 2021 00:22

dougbu commented Apr 8, 2021

View reviewed changes

dougbu added 8 commits April 7, 2021 21:45

Use Ubuntu 18.04 build agents

6d7bb39

- set locale consistently on all platforms - default locale on newer agents is unloved `C.UTF-8`

Remove obsolete Groovy code

ab356a7

Pick up latest EF sources

63ee84c

- minor change shouldn't affect submodule build but best to keep up

Improve the other CDN tests

780c75b

- apply efe9b95 to similar template test

Add debug output to CDN tests

10e5c3b

dougbu force-pushed the dougbu/branding.2.1.28 branch from 3d58e38 to 10e5c3b Compare April 8, 2021 04:49

!fixup! More for 'Stop using Ubuntu 14.04'

147d5ac

Back-port test reliability fix

5d3c985

- based on @javiercn's 3368ea6 / #13646 fix - avoid `TimeSpan` arithmetic and did not match refactoring done in 'release/3.1' - c# 8.0 is not supported in this branch

JunTaoLuo approved these changes Apr 8, 2021

View reviewed changes

wtgodbe approved these changes Apr 8, 2021

View reviewed changes

Skip CdnScriptTaghelperTests on .NET Framework

7c41d9f

- failures appear specific to that platform - leave src/Templating/test/Templates.Test/CdnScriptTagTests.cs unchanged - test project does not target .NET Framework

dougbu commented Apr 9, 2021

View reviewed changes

dougbu merged commit 2c42322 into release/2.1 Apr 9, 2021

dougbu deleted the dougbu/branding.2.1.28 branch April 9, 2021 16:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[release/2.1] Update branding to 2.1.28 #31547

[release/2.1] Update branding to 2.1.28 #31547

dougbu commented Apr 5, 2021

TanayParikh left a comment

TanayParikh Apr 5, 2021

TanayParikh Apr 5, 2021

dougbu Apr 5, 2021

dougbu commented Apr 5, 2021

dougbu commented Apr 6, 2021

dougbu commented Apr 6, 2021

dougbu commented Apr 7, 2021

dougbu Apr 8, 2021

dougbu Apr 8, 2021

dougbu commented Apr 8, 2021

dougbu commented Apr 8, 2021 •

edited

Loading

dougbu commented Apr 8, 2021

Pilchie commented Apr 8, 2021

dougbu commented Apr 8, 2021

dougbu commented Apr 8, 2021

Pilchie commented Apr 8, 2021

dougbu commented Apr 8, 2021

dougbu commented Apr 9, 2021

dougbu Apr 9, 2021

wtgodbe Apr 9, 2021

wtgodbe Apr 9, 2021

dougbu Apr 9, 2021

karelz Apr 13, 2021

dougbu commented Apr 9, 2021

		@@ -29,7 +30,8 @@ RUN apt-get update && \
		lldb-3.6 \
		wget && \

[release/2.1] Update branding to 2.1.28 #31547

[release/2.1] Update branding to 2.1.28 #31547

Conversation

dougbu commented Apr 5, 2021

TanayParikh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dougbu commented Apr 5, 2021

dougbu commented Apr 6, 2021

dougbu commented Apr 6, 2021

dougbu commented Apr 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dougbu commented Apr 8, 2021

dougbu commented Apr 8, 2021 • edited Loading

dougbu commented Apr 8, 2021

Pilchie commented Apr 8, 2021

dougbu commented Apr 8, 2021

dougbu commented Apr 8, 2021

Pilchie commented Apr 8, 2021

dougbu commented Apr 8, 2021

dougbu commented Apr 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dougbu commented Apr 9, 2021

dougbu commented Apr 8, 2021 •

edited

Loading