fix: WaitForExit() in Invoke-ExecutableCommand() hangs (#17)

Minor fix: Fix CodeFactor badge

Author: TinaMor

README.md

@@ -8,8 +8,8 @@
 [ci-build-site]: https://github.com/microsoft/containers-toolkit/actions/workflows/ci-build.yaml
 [devskim-image]: https://github.com/microsoft/containers-toolkit/actions/workflows/sdl-compliance.yaml/badge.svg
 [devskim-site]: https://github.com/microsoft/containers-toolkit/actions/workflows/sdl-compliance.yaml
-[cf-image]: https://www.codefactor.io/repository/github/powershell/powershell/badge
-[cf-site]: https://www.codefactor.io/repository/github/powershell/powershell
+[cf-image]: https://www.codefactor.io/repository/github/microsoft/containers-toolkit/badge/main
+[cf-site]: https://www.codefactor.io/repository/github/microsoft/containers-toolkit/overview/main
 
 ## Table of contents
 

Tests/ContainerdTools.Tests.ps1

@@ -162,7 +162,7 @@ Describe "ContainerdTools.psm1" {
 
     Context "Register-ContainerdService" -Tag "Register-ContainerdService" {
         BeforeAll {
-            $MockContainerdPath = 'TestDrive:\Program Files\Containerd'
+            $MockContainerdPath = "$TestDrive\Program Files\Containerd"
             # New-Item -Path $MockContainerdPath -ItemType 'Directory' -Force | Out-Null
             New-Item -Path "$MockContainerdPath\bin\containerd.exe" -ItemType 'File' -Force | Out-Null
 
@@ -172,15 +172,19 @@ Describe "ContainerdTools.psm1" {
             Mock Get-DefaultInstallPath -ModuleName "ContainerdTools" `
                 -MockWith { return $MockContainerdPath } `
                 -ParameterFilter { $Tool -eq "Containerd" }
-            Mock Invoke-ExecutableCommand -ModuleName "ContainerdTools" `
-                -ParameterFilter { $Arguments -eq "config default" } `
-                -MockWith { return 'Sample containerd default config data' }
 
-            $obj = New-MockObject -Type 'System.Diagnostics.Process' -Properties @{ ExitCode = 0 }
-            Mock Invoke-ExecutableCommand -ModuleName "ContainerdTools" -MockWith { return $obj }
+            $mockProcess = New-MockObject -Type 'System.Diagnostics.Process' -Properties @{ ExitCode = 0 }
+            Mock Invoke-ExecutableCommand -ModuleName "ContainerdTools" -MockWith { return $mockProcess }
 
-            # $MockService = New-MockObject -Type System.ServiceProcess.ServiceController -Methods @{ WaitForStatus = { } }
-            # Mock Get-Service -ModuleName "ContainerdTools" -MockWith { return $MockService }
+            # Mock for default config
+            $mockConfigStdOut = New-MockObject -Type 'System.IO.StreamReader' -Methods @{ ReadToEnd = { return "Sample containerd default config data" } }
+            $mockConfigProcess = New-MockObject -Type 'System.Diagnostics.Process' -Properties @{ 
+                StandardOutput = $mockConfigStdOut 
+            }
+            Mock Invoke-ExecutableCommand -ModuleName "ContainerdTools" `
+                -ParameterFilter { $Arguments -eq "config default" } `
+                -MockWith { return $mockConfigProcess }
+           
             Mock Get-Service -ModuleName "ContainerdTools" -MockWith { return [MockService]::new('Containerd') }
             Mock Set-Service -ModuleName "ContainerdTools"
             Mock Start-ContainerdService -ModuleName "ContainerdTools"
@@ -254,6 +258,19 @@ Describe "ContainerdTools.psm1" {
 
             { Register-ContainerdService -Force } | Should -Throw "Failed to register containerd service.*"
         }
+
+        It "Should throw an error if config file is empty" {
+            # Mock for default config
+            $mockConfigStdOut = New-MockObject -Type 'System.IO.StreamReader' -Methods @{ ReadToEnd = { return } }
+            $mockConfigProcess = New-MockObject -Type 'System.Diagnostics.Process' -Properties @{ 
+                StandardOutput = $mockConfigStdOut 
+            }
+            Mock Invoke-ExecutableCommand -ModuleName "ContainerdTools" `
+                -ParameterFilter { $Arguments -eq "config default" } `
+                -MockWith { return $mockConfigProcess }
+
+            { Register-ContainerdService -Force } | Should -Throw "Config file is empty. '$MockContainerdPath\config.toml'"
+        }
     }
 
     Context "Uninstall-Containerd" -Tag "Uninstall-Containerd" {

containers-toolkit/Private/CommonToolUtilities.psm1

@@ -171,7 +171,6 @@ function Install-ContainerToolConsent ($tool) {
     return [ActionConsent]$consent -eq [ActionConsent]::Yes
 }
 
-
 function Uninstall-ContainerToolConsent ($tool, $path) {
     $question = "Do you want to uninstall $tool from $($path)?"
     $choices = '&Yes', '&No'
@@ -266,14 +265,27 @@ function Invoke-StopService($service) {
     }
 }
 
+function Test-ConfFileEmpty($Path) {
+    if (!(Test-Path -LiteralPath $Path)) {
+        return $true
+    }
+
+    $isFileNotEmpty = (([System.IO.File]::ReadAllText($Path)) -match '\S')
+    return (-not $isFileNotEmpty )
+}
+
 function Invoke-ExecutableCommand {
     param (
         [parameter(Mandatory)]
         [String] $executable,
         [parameter(Mandatory)]
-        [String] $arguments
+        [String] $arguments,
+        [Parameter(Mandatory = $false, HelpMessage = "Period of time to wait (in seconds) for the associated process to exit. Default is 15 seconds.")]
+        [Int32] $timeout = 15
     )
 
+    Write-Debug "Executing '$executable $arguments'"
+
     $pinfo = New-Object System.Diagnostics.ProcessStartInfo
     $pinfo.FileName = $executable
     $pinfo.RedirectStandardError = $true
@@ -283,7 +295,15 @@ function Invoke-ExecutableCommand {
     $p = New-Object System.Diagnostics.Process
     $p.StartInfo = $pinfo
     $p.Start() | Out-Null
-    $p.WaitForExit()
+    # Blocks the current thread of execution until the time has elapsed or the process has exited.
+    $p.WaitForExit($timeout * 1000) | Out-Null
+
+    if (-not $p.HasExited) {
+        Write-Debug "Execution did not complete in $timeout seconds."
+    }
+    else {
+        Write-Debug "Command execution completed. Exit code: $($p.ExitCode)"
+    }
 
     return $p
 }
@@ -301,3 +321,4 @@ Export-ModuleMember -Function Test-ServiceRegistered
 Export-ModuleMember -Function Add-FeatureToPath
 Export-ModuleMember -Function Remove-FeatureFromPath
 Export-ModuleMember -Function Invoke-ServiceAction
+Export-ModuleMember -Function Test-ConfFileEmpty

containers-toolkit/Public/BuildkitTools.psm1

@@ -230,7 +230,10 @@ function Register-BuildkitdService {
 
             # Check buildkitd service is already registered
             if (Test-ServiceRegistered -Service 'Buildkitd') {
-                Write-Warning "Buildkitd service already registered."
+                Write-Warning (-join @("buildkitd service already registered. To re-register the service, "
+                "stop the service by running 'Stop-Service buildkitd' or 'Stop-BuildkitdService', then "
+                "run 'buildkitd --unregister-service'. Wait for buildkitd service to be deregistered, "
+                "then re-reun this command."))
                 return
             }
 
@@ -404,14 +407,6 @@ function Uninstall-BuildkitHelper {
     Write-Output "Successfully uninstalled buildkit."
 }
 
-function Test-ConfFileEmpty($Path) {
-    if (!(Test-Path -LiteralPath $Path)) {
-        return $true
-    }
-
-    $isFileNotEmpty = (([System.IO.File]::ReadAllText($Path)) -match '\S')
-    return (-not $isFileNotEmpty )
-}
 
 function Get-ConsentToRegisterBuildkit ($path) {
     $retry = 2

containers-toolkit/Public/ContainerdTools.psm1

@@ -206,7 +206,10 @@ function Register-ContainerdService {
 
             # Check containerd service is already registered
             if (Test-ServiceRegistered -Service 'containerd') {
-                Write-Warning "Containerd service already registered."
+                Write-Warning (-join @("Containerd service already registered. To re-register the service, "
+                "stop the service by running 'Stop-Service containerd' or 'Stop-ContainerdService', then "
+                "run 'containerd --unregister-service'. Wait for containerd service to be deregistered, "
+                "then re-reun this command."))
                 return
             }
 
@@ -224,11 +227,20 @@ function Register-ContainerdService {
 
             Add-MpPreference -ExclusionProcess $containerdExecutable
 
-            #Configure containerd service
+            # Get default containerd config and write to file
             $containerdConfigFile = "$ContainerdPath\config.toml"
-            Invoke-ExecutableCommand -Executable $containerdExecutable -Arguments "config default" | `
-                Out-File $containerdConfigFile -Encoding ascii
-            Write-Information -InformationAction Continue -MessageData "Review containerd configutations at $containerdConfigFile"
+            Write-Debug "Containerd config file: $containerdConfigFile"
+
+            $output = Invoke-ExecutableCommand -Executable $containerdExecutable -Arguments "config default"
+            $output.StandardOutput.ReadToEnd() | Out-File -FilePath $containerdConfigFile  -Encoding ascii -Force
+            
+            # Check config file is not empty
+            $isEmptyConfig = Test-ConfFileEmpty -Path  "$containerdConfigFile"
+            if ($isEmptyConfig) {
+                Throw "Config file is empty. '$containerdConfigFile'"
+            }
+            
+            Write-Output "Review containerd configutations at $containerdConfigFile"
 
             # Register containerd service
             $output = Invoke-ExecutableCommand -Executable $containerdExecutable -Arguments "--register-service --log-level debug --service-name containerd --log-file `"$env:TEMP\containerd.log`""
@@ -366,6 +378,8 @@ function Unregister-Containerd ($containerdPath) {
         Throw "Could not unregister containerd service. $($output.StandardError.ReadToEnd())"
     }
     else {
+        # Wait for service to be unregistered
+        # Failure to wait causes "The specified service has been marked for deletion." error
         Start-Sleep -Seconds 15
     }
 }