diff --git a/integration/windows/environment_test.go b/integration/windows/environment_test.go index 188a63e78..840afbe4e 100644 --- a/integration/windows/environment_test.go +++ b/integration/windows/environment_test.go @@ -35,6 +35,7 @@ func (e *WindowsEnvironment) ShrinkRootPartition() { sizeMinBuffer := 1 * GB cmdFmtString := "Get-Partition -DriveLetter C | Resize-Partition -Size $((Get-PartitionSupportedSize -DriveLetter C).SizeMin + %d)" + shrunkSuccessfully := false for i := 0; i < 5; i++ { cmd := fmt.Sprintf(cmdFmtString, sizeMinBuffer) stdout, stderr, exitCode, err := e.RunPowershellCommandWithOffsetAndResponses(cmd) @@ -72,8 +73,14 @@ func (e *WindowsEnvironment) ShrinkRootPartition() { } } + shrunkSuccessfully = true break } + + Expect(shrunkSuccessfully).WithOffset(1).To( + BeTrue(), + fmt.Sprintf("Failed to shrink root partition after 5 attempts; last command: %s", fmt.Sprintf(cmdFmtString, sizeMinBuffer)), + ) } func (e *WindowsEnvironment) EnsureRootPartitionAtMaxSize() { diff --git a/integration/windows/ephemeral_disk_test.go b/integration/windows/ephemeral_disk_test.go index a25c0dc3f..e3d4342f2 100644 --- a/integration/windows/ephemeral_disk_test.go +++ b/integration/windows/ephemeral_disk_test.go @@ -29,6 +29,10 @@ var _ = Describe("EphemeralDisk", func() { agent.EnsureDataDirDoesntExist() if diskLetter != "" { + Expect(diskLetter).NotTo(Equal("C"), + "diskLetter must never be the system partition (C:) — "+ + "this indicates the agent created the data-dir junction pointing to the root drive, "+ + "or AssignDriveLetter returned an unexpected value") agent.RunPowershellCommand(fmt.Sprintf("Remove-Partition -DriveLetter %s -Confirm:$false", diskLetter)) } if diskNumber != "0" { diff --git a/platform/windows/disk/partitioner.go b/platform/windows/disk/partitioner.go index c1549b493..b621e6623 100644 --- a/platform/windows/disk/partitioner.go +++ b/platform/windows/disk/partitioner.go @@ -5,6 +5,7 @@ import ( "regexp" "strconv" "strings" + "time" boshsys "github.com/cloudfoundry/bosh-utils/system" ) @@ -13,6 +14,11 @@ var diskNumberPattern = regexp.MustCompile(`^[0-9]+$`) type Partitioner struct { Runner boshsys.CmdRunner + + // DriveLetterPollInterval controls how long AssignDriveLetter waits between + // queries when Get-Partition reports no letter yet (DriveLetter == char(0)). + // Zero (the default) uses 1 second. Override in tests to avoid delays. + DriveLetterPollInterval time.Duration } // ParseDiskNumberString validates a non-negative decimal disk or partition index for Windows PowerShell -Number parameters. @@ -151,15 +157,45 @@ func (p *Partitioner) AssignDriveLetter(diskNumber, partitionNumber string) (str dn, pn, ) - stdout, _, _, err := p.ps(driveScript) - if err != nil { - return "", fmt.Errorf( - "failed to find drive letter for partition %s on disk %s: %s", - partitionNumber, - diskNumber, - err, - ) + // PowerShell's Partition.DriveLetter is a .NET char. When no letter has been + // assigned the property holds char(0) — the null character — which PowerShell + // prints as "\x00\r\n". If we returned that empty string the caller would call + // linker.Link with a bare ":" target; on Windows that resolves to the current + // directory on the default drive (typically C:\), silently creating a junction + // that points back to the system partition. + // + // Retry up to 10 times to tolerate transient WMI staleness after the access + // path is added. + pollInterval := p.DriveLetterPollInterval + if pollInterval == 0 { + pollInterval = time.Second } - return strings.TrimSpace(stdout), nil + for attempt := 0; attempt < 10; attempt++ { + stdout, _, _, err := p.ps(driveScript) + if err != nil { + return "", fmt.Errorf( + "failed to find drive letter for partition %s on disk %s: %s", + partitionNumber, + diskNumber, + err, + ) + } + + // The DriveLetter property is a char; an unassigned partition returns the null + // character (\x00). Strip it along with whitespace before checking. + letter := strings.Trim(strings.TrimSpace(stdout), "\x00") + if letter != "" { + return letter, nil + } + + time.Sleep(pollInterval) + } + + return "", fmt.Errorf( + "drive letter was not assigned to partition %s on disk %s after %d attempts", + partitionNumber, + diskNumber, + 10, + ) } diff --git a/platform/windows/disk/partitioner_test.go b/platform/windows/disk/partitioner_test.go index ccb03a0f7..a77335d85 100644 --- a/platform/windows/disk/partitioner_test.go +++ b/platform/windows/disk/partitioner_test.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "strings" + "time" "github.com/cloudfoundry/bosh-utils/system/fakes" . "github.com/onsi/ginkgo/v2" @@ -264,5 +265,45 @@ var _ = Describe("Partitioner", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(`AssignDriveLetter: invalid partition number "2;Invoke-Expression"`)) }) + + It("retries when Get-Partition reports char(0) before returning the valid letter", func() { + // Simulate WMI returning the null character on the first poll, then the + // real letter on the second. Without the retry the function would return "" + // and the caller would create a junction pointing at bare ":", which Windows + // resolves to the current directory on the default drive (often C:\). + partitioner.DriveLetterPollInterval = time.Millisecond + + addKey := strings.Join([]string{"-NoProfile", "-NonInteractive", "-Command", + `Add-PartitionAccessPath -DiskNumber 1 -PartitionNumber 2 -AssignDriveLetter`}, " ") + getKey := strings.Join([]string{"-NoProfile", "-NonInteractive", "-Command", + `Get-Partition -DiskNumber 1 -PartitionNumber 2 | Select-Object -ExpandProperty DriveLetter`}, " ") + + cmdRunner.AddCmdResult(addKey, fakes.FakeCmdResult{}) + cmdRunner.AddCmdResult(getKey, fakes.FakeCmdResult{Stdout: "\x00\n"}) // null char — unassigned + cmdRunner.AddCmdResult(getKey, fakes.FakeCmdResult{Stdout: "E\n"}) // assigned on second poll + + driveLetter, err := partitioner.AssignDriveLetter(diskNumber, partitionNumber) + Expect(err).NotTo(HaveOccurred()) + Expect(driveLetter).To(Equal("E")) + }) + + It("returns an error when DriveLetter stays null across all retry attempts", func() { + partitioner.DriveLetterPollInterval = time.Millisecond + + addKey := strings.Join([]string{"-NoProfile", "-NonInteractive", "-Command", + `Add-PartitionAccessPath -DiskNumber 1 -PartitionNumber 2 -AssignDriveLetter`}, " ") + getKey := strings.Join([]string{"-NoProfile", "-NonInteractive", "-Command", + `Get-Partition -DiskNumber 1 -PartitionNumber 2 | Select-Object -ExpandProperty DriveLetter`}, " ") + + cmdRunner.AddCmdResult(addKey, fakes.FakeCmdResult{}) + // Sticky so every Get-Partition call returns the null char. + cmdRunner.AddCmdResult(getKey, fakes.FakeCmdResult{Stdout: "\x00\n", Sticky: true}) + + _, err := partitioner.AssignDriveLetter(diskNumber, partitionNumber) + Expect(err).To(MatchError(ContainSubstring( + fmt.Sprintf("drive letter was not assigned to partition %s on disk %s after 10 attempts", + partitionNumber, diskNumber), + ))) + }) }) })