Skip to content
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

ContainerImageTags doesn't work #236

Closed
flcdrg opened this issue Nov 6, 2022 · 6 comments · Fixed by #239
Closed

ContainerImageTags doesn't work #236

flcdrg opened this issue Nov 6, 2022 · 6 comments · Fixed by #239

Comments

@flcdrg
Copy link
Contributor

flcdrg commented Nov 6, 2022

Adding ContainerImageTags to a csproj doesn't work.. eg.

dotnet new web
dotnet add package Microsoft.NET.Build.Containers

Edit .csproj and add <ContainerImageTags>1.2.3-alpha2;latest</ContainerImageTags> to the first PropertyGroup

dotnet publish --os linux --arch x64  -c release -p:PublishProfile=DefaultContainer

The following output is produced:

MSBuild version 17.4.0-preview-22470-08+6521b1591 for .NET
  Determining projects to restore...
  All projects are up-to-date for restore.
C:\Program Files\dotnet\sdk\7.0.100-rc.2.22477.23\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.RuntimeIdentifierInferen
ce.targets(257,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-poli
cy [C:\tmp\container-demo\container-demo.csproj]
  container-demo -> C:\tmp\container-demo\bin\release\net7.0\linux-x64\container-demo.dll
  container-demo -> C:\tmp\container-demo\bin\release\net7.0\linux-x64\publish\
  Building image 'container-demo' with tags  on top of base image mcr.microsoft.com/dotnet/aspnet:7.0

Note that no tags are listed in the output.

Checking Docker and there are no new images published.

I can confirm that ContainerImageTag (without the 's') does work

@baronfel
Copy link
Member

baronfel commented Nov 7, 2022

Good catch - it looks like we have some kind of unhandled crash here. We need to have at least some kind of.output when this occurs, in addition to fixing the underlying multiple-tags issue.

@rainersigwald
Copy link
Member

Thanks for the report @flcdrg!

You can work around this in your project:

  <ItemGroup>
    <!-- Work around https://github.com/dotnet/sdk-container-builds/issues/236 -->
    <ContainerImageTags Include="$(ContainerImageTags)" />
  </ItemGroup>

That shouldn't be required, though!

@rainersigwald
Copy link
Member

The silent failure is because there is nothing in this foreach:

So we just don't attempt any push. We should probably error if ImageTags is empty.

@rainersigwald
Copy link
Member

But why is it empty? It's getting lost somewhere in ParseContainerProperties:

ParseContainerProperties
    Assembly = C:\Users\raines\.nuget\packages\microsoft.net.build.containers\0.2.7\build\..\tasks\net7.0\Microsoft.NET.Build.Containers.dll
    Parameters
        ContainerImageTags
            1.2.3-alpha2
            latest
        ContainerImageName = 236
        FullyQualifiedBaseImageName = mcr.microsoft.com/dotnet/aspnet:7.0
        ContainerEnvironmentVariables
            ASPNETCORE_URLS
                Value = http://+:80
    Parsed the following properties. Note: Spaces are replaced with dashes.
    Host: mcr.microsoft.com
    Image: dotnet/aspnet
    Tag: 7.0
    Image Name: 236
    Image Tags: 
    OutputProperties
        ContainerBaseRegistry = mcr.microsoft.com
        ContainerBaseName = dotnet/aspnet
        ContainerBaseTag = 7.0
        ContainerRegistry = 
        ContainerImageName = 236
    OutputItems
        ContainerEnvironmentVariables
            ASPNETCORE_URLS
                Value = http://+:80

... oh

else if (ContainerImageTags.Length != 0 && !TryValidateTags(ContainerImageTags, out var valids, out var invalids))

rainersigwald added a commit to rainersigwald/sdk-container-builds that referenced this issue Nov 7, 2022
A logic error caused ParseContainerProperties to fail if multi-valued
tags were passed into ContainerImageTags. A test error made that non-
obvious.

Fix the tests to use CollectionAssert methods, and invert the check
on TryValidateTags so that when the tags are valid we respect them.

Fixes dotnet#236.
@rainersigwald
Copy link
Member

I think #239 will do the trick but haven't validated the full scenario yet.

baronfel pushed a commit that referenced this issue Nov 7, 2022
A logic error caused ParseContainerProperties to fail if multi-valued
tags were passed into ContainerImageTags. A test error made that non-
obvious.

Fix the tests to use CollectionAssert methods, and invert the check
on TryValidateTags so that when the tags are valid we respect them.

Fixes #236.
@rainersigwald
Copy link
Member

@flcdrg I confirmed that prerelease package 0.3.0-alpha.7 works:

dotnet publish --os linux --arch x64  -c release -p:PublishProfile=DefaultContainer -nr:false
MSBuild version 17.4.0+18d5aef85 for .NET
  Determining projects to restore...
  All projects are up-to-date for restore.
  236 -> S:\repro\dotnet\sdk-container-builds\issues\236\bin\release\net7.0\linux-x64\236.dll
  236 -> S:\repro\dotnet\sdk-container-builds\issues\236\bin\release\net7.0\linux-x64\publish\
  Building image '236' with tags 1.2.3-alpha2,latest on top of base image mcr.microsoft.com/dotnet/aspnet:7.0
  Pushed container '236:1.2.3-alpha2' to Docker daemon
  Pushed container '236:latest' to Docker daemon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants