dotnet: Use dotnet publish to build and push container images
#3954
Conversation
When deploying to a host that requires a container image, if the `language` of a project was `dotnet` we required the use of Docker to both build and push the container image. If the user had not authored a Dockerfile, we would try to use Oryx to build the container image "from source". With .NET 8, the SDK itself now has good support for both building a container image for an app and pushing it to a remote registry. We leveraged this for our Aspire work but non Aspire dotnet projects couldn't leverage this. This change fixes the situation. If your language is `dotnet` and your host requires a container, we'll now use `dotnet publish /t:PublishContainer` to build and push the container image. This does mean that the typical "Build" and "Package" steps behave differently for these apps now. Since our "Publish" step now both builds and pushes the container image, we defer the the work we would have done in "Build" and "Package" into the "Publish" step. Doing this allows us to eschew the need for a local Docker daemon to be installed and running. The .NET tooling does support creating a `.tgz` which could later be `docker load`'ed, and you could imagine that "Build" or "Package" produces such a thing, but doesn't allow us to use this as the input to a publish, so for now we don't spend time producing this artifact which would be unused by the downstream publish step. Fixes Azure#2632
| @@ -583,8 +583,14 @@ func (sm *serviceManager) GetFrameworkService(ctx context.Context, serviceConfig | |||
| // project that handles the containerization. | |||
| requiresLanguage := serviceConfig.Language != ServiceLanguageDocker && serviceConfig.Language != ServiceLanguageNone | |||
| if serviceConfig.Host.RequiresContainer() && requiresLanguage { | |||
| var serviceTargetType = string(ServiceLanguageDocker) | |||
|
|
|||
| if serviceConfig.Language == ServiceLanguageDotNet { | |||
There was a problem hiding this comment.
I wonder if we want to provide a way for folks to get the old behavior (where we'd wrap this in a DockerProject which would use a Dockerfile). Maybe if there was a Dockerfile at the root of the project folder?
There was a problem hiding this comment.
I really love the decision here to pick dotnet publish over Oryx build. I could see docker build if Dockerfile was set explicitly, or Dockerfile is unset but present in the project.
| // Gets the requirements for the language or framework service. | ||
| // This enables more fine grain control on whether the language / framework | ||
| // supports or requires lifecycle commands such as restore, build, and package | ||
| func (dp *dotNetContainerPublishProject) Requirements() FrameworkRequirements { |
There was a problem hiding this comment.
The implementations of both restore and build just call into the underlying project, so that azd restore and azd build continue to work (for folks that use them) but we need not run either of these since dotnet publish will take care of that for us.
|
If we had changes like this, we could have done the following with one of the AI samples: ellismg/contoso-chat-csharp-prompty@35064d9 This would require that the person running the sample have |
| inner FrameworkService | ||
| } | ||
|
|
||
| func NewDotNetContianerPublishProject() CompositeFrameworkService { |
There was a problem hiding this comment.
Not sure if this is better than peppering the existing Docker target with some ifs, but it felt like maybe it was playing a bit more by the conventions. Not wed to the idea of introducing a new CompositeFrameworkService and instead just adding more logic to the docker one, if folks think that would be helpful.
| @@ -216,6 +221,52 @@ func (ch *ContainerHelper) Deploy( | |||
| ) *async.TaskWithProgress[*ServiceDeployResult, ServiceProgress] { | |||
| return async.RunTaskWithProgress( | |||
| func(task *async.TaskContextWithProgress[*ServiceDeployResult, ServiceProgress]) { | |||
| if dotnetDetails, ok := packageOutput.Details.(*dotNetSdkPublishConfiguration); ok { | |||
| task.SetProgress(NewServiceProgress("Logging into container registry")) | |||
| dockerCreds, err := ch.Credentials(ctx, serviceConfig, targetResource) | |||
There was a problem hiding this comment.
Guessing the shape of this is going to change with your upcoming changes, @vhvb1989. Guessing we will get to a place where I can just call ch.Login(...) (which may no longer require docker?) and then ch.dotNetCli.PublishContainer without passing creds explicitly?
Anyway, calling your eye to this as a sense of what I'd like to be able to do.
I don't know how bad I should feel about this. We faced the same problem in the remote build draft that I drafted up in #3945. The practical result of this is that If we wanted to support some sort of |
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSIDocumentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
| } | ||
|
|
||
| task.SetProgress(NewServiceProgress("Building and pushing container image")) | ||
| imageName := fmt.Sprintf("azd-deploy-%s-%d", serviceConfig.Name, time.Now().Unix()) |
There was a problem hiding this comment.
ContainerHelper already contains functions that can be reused for proper image name generation based on all our current configurable rules.
| return | ||
| } | ||
|
|
||
| remoteImage := fmt.Sprintf("%s/%s", dockerCreds.LoginServer, imageName) |
There was a problem hiding this comment.
Same as above regarding remote image name generation. ContainerHelper already contains functions that can be reused for proper image name generation based on all our current configurable rules.
| @@ -216,6 +221,52 @@ func (ch *ContainerHelper) Deploy( | |||
| ) *async.TaskWithProgress[*ServiceDeployResult, ServiceProgress] { | |||
| return async.RunTaskWithProgress( | |||
| func(task *async.TaskContextWithProgress[*ServiceDeployResult, ServiceProgress]) { | |||
| if dotnetDetails, ok := packageOutput.Details.(*dotNetSdkPublishConfiguration); ok { | |||
There was a problem hiding this comment.
I think this set of changes can play well from your other PR with generic support for remote build. Having all container build scenarios funnel through ContainerHelper.Deploy whether it is local build, generic remote build or dotnet publish will allow all service hosts to easily take advantage of these new features.
| ) | ||
|
|
||
| type dotNetContainerPublishProject struct { | ||
| inner FrameworkService |
There was a problem hiding this comment.
I'm having a hard time understanding the root reason we need this new framework service implementation? This is just a variant of a dotnet project that uses dotnet publish for image deployment??
It feels like this check could be handled in the ContainerHelper implementation?
|
Hi @ellismg. Thank you for your interest in helping to improve the Azure Developer CLI experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
microsoft-github-policy-service
bot
added
the
no-recent-activity
|
Hi @ellismg. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing "/reopen" if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the "no-recent-activity" label; otherwise, this is likely to be closed again with the next cleanup pass. |
When deploying to a host that requires a container image, if the
languageof a project wasdotnetwe required the use of Docker to both build and push the container image. If the user had not authored a Dockerfile, we would try to use Oryx to build the container image "from source".With .NET 8, the SDK itself now has good support for both building a container image for an app and pushing it to a remote registry. We leveraged this for our Aspire work but non Aspire dotnet projects couldn't leverage this.
This change fixes the situation. If your language is
dotnetand your host requires a container, we'll now usedotnet publish /t:PublishContainerto build and push the container image.This does mean that the typical "Build" and "Package" steps behave differently for these apps now. Since our "Publish" step now both builds and pushes the container image, we defer the the work we would have done in "Build" and "Package" into the "Publish" step. Doing this allows us to eschew the need for a local Docker daemon to be installed and running.
The .NET tooling does support creating a
.tgzwhich could later bedocker load'ed, and you could imagine that "Build" or "Package" produces such a thing, but doesn't allow us to use this as the input to a publish, so for now we don't spend time producing this artifact which would be unused by the downstream publish step.Fixes #2632