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

[Enhancement]: $BUILDPLATFORM #993

Closed
MartinSchmidt opened this issue Sep 6, 2023 · 5 comments · Fixed by #1016
Closed

[Enhancement]: $BUILDPLATFORM #993

MartinSchmidt opened this issue Sep 6, 2023 · 5 comments · Fixed by #1016
Assignees
Labels
enhancement New feature or request

Comments

@MartinSchmidt
Copy link

MartinSchmidt commented Sep 6, 2023

Problem

I have a dockerfile that contains the following

FROM mcr.microsoft.com/dotnet/aspnet:7.0 AS base
WORKDIR /app
EXPOSE 80
EXPOSE 443

FROM --platform=$BUILDPLATFORM mcr.microsoft.com/dotnet/sdk:7.0 AS dotnet-build
WORKDIR /src
COPY . .
RUN dotnet restore
RUN dotnet build ProjectOrigin.Registry.Server -c Release --no-restore -o /app/build

FROM dotnet-build AS dotnet-publish
RUN dotnet publish ProjectOrigin.Registry.Server -c Release -o /app/publish

FROM base AS final
WORKDIR /app
COPY --from=dotnet-publish /app/publish .
HEALTHCHECK CMD curl --fail http://localhost:5000/health || exit 1
ENTRYPOINT ["dotnet", "ProjectOrigin.Registry.Server.dll"]

the --platform=$BUILDPLATFORMis only supported by BuildKit as I understand it, which cannot be used over the API currently.

This issue make me incapable of running the dockerfile as is with testcontainers.

Solution

Since the solution as now cannot support the functionality through docker, maybe we could include an option to strip these from the Dockerfile in a temporary file, before sending it to the context.

This has is what i ended up doing locally, since I didn't want two files to maintain, and then remove the file once the build is complete, but it ain't the most pretty solution.

_image = new ImageFromDockerfileBuilder()
            .WithDockerfileDirectory(CommonDirectoryPath.GetSolutionDirectory(), string.Empty)
            .WithDockerfile(relativeDockerfile)
            .StripPlatforms() // <-- making it an option
            .Build();

My thoughs was maybe to hook in the DockerfileArchive.tar method, as to replace the data just before being added to the tar, but just a thought.

Benefit

Being able to run testcontainers containing --platform=$BUILDPLATFORM with minimal friction

Alternatives

Wait for docker to support buildkit over the api.

Would you like to help contributing this enhancement?

Yes

@HofmeisterAn
Copy link
Collaborator

This might already work out of the box with the latest changes from the develop branch and is related to the following pull request: #979. Could you please test your configuration against develop?

@MartinSchmidt
Copy link
Author

MartinSchmidt commented Sep 8, 2023

I have just testet it against 3.5.0 which i can see is the same as the latest on develop.

I tried the following

TestcontainersSettings.Logger = LoggerFactory.Create(x =>
    {
        x.AddConsole();
        x.SetMinimumLevel(LogLevel.Debug);
    }).CreateLogger<ContainerTest>();

var image = new ImageFromDockerfileBuilder()
    .WithDockerfileDirectory(CommonDirectoryPath.GetSolutionDirectory(), string.Empty)
    .WithDockerfile(DockerfilePath)
    .WithBuildArgument("BUILDPLATFORM", "linux/arm64")
    .Build();

await image.CreateAsync().ConfigureAwait(false);

The attached debugger outputs the following, same if i set the WithBuildArgument or not, still fails.

info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Connected to Docker:
        Host: unix:///var/run/docker.sock
        Server Version: 23.0.6+azure-2
        Kernel Version: 5.15.49-linuxkit-pr
        API Version: 1.42
        Operating System: Debian GNU/Linux 11 (bullseye) (containerized)
        Total Memory: 15.61 GB
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Searching Docker registry credential in Auths
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Searching Docker registry credential in CredHelpers
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Searching Docker registry credential in Auths
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Searching Docker registry credential in CredsStore
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Docker registry credential mcr.microsoft.com not found
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pulling from dotnet/sdk
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Digest: sha256:bdcfb498261ca18f023ac67615d814ea743aa3288eb880855fa2eb86c6313ccc
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Status: Image is up to date for mcr.microsoft.com/dotnet/sdk:7.0
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Docker image mcr.microsoft.com/dotnet/sdk:7.0 created
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pulling from dotnet/aspnet
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Digest: sha256:b236eb1df512af4d8034ce8f65f9e8740f1845d894b7906a4b1b66890946c03f
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Status: Image is up to date for mcr.microsoft.com/dotnet/aspnet:7.0
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Docker image mcr.microsoft.com/dotnet/aspnet:7.0 created
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^(.+)\/\.idea added to the regex cache
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^([\\\/]?(\.idea\b|$)) added to the regex cache
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^([\\\/]?((.+)\/\.vs\b|$)) added to the regex cache
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^([\\\/]?(\.vs\b|$)) added to the regex cache
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^([\\\/]?(([^\\\/]+)?\/bin\b|$)) added to the regex cache
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^([\\\/]?(bin\b|$)) added to the regex cache
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^([\\\/]?(([^\\\/]+)?\/obj\b|$)) added to the regex cache
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^([\\\/]?(obj\b|$)) added to the regex cache
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^([\\\/]?(\.dockerignore\b|$)) added to the regex cache
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^([\\\/]?(ProjectOrigin\.Registry\.Server\/Dockerfile\b|$)) added to the regex cache
info: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Pattern ^([\\\/]?(Dockerfile\b|$)) added to the regex cache
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Step 1/21 : FROM mcr.microsoft.com/dotnet/aspnet:7.0 AS base
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
       ---> b5d2c273630a
      
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Step 2/21 : WORKDIR /app
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
       ---> Using cache
      
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
       ---> e7194a778446
      
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Step 3/21 : EXPOSE 80
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
       ---> Using cache
      
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
       ---> 7a7efbe26797
      
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Step 4/21 : EXPOSE 443
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
       ---> Using cache
      
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
       ---> f26c2b35aef1
      
dbug: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      Step 5/21 : FROM --platform=$BUILDPLATFORM mcr.microsoft.com/dotnet/sdk:7.0 AS dotnet-build
fail: ProjectOrigin.Electricity.IntegrationTests.ContainerTest[0]
      failed to parse platform : "" is an invalid component of "": platform specifier component must match "^[A-Za-z0-9_-]+$": invalid argument

@HofmeisterAn
Copy link
Collaborator

Oh, I understand now. The Docker build itself is failing. I initially thought it might be connected to the fix mentioned in the PR, where Testcontainers attempts to pull images in advance. I just tested whether Testcontainers correctly selects the image from the line FROM --platform=$BUILDPLATFORM mcr.microsoft.com/dotnet/sdk:7.0 AS dotnet-build, which it does.

Since the solution as now cannot support the functionality through docker, maybe we could include an option to strip these from the Dockerfile in a temporary file, before sending it to the context.

As you have mentioned, perhaps the builder can handle this task by generating a temporary Dockerfile next to the original Dockerfile. Using the hash of the original file to determine whether it needs to be created or not. Nevertheless, I am considering whether it might be more straightforward to include an additional Dockerfile, such as "Dockerfile.arm64," within your project. I don't have a strong opinion on this matter; if it helps developers write better tests, a contribution is welcome. I would favor a more generic approach, which may allow more modifications in the future.

@MartinSchmidt
Copy link
Author

My current solution is to create a temporary file, where i strip the --platform=$BUILDPLATFORM part

The --platform=$BUILDPLATFORM is only used when building the image for multiple platforms, and the $BUILDPLATFORM is automatically set by the docker buildkit.

I could simple create a file without it, but keeping two files up to date is something that never happens in the long run, so made it happen automatically, but generally, I cant be the only one with this issue.

The idea to create this issue was exactly to discuss how to do it, another approach could be to enable one to enable one to pass a textreader, buffer or action<string> that could return the data into the build process, then one could basically do whatever one wanted.

var image = new ImageFromDockerfileBuilder()
    .WithDockerfileDirectory(CommonDirectoryPath.GetSolutionDirectory(), string.Empty)
    .WithDockerfile(() => File.ReadAllLines("MyDockerfile") )
    .Build();

What do you think?

@HofmeisterAn
Copy link
Collaborator

but keeping two files up to date is something that never happens in the long run, so made it happen automatically, but generally, I cant be the only one with this issue.

Yes, I agree. I wouldn't favor maintaining (almost) duplicate configurations either.

that could return the data into the build process, then one could basically do whatever one wanted.

I was thinking about something similar. What do you think about an implementation of Dockerfile:

public class Dockerfile
{
    private readonly StringBuilder _stringBuilder = new StringBuilder();

    private readonly string _dockerfileFilePath;

    public Dockerfile(string dockerfileFilePath)
    {
        _dockerfileFilePath = dockerfileFilePath;
    }

    public virtual string Resolve()
    {
        using (var streamReader = new StreamReader(_dockerfileFilePath))
        {
            string filePath;

            while (!streamReader.EndOfStream)
            {
                Process(streamReader.ReadLine());
            }

            using (var sha256 = SHA256.Create())
            {
                var hashBytes = sha256.ComputeHash(streamReader.CurrentEncoding.GetBytes(_stringBuilder.ToString()));
                var hash = BitConverter.ToString(hashBytes).Replace("-", string.Empty).ToLower();
                filePath = string.Join(".", _dockerfileFilePath, hash.Substring(0, 8));
            }

            using (var fileStream = new StreamWriter(filePath))
            {
                fileStream.Write(_stringBuilder.ToString());
            }

            return filePath;
        }
    }

    protected virtual void Process(string line)
    {
        _stringBuilder.AppendLine(line);
    }
}

Then we can call the following line to create the Dockerfile and configure the builder:

return Merge(DockerResourceConfiguration, new ImageFromDockerfileConfiguration(dockerfile: dockerfile.Resolve()));

We can offer a default implementation, and others have the option to override it in order to customize the generation of the Dockerfile. However, I would like to conduct further research to identify common patterns for addressing this type of issue.

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

Successfully merging a pull request may close this issue.

2 participants