-
Notifications
You must be signed in to change notification settings - Fork 39
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
Infer container image properties from Project metadata #11
Comments
I'd like any feedback on the
<ItemGroup>
<ContainerConfig>
<Port Include="1234" Type="udp" />
</ContainerConfig>
</ItemGroup> and so one, dropping the |
We should have a public target at which point all the property inference has been done, so that consumers of this information (not just our publish target, but potentially IDE container tooling or scanners, etc) have a solid integration point to hang off of. |
Unfortunately this isn't possible in today's MSBuild. Were you thinking of adding support for Is there a list of "required" properties we expect customers to fill in the blanks for? Those could fall under a single Your example (pasted below) makes the most sense if we're not planning to modify MSBuild to support a more specialized type. Especially if we expect certain items to increase their number of potential metadata over time. <ItemGroup>
<ContainerPort Include="1234" Type="udp" />
<ContainerEnvironmentVariable Include="LOG_LEVEL" Value="trace" />
<ContainerLabel Include="org.contoso.businessunit" Value="C+AI" />
</ItemGroup> |
I figured that an entire new item might be too gnarly :) I'm ok with having a set of |
@benvillalobos I updated the table - one thing I'm wrestling with is that a mix of properties and items is not great from a end-user experience point of view - the user has to hop around to separate XML elements to describe their container. Another thing that's worth digging into is that the Entrypoint and Cmd properties natively are string arrays, so that we don't have to handle shell token escaping, but the most 'natural' way for a user to define them in MSBuild would be a single string property. @rainersigwald and I have chatted a bit about maybe a CLI-splitting property function? or just accepting the pain and requiring Items for this. What are your thoughts? |
All of the items other than the command line could be semicolon-delimited properties (and there's precedent for that in other places in MSBuild). That's not ideal but it would keep things together. Another option to keep things together, maybe, is metadata on an item, but that's a lot harder to customize in targets at build time. |
Focusing on Cmd & how users should define that for a bit, I'm gonna brain dump the options I immediately see. The Item Route
The Property Route
The CLI portion is where I'm not 100% in sync. Can you describe a super brief scenario for that? My understanding is: Add a property function that will turn a space-delimited command line into a string[]? Or Item to flow into the build?
To play devil's advocate: Do we care about modifying this stuff during the build? Would the output of some build step determine some metadata within an item? I think I'm just lacking scenarios to base this on so it's difficult to think in terms of flexibility (complexity) vs simplicity |
Questions as I'm defining defaults: ContainerBaseImageName
ContainerBaseImageTag
|
The images we're used to can be browsed over on Microsoft Container Registry - IMO we'll mostly be interested in the following images specifically for apps:
These are pushed by our internal teams, and they have a regular tag format. You can see some of them here. These tags might be for:
|
ProjectCapabilities are Yet Another MSBuild Item that VS (and other IDEs) can use to light-up UI or editor capabilities. They are 'markers' more than anything else. The AspNetCore one is included in a set of props/targets called the ProjectSystem targets, but that's always included in the 'core' WebSDK so for an aspnet project it should always be present. That's why I thought it would be a good, reliable differentiation point for the feature. |
We should cross-check with the Azure Container tooling docs here: https://docs.microsoft.com/en-us/visualstudio/containers/container-msbuild-properties?view=vs-2022 |
Right now we're fixated on the framework-dependent scenario, especially when it comes to inferring what the base image should be. For single-file, self-contained style deployments, though, we should support using the runtime-deps images for linux or the nanoserver images for Windows (even though we're not explicitly looking at Windows at the moment). EDIT: We have pivots for self-contained to use dotnet/runtime-deps. The windows scenario is still lacking. |
Some random thoughts after reading through this list:
|
Sorry for the delay here @tmds -
|
Users should be able to easily set the key aspects of image configuration from their project file. There should be a clear integration point at which those values are 'fixed' and can no longer be changed, and there should be reasonable inference for these properties where it makes sense. Where relevant, we should rely on properties that are already conventionally available - packaging information from NuGet and source control information from SourceLink are good examples of these. That lets us make use of documentation and examples from those projects to lessen the number of new concepts here.
dotnet/aspnet
base image, otherwise prefer thedotnet/runtime
image:
.$(AssemblyName)
property, lowercased so that it matches image name requirementsVersion
property. This one is a bit loose, would folks prefer multiple tags here?/app
. This directory will be the 'root' of the project's deployed files./app
dotnet <path to project's output.dll>
dotnet weatherapp.dll
. The path to the apphost or output dll will change based on theContainerWorkingDirectory
-
<ContainerPort Include="<port number>" Type="tcp, udp" />
AspNetCore
capability, port entries for TCP 5000 and TCP 5001 will be created<ContainerPort Include="5000" Type="tcp"/><ContainerPort Include="5001" Type="tcp"/>
<ContainerEnvironmentVariable Include="ASPNETCORE_URLS" Value="http://localhost:5000" />
AspNetCore
capability, theASPNETCORE_URLS
variable will be set tohttp://localhost:5000;https://localhost:5001
<ContainerLabel Include="org.opencontainers.image.created" Value="2022-06-07T12:10:12Z" />
Labels
There are some well-known labels that we should consider setting by default:
$([System.DateTime]::UtcNow.ToString("o"))
$(Authors)
$(RepositoryUrl)
if present.$(PackageLicenseUrl)
is deprecated, we may just have to default to$(RepositoryUrl)
. Users should be able to override this easily though.$(RepositoryUrl)
if present.$(Version)
if present. Note that there's not really a consistent place to hook into where we can be assured of a version being set, so we'll want to be sometime directly beforeBuild
, but as late as possible otherwise.$(PackageOwners)
or$(Owners)
to bootstrap off of, but maybe we should make one?$(PackageLicenseExpression)
$(RepositoryBranch)
or$(RepositoryCommit)
. There's not a baked-in for git repo tags in SourceLink, which would be ideal. Commit is probably the best one.$(Title)
$(Description)
$(ContainerBaseImage)
$(ContainerBaseImage)
Example project file
Work Items
The text was updated successfully, but these errors were encountered: