search

dotnet / docker-tools

This is a repo to house some common tools for our various docker repos.
MIT License
122 stars 46 forks source link

Pipeline Changes for Pushing Mirror and Build-Staging Repositories to Separate ACR #1309

Closed ellahathaway closed 3 months ago

ellahathaway commented 4 months ago

Related to https://github.com/dotnet/dotnet-docker-internal/issues/5024 and https://github.com/dotnet/dotnet-docker-internal/issues/5026

This PR updates the ymls to use the appropriate ACR server and serviceConnectionName for the given repo. The changes were made to the following ymls:

ImageBuilder pipeline run Windows tests pipeline run Nightly pipeline run Cleanup pipeline run

dotnet-issue-labeler[bot] commented 4 months ago

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

MichaelSimons commented 3 months ago

Given @ellahathaway is offsite until Friday and then the short time-frame until patch Tuesday, it feels like we should defer merging until after Patch Tuesday. Thoughts @mthalman and @lbussell?

lbussell commented 3 months ago

@michaelsimons I am OK with waiting since there is no immediate forcing function for these changes.

However, that would also mean we can't merge any other ImageBuilder changes all week, since the latest version requires these pipeline changes.

So, if we want to wait until after PT, we should either revert the source code changes, or adopt some version of these pipeline changes that don't push to the staging ACR. At which point we may as well just try out the whole set of changes.

I made the point in https://github.com/dotnet/docker-tools/pull/1295#issuecomment-2145579829 that we have a fair amount of time until Patch Tuesday. If @ellahathaway would like to be the one on point to fix potential issues from this change then like I said I'm OK with waiting.

ellahathaway commented 3 months ago

If @ellahathaway would like to be the one on point to fix potential issues from this change then like I said I'm OK with waiting.

I'm happy to fix any potential issues that might arise, but like Michael mentioned, I won't be able to address anything until Friday of this week.

That said, I feel pretty confident in the changes in this PR (tested thoroughly with the specified pipelines in the PR description), but at the same time, I am not privy to all the repercussions that merging these changes might have.

Regardless, I'll take the lead in fixing any issues on Friday for whatever decision is made.

mthalman commented 3 months ago

We might as well wait until after PT since these changes aren't urgent. I don't think there's a need to revert the other PR at this time. We can revert it if there's an Image Builder change we need to make in the mean time.

mthalman commented 3 months ago

Build is blocked. Need this fix: https://github.com/dotnet/docker-tools/pull/1330

ellahathaway commented 3 months ago

Build failed with:

Unhandled exception: Azure.Identity.CredentialUnavailableException: DefaultAzureCredential failed to retrieve a token from the included credentials. on this line. The line was brought in by https://github.com/dotnet/docker-tools/pull/1321. cc @mthalman