Arcade: [darc/maestro++] Build shouldn't silently finish if it's supposed to publish to a channel that doesn't have a publishing implementation set up.

[agocke]

• Is this issue blocking (no)
• Is this issue causing unreasonable pain (yes)

https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3441625

It looks like this PR https://github.com/dotnet/toolset/pull/4223 got a maestro update with a bad build.

[nguerrera]

https://github.com/dotnet/toolset/pull/4223#issuecomment-582131750

[nguerrera]

@wtgodbe

[JohnTortugo]

That build doesn't have a default channel set in Maestro++. Which channel should it go? ".NET Core 5 Dev"?

[agocke]

Sorry, wrong build, check https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3439156

[agocke]

(949) https://github.com/dotnet/roslyn @ master-vs-deps -> .NET Core 5 Dev

[agocke]

(Check the Publish-build-artifacts pipeline in "Build and Test Official Build")

[agocke]

OK, I think that log may be showing something different -- I have no idea what, though.

If the problem is that the original build didn't have a feed subscription when it started, but does now, is Maestro pulling it even though the build doesn't exist in the channel?

[wtgodbe]

I think the issue is that the Arcade version here is old enough that it doesn't know about the 3.1.2xx channel, among others. Notice how many fewer 'publishing' stages there are than, say, https://dev.azure.com/dnceng/internal/_build/results?buildId=507142

[agocke]

So then why is Maestro assuming this is a valid build in the channel? Either Arcade didn't publish it to the channel and Maestro shouldn't try to use it, or there's a bug in Arcade, or both.

[nguerrera]

I agree, but on the other hand, how much longer would this have gone unnoticed if it just behaved as if there was no build?

[riarenas]

Can you confirm which build you were expecting to publish to which channel?

The build provided in the issue description: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3441625 found this default channel:
https://github.com/dotnet/roslyn@release/dev16.5-preview3-vs-deps => (558) .NET Core SDK 3.1.2xx however, is using an Arcade version that doesn't have the YAML templates to publish to that channel as Will pointed out.

This is a state that we can potentially fail the build when we see it: IE we found a default channel, but there's no channel publish implementation for that default channel you told us about.

[riarenas]

If this is not the issue, but actually that https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3439156 did not publish all the expected packages in this stage: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3439156&view=logs&j=0b850c37-f3ae-5bb9-a9fb-4f1e0fce2747&t=50d41e72-ce5e-50a3-efc7-c28c030be91c

Then that's something else to investigate.

[agocke]

OK, let's reset to the issue that started everything. Toolset got a maestro PR trying to take a build that doesn't exist in that channel.

Why?

[wtgodbe]

OK, let's reset to the issue that started everything. Toolset got a maestro PR trying to take a build that doesn't exist in that channel.

My understanding is that the build _does_ exist in the channel, it just didn't publish any assets to the AzDO feed associated with that channel.

[JohnTortugo]

About build 20200204.2 :

1. The repo + branch was configured in BAR to publish to a channel 558
2. The repo + branch were using an outdated version of Arcade eng\common which didn't have YAML files to publish to channel 558
3. Once the build finished it understood that the build should have executed the YAMLs to publish to channel 558
4. Maestro triggers the subscriptions for repo + channel 558
5. PR updates fail because the assets weren't published, since no YAML was executed to publish them

[agocke]

The problem here seems to be that the operations are not atomic. If there are no assets to publish to the channel, then the build shouldn't be added to the channel, regardless of configuration. It should probably fail publishing, instead of skipping it.

As to fixing this, I presume we have to take a new build of Arcade into this branch? This is a release branch so we'll have to do that manually.

[JohnTortugo]

For build 20200203.12 :

• The repo + branch has default channel set to -> 949
• There is no subscription from Roslyn on channel 949
• No PR was created

[riarenas]

I updated the title. I agree that this is a state that shouldn't be difficult to detect and guard against.

This will continue happening for Roslyn builds from that branch until a new arcade is picked up from the ".net 3 eng" channel.

[JohnTortugo]

It's what Ricardo said :-)

[agocke]

OK, in that case we need a build to merge into the release branch. It looks like https://github.com/dotnet/arcade/issues/4748 has not been marked fixed.

How do I introduce a new arcade into the branch without breaking the signed build?

[riarenas]

We haven't done any publishing changes in the release/3.x branch of arcade in a while. The cause for #4748 was already fixed when @mattgal created the missing variable group in DevDiv. The remaining discussion is about how to prevent future similar breakages.

Updating through triggering the subscription from arcade to the problem branch, or by doing a darc update-dependencies --source-repo arcade --channel ".NET 3 Eng" should get you the right Arcade.

[agocke]

We don't have an arcade subscription for release branches, as they require M2 or QB approval for check-in.

How would I do a one-off insertion?

[riarenas]

From the branch where you want the updates:

1. eng/common/darc-init.ps1 -darcVersion 1.1.0-beta.20081.2
2. darc update-dependencies --source-repo arcade --channel ".NET 3 Eng"

That will add the updates to your workspace and you can create a PR.

For further validation, You can additionally push a branch with those changes and:

1. Set up a test default channel: darc add-default-channel --repo https://github.com/dotnet/roslyn" --branch "branch you want to test" --channel "General Testing"
2. Run an official build from that branch to check that publishing works as expected and that the "General Testing publish" stage publishes everything.

[agocke]

C:\Users\angocke\code\roslyn\artifacts\tmp\Debug\trrapovw.0kb\restore.csproj : error NU1211: Project restore must have a single package reference(s).
C:\Users\angocke\code\roslyn\artifacts\tmp\Debug\trrapovw.0kb\restore.csproj : error NU1212: Invalid project-package combination for Microsoft.CodeQuality.Analyzers 3.0.0-beta2.20059.3. DotnetToolReference project style can only contain references of the DotnetTool type
C:\Users\angocke\code\roslyn\artifacts\tmp\Debug\trrapovw.0kb\restore.csproj : error NU1212: Invalid project-package combination for Microsoft.NetCore.Analyzers 3.0.0-beta2.20059.3. DotnetToolReference project style can only contain references of the DotnetTool type
C:\Users\angocke\code\roslyn\artifacts\tmp\Debug\trrapovw.0kb\restore.csproj : error NU1212: Invalid project-package combination for Roslyn.Diagnostics.Analyzers 3.0.0-beta2.20059.3. DotnetToolReference project style can only contain references of the DotnetTool type
C:\Users\angocke\code\roslyn\artifacts\tmp\Debug\trrapovw.0kb\restore.csproj : error NU1212: Invalid project-package combination for Microsoft.VisualStudio.Threading.Analyzers 16.4.16. DotnetToolReference project style can only contain references of the DotnetTool type
C:\Users\angocke\code\roslyn\artifacts\tmp\Debug\trrapovw.0kb\restore.csproj : error NU1212: Invalid project-package combination for Microsoft.CodeAnalysis.CSharp.CodeStyle 3.5.0-beta3-20078-04. DotnetToolReference project style can only contain references of the DotnetTool type

[nguerrera]

Do you have a PR with the diff we can check? Did this include a new SDK? @nkolev92 thoughts on NU1212?

[nguerrera]

Oh that must be a dotnet tool install.

[nguerrera]

Do you have the rest of the log so we can see the context?

[agocke]

Right this is the full log of running darc init

[nguerrera]

Oh, I thought this was a snippet from the roslyn build after step 2. 🤦‍♂

[nguerrera]

Hmm, works for me. This looks like the issue with the temp restore project being inside the repo and getting a bad Directory.Build.props. Is the TEMP environment variable customized where you've run darc-init?

[riarenas]

Can you try running the darc init from a clone of the master branch of arcade and see if that still fails? Haven't seen that error before and I'll try to get a repro, but it might be just another side effect of the old arcade setup that branch is using.

[agocke]

That seemed to work, but now I get:

Looking up latest build of https://github.com/dotnet/arcade on .NET 3 Eng
Updating 'Microsoft.DotNet.Arcade.Sdk': '5.0.0-beta.20080.4' => '1.0.0-beta.20077.3' (from build '20200127.3' of 'https://github.com/dotnet/arcade')
Checking for coherency updates...
fail: Microsoft.DotNet.Darc.Operations.Operation[0]
      Error: Failed to update dependencies to channel .NET 3 Eng
Microsoft.DotNet.Maestro.Client.RestApiException`1[Microsoft.DotNet.Maestro.Client.Models.ApiError]: The response contained an invalid status code 404 Not Found

Body:
   at Microsoft.DotNet.Maestro.Client.Assets.OnListAssetsFailed(Request req, Response res) in /_/src/Maestro/Client/src/Generated/Assets.cs:line 215
   at Microsoft.DotNet.Maestro.Client.Assets.ListAssetsPageAsync(Nullable`1 buildId, Nullable`1 loadLocations, String name, Nullable`1 nonShipping, Nullable`1 page, Nullable`1 perPage, String version, CancellationToken cancellationToken) in /_/src/Maestro/Client/src/Generated/Assets.cs:line 180
   at Microsoft.DotNet.Maestro.Client.Assets.<>c__DisplayClass6_0.<<ListAssetsAsync>g__GetPages|0>d.MoveNext() in /_/src/Maestro/Client/src/Generated/Assets.cs:line 95
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.DotNet.Maestro.Client.Assets.<>c__DisplayClass6_0.<<ListAssetsAsync>g__GetPages|0>d.System.Threading.Tasks.Sources.IValueTaskSource<System.Boolean>.GetResult(Int16 token)
   at Azure.AsyncPageable`1.GetAsyncEnumerator(CancellationToken cancellationToken)+MoveNext()
   at Azure.AsyncPageable`1.GetAsyncEnumerator(CancellationToken cancellationToken)+MoveNext()
   at Azure.AsyncPageable`1.GetAsyncEnumerator(CancellationToken cancellationToken)+System.Threading.Tasks.Sources.IValueTaskSource<System.Boolean>.GetResult()
   at Microsoft.DotNet.Maestro.Client.AsyncEnumerable.ToListAsync[T](IAsyncEnumerable`1 that, CancellationToken cancellationToken) in /_/src/Maestro/Client/src/Generated/PagedResponse.cs:line 15
   at Microsoft.DotNet.Maestro.Client.AsyncEnumerable.ToListAsync[T](IAsyncEnumerable`1 that, CancellationToken cancellationToken) in /_/src/Maestro/Client/src/Generated/PagedResponse.cs:line 15
   at Microsoft.DotNet.DarcLib.MaestroApiBarClient.GetAssetsAsync(String name, String version, Nullable`1 buildId, Nullable`1 nonShipping) in /_/src/Microsoft.DotNet.Darc/src/DarcLib/MaestroApiBarClient.cs:line 308
   at Microsoft.DotNet.DarcLib.Remote.AddAssetLocationToDependenciesAsync(IEnumerable`1 dependencies) in /_/src/Microsoft.DotNet.Darc/src/DarcLib/Actions/Remote.cs:line 1157
   at Microsoft.DotNet.DarcLib.Local.UpdateDependenciesAsync(List`1 dependencies, IRemoteFactory remoteFactory) in /_/src/Microsoft.DotNet.Darc/src/DarcLib/Actions/Local.cs:line 64
   at Microsoft.DotNet.Darc.Operations.UpdateDependenciesOperation.ExecuteAsync() in /_/src/Microsoft.DotNet.Darc/src/Darc/Operations/UpdateDependenciesOperation.cs:line 226

[riarenas]

There's an issue with the current latest darc. I can link the issue we have tracking this tomorrow morning, but the version I suggested "1.1.0-beta.20081.2" should not experience this.

Can you confirm whether you're seeing the issue with that version? (to raise priority on the other issue if so?)

[agocke]

That worked, thanks. Created a PR with the changes.