Merge pull request #1 from tmds/fix_podman

baronfel · web-flow · commit 48a07b6faa5b · 2023-08-30T09:16:18.000-06:00
DockerCli: fix docker being used when podman was detected.
diff --git a/src/Containers/Microsoft.NET.Build.Containers/LocalDaemons/DockerCli.cs b/src/Containers/Microsoft.NET.Build.Containers/LocalDaemons/DockerCli.cs
@@ -21,6 +21,7 @@ internal sealed class DockerCli : ILocalRegistry
 
     private readonly ILogger _logger;
     private string? _commandPath;
+    private string? _fullCommandPath;
 
     public DockerCli(string? command, ILoggerFactory logger)
     {
@@ -38,13 +39,8 @@ public DockerCli(string? command, ILoggerFactory logger)
     public DockerCli(ILoggerFactory loggerFactory) : this(null, loggerFactory)
     { }
 
-    private static string? FindFullPathFromPath(string? command)
+    private static string FindFullPathFromPath(string command)
     {
-        if (string.IsNullOrEmpty(command))
-        {
-            return command;
-        }
-
         foreach (string directory in (Environment.GetEnvironmentVariable("PATH") ?? string.Empty).Split(Path.PathSeparator))
         {
             string fullPath = Path.Combine(directory, command + FileNameSuffixes.CurrentPlatform.Exe);
@@ -57,19 +53,32 @@ public DockerCli(ILoggerFactory loggerFactory) : this(null, loggerFactory)
         return command;
     }
 
-    public async Task LoadAsync(BuiltImage image, ImageReference sourceReference, ImageReference destinationReference, CancellationToken cancellationToken)
+    private async ValueTask<string> FindFullCommandPath(CancellationToken cancellationToken)
     {
-        cancellationToken.ThrowIfCancellationRequested();
-        string dockerPath = FindFullPathFromPath("docker") ?? "docker";
+        if (_fullCommandPath != null)
+        {
+            return _fullCommandPath;
+        }
 
-        string? commandPath = await GetCommandPathAsync(cancellationToken);
-        if (commandPath is null)
+        string? command = await GetCommandAsync(cancellationToken);
+        if (command is null)
         {
             throw new NotImplementedException(Resource.FormatString(Strings.DockerProcessCreationFailed, Commands));
         }
 
+        _fullCommandPath = FindFullPathFromPath(command);
+
+        return _fullCommandPath;
+    }
+
+    public async Task LoadAsync(BuiltImage image, ImageReference sourceReference, ImageReference destinationReference, CancellationToken cancellationToken)
+    {
+        cancellationToken.ThrowIfCancellationRequested();
+
+        string commandPath = await FindFullCommandPath(cancellationToken);
+
         // call `docker load` and get it ready to receive input
-        ProcessStartInfo loadInfo = new(dockerPath, $"load");
+        ProcessStartInfo loadInfo = new(commandPath, $"load");
         loadInfo.RedirectStandardInput = true;
         loadInfo.RedirectStandardOutput = true;
         loadInfo.RedirectStandardError = true;
@@ -102,20 +111,20 @@ public async Task LoadAsync(BuiltImage image, ImageReference sourceReference, Im
     public async Task<bool> IsAvailableAsync(CancellationToken cancellationToken)
     {
         bool commandPathWasUnknown = this._commandPath is null; // avoid running the version command twice.
-        string? commandPath = await GetCommandPathAsync(cancellationToken);
-        if (commandPath is null)
+        string? command = await GetCommandAsync(cancellationToken);
+        if (command is null)
         {
             _logger.LogError($"Cannot find {Commands} executable.");
             return false;
         }
 
         try
         {
-            switch (commandPath)
+            switch (command)
             {
                 case DockerCommand:
                     {
-                        JsonDocument config = GetConfig();
+                        JsonDocument config = GetDockerConfig();
 
                         if (!config.RootElement.TryGetProperty("ServerErrors", out JsonElement errorProperty))
                         {
@@ -136,7 +145,7 @@ public async Task<bool> IsAvailableAsync(CancellationToken cancellationToken)
                 case PodmanCommand:
                     return commandPathWasUnknown || await TryRunVersionCommandAsync(PodmanCommand, cancellationToken);
                 default:
-                    throw new NotImplementedException($"{commandPath} is an unknown command.");
+                    throw new NotImplementedException($"{command} is an unknown command.");
             }
         }
         catch (Exception ex)
@@ -152,16 +161,16 @@ public bool IsAvailable()
         => IsAvailableAsync(default).GetAwaiter().GetResult();
 
     public string? GetCommand()
-        => GetCommandPathAsync(default).GetAwaiter().GetResult();
+        => GetCommandAsync(default).GetAwaiter().GetResult();
 
     /// <summary>
     /// Gets docker configuration.
     /// </summary>
     /// <param name="sync">when <see langword="true"/>, the method is executed synchronously.</param>
     /// <exception cref="DockerLoadException">when failed to retrieve docker configuration.</exception>
-    internal static JsonDocument GetConfig()
+    internal static JsonDocument GetDockerConfig()
     {
-        string dockerPath = FindFullPathFromPath("docker") ?? "docker";
+        string dockerPath = FindFullPathFromPath("docker");
         Process proc = new()
         {
             StartInfo = new ProcessStartInfo(dockerPath, "info --format=\"{{json .}}\"")
@@ -265,7 +274,7 @@ private static async Task WriteImageToStreamAsync(BuiltImage image, ImageReferen
         }
     }
 
-    private async ValueTask<string?> GetCommandPathAsync(CancellationToken cancellationToken)
+    private async ValueTask<string?> GetCommandAsync(CancellationToken cancellationToken)
     {
         if (_commandPath != null)
         {
@@ -306,7 +315,7 @@ private static bool IsPodmanAlias()
         // or if it is a podman script in a trenchcoat.
         try
         {
-            var dockerinfo = GetConfig().RootElement;
+            var dockerinfo = GetDockerConfig().RootElement;
             // Docker's info output has a 'DockerRootDir' top-level property string that is a good marker,
             // while Podman has a 'host' top-level property object with a 'buildahVersion' subproperty
             var hasdockerProperty =
diff --git a/src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/CreateNewImageTests.cs b/src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/CreateNewImageTests.cs
@@ -52,7 +52,7 @@ public void CreateNewImage_Baseline()
         task.BaseImageTag = "7.0";
 
         task.OutputRegistry = "localhost:5010";
-        task.LocalRegistry = "Docker";
+        task.LocalRegistry = DockerAvailableFactAttribute.LocalRegistry;
         task.PublishDirectory = Path.Combine(newProjectDir.FullName, "bin", "Release", ToolsetInfo.CurrentTargetFramework, "linux-arm64", "publish");
         task.Repository = "dotnet/create-new-image-baseline";
         task.ImageTags = new[] { "latest" };
@@ -197,7 +197,7 @@ public void Tasks_EndToEnd_With_EnvironmentVariable_Validation()
         cni.ContainerEnvironmentVariables = pcp.NewContainerEnvironmentVariables;
         cni.ContainerRuntimeIdentifier = "linux-x64";
         cni.RuntimeIdentifierGraphPath = ToolsetUtils.GetRuntimeGraphFilePath();
-        cni.LocalRegistry = global::Microsoft.NET.Build.Containers.KnownLocalRegistryTypes.Docker;
+        cni.LocalRegistry = DockerAvailableFactAttribute.LocalRegistry;
 
         Assert.True(cni.Execute(), FormatBuildMessages(errors));
 
diff --git a/src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/DockerSupportsArchInlineData.cs b/src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/DockerSupportsArchInlineData.cs
@@ -13,7 +13,7 @@ public class DockerSupportsArchInlineData : DataAttribute
     private static string[] LinuxPlatforms = GetSupportedLinuxPlatforms();
 
     // another optimization - daemons don't switch types easily or quickly, so this is as good as static
-    private static bool IsWindowsDaemon = GetIsWindowsDaemon();
+    private static bool IsWindowsDockerDaemon = GetIsWindowsDockerDaemon();
 
     private readonly string _arch;
     private readonly object[] _data;
@@ -41,7 +41,7 @@ private bool DaemonSupportsArch(string arch)
         }
         else
         {
-            if (IsWindowsDaemon && arch.StartsWith("windows", StringComparison.OrdinalIgnoreCase))
+            if (IsWindowsDockerDaemon && arch.StartsWith("windows", StringComparison.OrdinalIgnoreCase))
             {
                 return true;
             }
@@ -68,11 +68,15 @@ private static string[] GetSupportedLinuxPlatforms()
         }
     }
 
-    private static bool GetIsWindowsDaemon()
+    private static bool GetIsWindowsDockerDaemon()
     {
+        if (ContainerCli.IsPodman)
+        {
+            return false;
+        }
         // the config json has an OSType property that is either "linux" or "windows" -
         // we can't use this for linux arch detection because that isn't enough information.
-        var config = DockerCli.GetConfig();
+        var config = DockerCli.GetDockerConfig();
         if (config.RootElement.TryGetProperty("OSType", out JsonElement osTypeProperty))
         {
             return osTypeProperty.GetString() == "windows";
diff --git a/src/Tests/Microsoft.NET.Build.Containers.UnitTests/DockerAvailableUtils.cs b/src/Tests/Microsoft.NET.Build.Containers.UnitTests/DockerAvailableUtils.cs
@@ -5,6 +5,8 @@ namespace Microsoft.NET.Build.Containers.UnitTests;
 
 public class DockerAvailableTheoryAttribute : TheoryAttribute
 {
+    public static string LocalRegistry => DockerCliStatus.LocalRegistry;
+
     public DockerAvailableTheoryAttribute(bool skipPodman = false)
     {
         if (!DockerCliStatus.IsAvailable)
@@ -21,6 +23,8 @@ public DockerAvailableTheoryAttribute(bool skipPodman = false)
 
 public class DockerAvailableFactAttribute : FactAttribute
 {
+    public static string LocalRegistry => DockerCliStatus.LocalRegistry;
+
     public DockerAvailableFactAttribute(bool skipPodman = false)
     {
         if (!DockerCliStatus.IsAvailable)
@@ -41,6 +45,9 @@ file static class DockerCliStatus
 {
     public static readonly bool IsAvailable;
     public static readonly string? Command;
+    public static string LocalRegistry
+        => Command == DockerCli.PodmanCommand ? KnownLocalRegistryTypes.Podman
+                                              : KnownLocalRegistryTypes.Docker;
 
     static DockerCliStatus()
     {

Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@ internal sealed class DockerCli : ILocalRegistry`
`21`	`21`
`22`	`22`	`private readonly ILogger _logger;`
`23`	`23`	`private string? _commandPath;`
	`24`	`+ private string? _fullCommandPath;`
`24`	`25`
`25`	`26`	`public DockerCli(string? command, ILoggerFactory logger)`
`26`	`27`	`{`
`@@ -38,13 +39,8 @@ public DockerCli(string? command, ILoggerFactory logger)`
`38`	`39`	`public DockerCli(ILoggerFactory loggerFactory) : this(null, loggerFactory)`
`39`	`40`	`{ }`
`40`	`41`
`41`		`- private static string? FindFullPathFromPath(string? command)`
	`42`	`+ private static string FindFullPathFromPath(string command)`
`42`	`43`	`{`
`43`		`- if (string.IsNullOrEmpty(command))`
`44`		`- {`
`45`		`- return command;`
`46`		`- }`
`47`		`-`
`48`	`44`	`foreach (string directory in (Environment.GetEnvironmentVariable("PATH") ?? string.Empty).Split(Path.PathSeparator))`
`49`	`45`	`{`
`50`	`46`	`string fullPath = Path.Combine(directory, command + FileNameSuffixes.CurrentPlatform.Exe);`
`@@ -57,19 +53,32 @@ public DockerCli(ILoggerFactory loggerFactory) : this(null, loggerFactory)`
`57`	`53`	`return command;`
`58`	`54`	`}`
`59`	`55`
`60`		`- public async Task LoadAsync(BuiltImage image, ImageReference sourceReference, ImageReference destinationReference, CancellationToken cancellationToken)`
	`56`	`+ private async ValueTask<string> FindFullCommandPath(CancellationToken cancellationToken)`
`61`	`57`	`{`
`62`		`- cancellationToken.ThrowIfCancellationRequested();`
`63`		`- string dockerPath = FindFullPathFromPath("docker") ?? "docker";`
	`58`	`+ if (_fullCommandPath != null)`
	`59`	`+ {`
	`60`	`+ return _fullCommandPath;`
	`61`	`+ }`
`64`	`62`
`65`		`- string? commandPath = await GetCommandPathAsync(cancellationToken);`
`66`		`- if (commandPath is null)`
	`63`	`+ string? command = await GetCommandAsync(cancellationToken);`
	`64`	`+ if (command is null)`
`67`	`65`	`{`
`68`	`66`	`throw new NotImplementedException(Resource.FormatString(Strings.DockerProcessCreationFailed, Commands));`
`69`	`67`	`}`
`70`	`68`
	`69`	`+ _fullCommandPath = FindFullPathFromPath(command);`
	`70`	`+`
	`71`	`+ return _fullCommandPath;`
	`72`	`+ }`
	`73`	`+`
	`74`	`+ public async Task LoadAsync(BuiltImage image, ImageReference sourceReference, ImageReference destinationReference, CancellationToken cancellationToken)`
	`75`	`+ {`
	`76`	`+ cancellationToken.ThrowIfCancellationRequested();`
	`77`	`+`
	`78`	`+ string commandPath = await FindFullCommandPath(cancellationToken);`
	`79`	`+`
`71`	`80`	// call `docker load` and get it ready to receive input
`72`		`- ProcessStartInfo loadInfo = new(dockerPath, $"load");`
	`81`	`+ ProcessStartInfo loadInfo = new(commandPath, $"load");`
`73`	`82`	`loadInfo.RedirectStandardInput = true;`
`74`	`83`	`loadInfo.RedirectStandardOutput = true;`
`75`	`84`	`loadInfo.RedirectStandardError = true;`
`@@ -102,20 +111,20 @@ public async Task LoadAsync(BuiltImage image, ImageReference sourceReference, Im`
`102`	`111`	`public async Task<bool> IsAvailableAsync(CancellationToken cancellationToken)`
`103`	`112`	`{`
`104`	`113`	`bool commandPathWasUnknown = this._commandPath is null; // avoid running the version command twice.`
`105`		`- string? commandPath = await GetCommandPathAsync(cancellationToken);`
`106`		`- if (commandPath is null)`
	`114`	`+ string? command = await GetCommandAsync(cancellationToken);`
	`115`	`+ if (command is null)`
`107`	`116`	`{`
`108`	`117`	`_logger.LogError($"Cannot find {Commands} executable.");`
`109`	`118`	`return false;`
`110`	`119`	`}`
`111`	`120`
`112`	`121`	`try`
`113`	`122`	`{`
`114`		`- switch (commandPath)`
	`123`	`+ switch (command)`
`115`	`124`	`{`
`116`	`125`	`case DockerCommand:`
`117`	`126`	`{`
`118`		`- JsonDocument config = GetConfig();`
	`127`	`+ JsonDocument config = GetDockerConfig();`
`119`	`128`
`120`	`129`	`if (!config.RootElement.TryGetProperty("ServerErrors", out JsonElement errorProperty))`
`121`	`130`	`{`
`@@ -136,7 +145,7 @@ public async Task<bool> IsAvailableAsync(CancellationToken cancellationToken)`
`136`	`145`	`case PodmanCommand:`
`137`	`146`	`return commandPathWasUnknown \|\| await TryRunVersionCommandAsync(PodmanCommand, cancellationToken);`
`138`	`147`	`default:`
`139`		`- throw new NotImplementedException($"{commandPath} is an unknown command.");`
	`148`	`+ throw new NotImplementedException($"{command} is an unknown command.");`
`140`	`149`	`}`
`141`	`150`	`}`
`142`	`151`	`catch (Exception ex)`
`@@ -152,16 +161,16 @@ public bool IsAvailable()`
`152`	`161`	`=> IsAvailableAsync(default).GetAwaiter().GetResult();`
`153`	`162`
`154`	`163`	`public string? GetCommand()`
`155`		`- => GetCommandPathAsync(default).GetAwaiter().GetResult();`
	`164`	`+ => GetCommandAsync(default).GetAwaiter().GetResult();`
`156`	`165`
`157`	`166`	`/// <summary>`
`158`	`167`	`/// Gets docker configuration.`
`159`	`168`	`/// </summary>`
`160`	`169`	`/// <param name="sync">when <see langword="true"/>, the method is executed synchronously.</param>`
`161`	`170`	`/// <exception cref="DockerLoadException">when failed to retrieve docker configuration.</exception>`
`162`		`- internal static JsonDocument GetConfig()`
	`171`	`+ internal static JsonDocument GetDockerConfig()`
`163`	`172`	`{`
`164`		`- string dockerPath = FindFullPathFromPath("docker") ?? "docker";`
	`173`	`+ string dockerPath = FindFullPathFromPath("docker");`
`165`	`174`	`Process proc = new()`
`166`	`175`	`{`
`167`	`176`	`StartInfo = new ProcessStartInfo(dockerPath, "info --format=\"{{json .}}\"")`
`@@ -265,7 +274,7 @@ private static async Task WriteImageToStreamAsync(BuiltImage image, ImageReferen`
`265`	`274`	`}`
`266`	`275`	`}`
`267`	`276`
`268`		`- private async ValueTask<string?> GetCommandPathAsync(CancellationToken cancellationToken)`
	`277`	`+ private async ValueTask<string?> GetCommandAsync(CancellationToken cancellationToken)`
`269`	`278`	`{`
`270`	`279`	`if (_commandPath != null)`
`271`	`280`	`{`
`@@ -306,7 +315,7 @@ private static bool IsPodmanAlias()`
`306`	`315`	`// or if it is a podman script in a trenchcoat.`
`307`	`316`	`try`
`308`	`317`	`{`
`309`		`- var dockerinfo = GetConfig().RootElement;`
	`318`	`+ var dockerinfo = GetDockerConfig().RootElement;`
`310`	`319`	`// Docker's info output has a 'DockerRootDir' top-level property string that is a good marker,`
`311`	`320`	`// while Podman has a 'host' top-level property object with a 'buildahVersion' subproperty`
`312`	`321`	`var hasdockerProperty =`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ public class DockerSupportsArchInlineData : DataAttribute`
`13`	`13`	`private static string[] LinuxPlatforms = GetSupportedLinuxPlatforms();`
`14`	`14`
`15`	`15`	`// another optimization - daemons don't switch types easily or quickly, so this is as good as static`
`16`		`- private static bool IsWindowsDaemon = GetIsWindowsDaemon();`
	`16`	`+ private static bool IsWindowsDockerDaemon = GetIsWindowsDockerDaemon();`
`17`	`17`
`18`	`18`	`private readonly string _arch;`
`19`	`19`	`private readonly object[] _data;`
`@@ -41,7 +41,7 @@ private bool DaemonSupportsArch(string arch)`
`41`	`41`	`}`
`42`	`42`	`else`
`43`	`43`	`{`
`44`		`- if (IsWindowsDaemon && arch.StartsWith("windows", StringComparison.OrdinalIgnoreCase))`
	`44`	`+ if (IsWindowsDockerDaemon && arch.StartsWith("windows", StringComparison.OrdinalIgnoreCase))`
`45`	`45`	`{`
`46`	`46`	`return true;`
`47`	`47`	`}`
`@@ -68,11 +68,15 @@ private static string[] GetSupportedLinuxPlatforms()`
`68`	`68`	`}`
`69`	`69`	`}`
`70`	`70`
`71`		`- private static bool GetIsWindowsDaemon()`
	`71`	`+ private static bool GetIsWindowsDockerDaemon()`
`72`	`72`	`{`
	`73`	`+ if (ContainerCli.IsPodman)`
	`74`	`+ {`
	`75`	`+ return false;`
	`76`	`+ }`
`73`	`77`	`// the config json has an OSType property that is either "linux" or "windows" -`
`74`	`78`	`// we can't use this for linux arch detection because that isn't enough information.`
`75`		`- var config = DockerCli.GetConfig();`
	`79`	`+ var config = DockerCli.GetDockerConfig();`
`76`	`80`	`if (config.RootElement.TryGetProperty("OSType", out JsonElement osTypeProperty))`
`77`	`81`	`{`
`78`	`82`	`return osTypeProperty.GetString() == "windows";`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,8 @@ namespace Microsoft.NET.Build.Containers.UnitTests;`
`5`	`5`
`6`	`6`	`public class DockerAvailableTheoryAttribute : TheoryAttribute`
`7`	`7`	`{`
	`8`	`+ public static string LocalRegistry => DockerCliStatus.LocalRegistry;`
	`9`	`+`
`8`	`10`	`public DockerAvailableTheoryAttribute(bool skipPodman = false)`
`9`	`11`	`{`
`10`	`12`	`if (!DockerCliStatus.IsAvailable)`
`@@ -21,6 +23,8 @@ public DockerAvailableTheoryAttribute(bool skipPodman = false)`
`21`	`23`
`22`	`24`	`public class DockerAvailableFactAttribute : FactAttribute`
`23`	`25`	`{`
	`26`	`+ public static string LocalRegistry => DockerCliStatus.LocalRegistry;`
	`27`	`+`
`24`	`28`	`public DockerAvailableFactAttribute(bool skipPodman = false)`
`25`	`29`	`{`
`26`	`30`	`if (!DockerCliStatus.IsAvailable)`
`@@ -41,6 +45,9 @@ file static class DockerCliStatus`
`41`	`45`	`{`
`42`	`46`	`public static readonly bool IsAvailable;`
`43`	`47`	`public static readonly string? Command;`
	`48`	`+ public static string LocalRegistry`
	`49`	`+ => Command == DockerCli.PodmanCommand ? KnownLocalRegistryTypes.Podman`
	`50`	`+ : KnownLocalRegistryTypes.Docker;`
`44`	`51`
`45`	`52`	`static DockerCliStatus()`
`46`	`53`	`{`