Skip to content

Refactor Docker status check in install-k8s.ps1 - #37

Merged
saadqbal merged 1 commit into
mainfrom
develop
Mar 6, 2026
Merged

Refactor Docker status check in install-k8s.ps1#37
saadqbal merged 1 commit into
mainfrom
develop

Conversation

@saadqbal

@saadqbalsaadqbal commented Mar 6, 2026

Copy link
Copy Markdown
Contributor
  • Updated the method for checking Docker's running status to use a more concise syntax, improving readability and maintaining functionality.

Note

Medium Risk
Small but user-facing installer change: removing try/catch may alter behavior when the docker CLI is missing/unavailable, potentially causing the script to fail earlier during setup.

Overview
Refactors Docker readiness detection in the Windows installer by replacing try/catch { docker info ... } probes with a simpler docker info *>$null call that checks $LASTEXITCODE both initially and inside the startup wait loop.

This keeps the same overall flow (probe → launch Docker Desktop → poll until ready) while changing how errors/output from docker info are handled.

Written by Cursor Bugbot for commit 4b30204. This will update automatically on new commits. Configure here.

- Updated the method for checking Docker's running status to use a more concise syntax, improving readability and maintaining functionality.
@saadqbal
saadqbal merged commit 3158376 into mainMar 6, 2026
1 check passed

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Stale $LASTEXITCODE causes false Docker running detection
    • Reintroduced try/catch around both docker info checks so CommandNotFoundException no longer falls through to stale $LASTEXITCODE logic.

Create PR

Or push these changes by commenting:

@cursor push e1108e46e8
Preview (e1108e46e8)
diff --git a/scripts/install-k8s.ps1 b/scripts/install-k8s.ps1--- a/scripts/install-k8s.ps1+++ b/scripts/install-k8s.ps1@@ -417,7 +417,7 @@
}
$dockerRunning = $false
- docker info *>$null 2>&1; if ($LASTEXITCODE -eq 0) { $dockerRunning = $true }+ try { docker info *>$null 2>&1; if ($LASTEXITCODE -eq 0) { $dockerRunning = $true } } catch {}
if (-not $dockerRunning) {
Start-Process $dockerExe -ErrorAction SilentlyContinue
@@ -428,7 +428,7 @@
$f = 0
for ($i = 1; $i -le $maxWait; $i++) {
Start-Sleep -Seconds 3
- docker info *>$null 2>&1; if ($LASTEXITCODE -eq 0) { $dockerRunning = $true; break }+ try { docker info *>$null 2>&1; if ($LASTEXITCODE -eq 0) { $dockerRunning = $true; break } } catch {}
Write-Host "`r " -NoNewline
Write-Host $frames[$f] -ForegroundColor Cyan -NoNewline
Write-Host " Waiting for Docker..." -NoNewline
This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.


$dockerRunning = $false
try { docker info 2>&1 | Out-Null; $dockerRunning = ($LASTEXITCODE -eq 0) } catch {}
docker info *>$null 2>&1; if ($LASTEXITCODE -eq 0) { $dockerRunning = $true }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale $LASTEXITCODE causes false Docker running detection

High Severity

Removing the try/catch around docker info introduces a false-positive detection bug. When docker is not in PATH, PowerShell throws a CommandNotFoundException (statement-terminating, not script-terminating), so execution continues to the if ($LASTEXITCODE -eq 0) check. Since no native command actually ran, $LASTEXITCODE retains its previous value — likely 0 from a preceding winget install — causing $dockerRunning to be incorrectly set to $true. The old try/catch prevented this by skipping the $LASTEXITCODE check on exception.

Additional Locations (1)

Fix in CursorFix in Web

@saadqbalsaadqbal self-assigned this Apr 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@saadqbal