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

ImageBuilder requires Docker to run all commands, even if they don't use it #1428

Open lbussell opened 2 weeks ago

lbussell commented 2 weeks ago

This is needed for https://github.com/dotnet/dotnet-docker/issues/5654.

Steps to reproduce the issue

  1. Run ImageBuilder in a scenario where Docker is not installed, or the host has not passed in the Docker socket to the ImageBuilder container. It should still be able to do tasks like generate Dockerfiles and readmes.

Additional information (e.g. issue happens only occasionally)

There are two issues here.

One is that this CLI middleware runs for every command, outputting docker version and docker info before the command starts:

https://github.com/dotnet/docker-tools/blob/eb53e75aa4157d922b8c30d9132f8d11b79e8183/src/Microsoft.DotNet.ImageBuilder/src/ImageBuilder.cs#L55-L63

This is only useful for CI scenarios, not local dev scenarios, so it could be hidden behind a common CLI argument (like --ci or --no-ci) that we only use in pipelines.

The second is that, for commands with a ManifestFilterOptions, the default values for those options rely on output from the Docker CLI:

https://github.com/dotnet/docker-tools/blob/eb53e75aa4157d922b8c30d9132f8d11b79e8183/src/Microsoft.DotNet.ImageBuilder/src/Commands/ManifestFilterOptions.cs#L29-L37

For cases like generating readmes and Dockerfiles, it doesn't make sense to filter by arch or OS type. We could split ManifestFilterOptions into DockerfileFilterOptions and PlatformFilterOptions. DockerfileFilterOptions would let you filter by version, OS, or path. The other would let you filter by architecture and OS type (Linux/Windows). We could combine both of those for some commands as needed.

I would probably argue that it doesn't make sense for readme and Dockerfile generation to take path filters at all - our goal should be for generating those to be as quick and frictionless as possible.