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

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

Open
lbussell opened this issue Sep 9, 2024 · 0 comments

Comments

@lbussell
Copy link
Contributor

lbussell commented Sep 9, 2024

This is needed for dotnet/dotnet-docker#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:

.UseMiddleware(context =>
{
if (context.ParseResult.CommandResult.Command != rootCliCommand)
{
// Capture the Docker version and info in the output.
ExecuteHelper.Execute(fileName: "docker", args: "version", isDryRun: false);
ExecuteHelper.Execute(fileName: "docker", args: "info", isDryRun: false);
}
})

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:

public IEnumerable<Option> GetCliOptions() =>
new Option[]
{
CreateOption("architecture", nameof(ManifestFilterOptions.Architecture),
"Architecture of Dockerfiles to operate on - wildcard chars * and ? supported (default is current OS architecture)",
() => DockerHelper.Architecture.GetDockerName()),
CreateOption("os-type", nameof(ManifestFilterOptions.OsType),
"OS type (linux/windows) of the Dockerfiles to build - wildcard chars * and ? supported (default is the Docker OS)",
() => DockerHelper.OS.GetDockerName()),

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

1 participant