From 0e83a68159111e4ee510f5aa56d47ba97bda60c7 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 3 Aug 2023 08:45:47 -0700 Subject: [PATCH 1/2] When using Powershell all strings must be passed as env vars Signed-off-by: James Sturtevant Co-authored-by: Mark Rossetti --- pkg/os/volume/api.go | 73 ++++++++++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 26 deletions(-) diff --git a/pkg/os/volume/api.go b/pkg/os/volume/api.go index ee59f8d4..a3b7a550 100644 --- a/pkg/os/volume/api.go +++ b/pkg/os/volume/api.go @@ -62,8 +62,9 @@ func New() VolumeAPI { } func getVolumeSize(volumeID string) (int64, error) { - cmd := fmt.Sprintf("(Get-Volume -UniqueId \"%s\" | Get-partition).Size", volumeID) - out, err := utils.RunPowershellCmd(cmd) + cmd := `(Get-Volume -UniqueId "$Env:volumeID" | Get-partition).Size` + cmdEnv := fmt.Sprintf("volumeID=%s", volumeID) + out, err := utils.RunPowershellCmd(cmd, cmdEnv) if err != nil || len(out) == 0 { return -1, fmt.Errorf("error getting size of the partition from mount. cmd %s, output: %s, error: %v", cmd, string(out), err) @@ -98,8 +99,9 @@ func (VolumeAPI) ListVolumesOnDisk(diskNumber uint32, partitionNumber uint32) (v // FormatVolume - Formats a volume with the NTFS format. func (VolumeAPI) FormatVolume(volumeID string) (err error) { - cmd := fmt.Sprintf("Get-Volume -UniqueId \"%s\" | Format-Volume -FileSystem ntfs -Confirm:$false", volumeID) - out, err := utils.RunPowershellCmd(cmd) + cmd := `Get-Volume -UniqueId "$Env:volumeID" | Format-Volume -FileSystem ntfs -Confirm:$false` + cmdEnv := fmt.Sprintf("volumeID=%s", volumeID) + out, err := utils.RunPowershellCmd(cmd, cmdEnv) if err != nil { return fmt.Errorf("error formatting volume. cmd: %s, output: %s, error: %v", cmd, string(out), err) } @@ -114,8 +116,9 @@ func (VolumeAPI) WriteVolumeCache(volumeID string) (err error) { // IsVolumeFormatted - Check if the volume is formatted with the pre specified filesystem(typically ntfs). func (VolumeAPI) IsVolumeFormatted(volumeID string) (bool, error) { - cmd := fmt.Sprintf("(Get-Volume -UniqueId \"%s\" -ErrorAction Stop).FileSystemType", volumeID) - out, err := utils.RunPowershellCmd(cmd) + cmd := `(Get-Volume -UniqueId "$Env:volumeID" -ErrorAction Stop).FileSystemType` + cmdEnv := fmt.Sprintf("volumeID=%s", volumeID) + out, err := utils.RunPowershellCmd(cmd, cmdEnv) if err != nil { return false, fmt.Errorf("error checking if volume is formatted. cmd: %s, output: %s, error: %v", cmd, string(out), err) } @@ -128,8 +131,12 @@ func (VolumeAPI) IsVolumeFormatted(volumeID string) (bool, error) { // MountVolume - mounts a volume to a path. This is done using the Add-PartitionAccessPath for presenting the volume via a path. func (VolumeAPI) MountVolume(volumeID, path string) error { - cmd := fmt.Sprintf("Get-Volume -UniqueId \"%s\" | Get-Partition | Add-PartitionAccessPath -AccessPath %s", volumeID, path) - out, err := utils.RunPowershellCmd(cmd) + cmd := `Get-Volume -UniqueId "$Env:volumeID" | Get-Partition | Add-PartitionAccessPath -AccessPath $Env:mountpath` + cmdEnv := []string{} + cmdEnv = append(cmdEnv, fmt.Sprintf("volumeID=%s", volumeID)) + cmdEnv = append(cmdEnv, fmt.Sprintf("mountpath=%s", path)) + out, err := utils.RunPowershellCmd(cmd, cmdEnv...) + if err != nil { return fmt.Errorf("error mount volume to path. cmd: %s, output: %s, error: %v", cmd, string(out), err) } @@ -141,8 +148,13 @@ func (VolumeAPI) UnmountVolume(volumeID, path string) error { if err := writeCache(volumeID); err != nil { return err } - cmd := fmt.Sprintf("Get-Volume -UniqueId \"%s\" | Get-Partition | Remove-PartitionAccessPath -AccessPath %s", volumeID, path) - out, err := utils.RunPowershellCmd(cmd) + + cmd := `Get-Volume -UniqueId "$Env:volumeID" | Get-Partition | Remove-PartitionAccessPath -AccessPath $Env:mountpath` + cmdEnv := []string{} + cmdEnv = append(cmdEnv, fmt.Sprintf("volumeID=%s", volumeID)) + cmdEnv = append(cmdEnv, fmt.Sprintf("mountpath=%s", path)) + out, err := utils.RunPowershellCmd(cmd, cmdEnv...) + if err != nil { return fmt.Errorf("error getting driver letter to mount volume. cmd: %s, output: %s,error: %v", cmd, string(out), err) } @@ -158,8 +170,9 @@ func (VolumeAPI) ResizeVolume(volumeID string, size int64) error { var finalSize int64 var outString string if size == 0 { - cmd = fmt.Sprintf("Get-Volume -UniqueId \"%s\" | Get-partition | Get-PartitionSupportedSize | Select SizeMax | ConvertTo-Json", volumeID) - out, err := utils.RunPowershellCmd(cmd) + cmd = `Get-Volume -UniqueId "$Env:volumeID" | Get-partition | Get-PartitionSupportedSize | Select SizeMax | ConvertTo-Json` + cmdEnv := fmt.Sprintf("volumeID=%s", volumeID) + out, err := utils.RunPowershellCmd(cmd, cmdEnv) if err != nil || len(out) == 0 { return fmt.Errorf("error getting sizemin,sizemax from mount. cmd: %s, output: %s, error: %v", cmd, string(out), err) @@ -190,8 +203,10 @@ func (VolumeAPI) ResizeVolume(volumeID string, size int64) error { return nil } - cmd = fmt.Sprintf("Get-Volume -UniqueId \"%s\" | Get-Partition | Resize-Partition -Size %d", volumeID, finalSize) - out, err = utils.RunPowershellCmd(cmd) + cmd = fmt.Sprintf(`Get-Volume -UniqueId "$Env:volumeID" | Get-Partition | Resize-Partition -Size %d`, finalSize) + cmdEnv := []string{} + cmdEnv = append(cmdEnv, fmt.Sprintf("volumeID=%s", volumeID)) + out, err = utils.RunPowershellCmd(cmd, cmdEnv...) if err != nil { return fmt.Errorf("error resizing volume. cmd: %s, output: %s size:%v, finalSize %v, error: %v", cmd, string(out), size, finalSize, err) } @@ -201,8 +216,9 @@ func (VolumeAPI) ResizeVolume(volumeID string, size int64) error { // GetVolumeStats - retrieves the volume stats for a given volume func (VolumeAPI) GetVolumeStats(volumeID string) (int64, int64, error) { // get the size and sizeRemaining for the volume - cmd := fmt.Sprintf("(Get-Volume -UniqueId \"%s\" | Select SizeRemaining,Size) | ConvertTo-Json", volumeID) - out, err := utils.RunPowershellCmd(cmd) + cmd := `(Get-Volume -UniqueId "$Env:volumeID" | Select SizeRemaining,Size) | ConvertTo-Json` + cmdEnv := fmt.Sprintf("volumeID=%s", volumeID) + out, err := utils.RunPowershellCmd(cmd, cmdEnv) if err != nil { return -1, -1, fmt.Errorf("error getting capacity and used size of volume. cmd: %s, output: %s, error: %v", cmd, string(out), err) @@ -227,8 +243,9 @@ func (VolumeAPI) GetVolumeStats(volumeID string) (int64, int64, error) { // GetDiskNumberFromVolumeID - gets the disk number where the volume is. func (VolumeAPI) GetDiskNumberFromVolumeID(volumeID string) (uint32, error) { // get the size and sizeRemaining for the volume - cmd := fmt.Sprintf("(Get-Volume -UniqueId \"%s\" | Get-Partition).DiskNumber", volumeID) - out, err := utils.RunPowershellCmd(cmd) + cmd := `(Get-Volume -UniqueId "$Env:volumeID" | Get-Partition).DiskNumber` + cmdEnv := fmt.Sprintf("volumeID=%s", volumeID) + out, err := utils.RunPowershellCmd(cmd, cmdEnv) if err != nil || len(out) == 0 { return 0, fmt.Errorf("error getting disk number. cmd: %s, output: %s, error: %v", cmd, string(out), err) @@ -261,8 +278,9 @@ func (VolumeAPI) GetVolumeIDFromTargetPath(mount string) (string, error) { } func getTarget(mount string) (string, error) { - cmd := fmt.Sprintf("(Get-Item -Path %s).Target", mount) - out, err := utils.RunPowershellCmd(cmd) + cmd := `(Get-Item -Path $Env:mountpath).Target` + cmdEnv := fmt.Sprintf("mountpath=%s", mount) + out, err := utils.RunPowershellCmd(cmd, cmdEnv) if err != nil || len(out) == 0 { return "", fmt.Errorf("error getting volume from mount. cmd: %s, output: %s, error: %v", cmd, string(out), err) } @@ -352,8 +370,9 @@ func ensureVolumePrefix(volume string) string { // dereferenceSymlink dereferences the symlink `path` and returns the stdout. func dereferenceSymlink(path string) (string, error) { - cmd := fmt.Sprintf(`(Get-Item -Path %s).Target`, path) - out, err := utils.RunPowershellCmd(cmd) + cmd := `(Get-Item -Path $Env:linkpath).Target` + cmdEnv := fmt.Sprintf("linkpath=%s", path) + out, err := utils.RunPowershellCmd(cmd, cmdEnv) if err != nil { return "", err } @@ -368,8 +387,9 @@ func getVolumeForDriveLetter(path string) (string, error) { return "", fmt.Errorf("The path=%s is not a valid DriverLetter", path) } - cmd := fmt.Sprintf(`(Get-Partition -DriveLetter %s | Get-Volume).UniqueId`, path) - out, err := utils.RunPowershellCmd(cmd) + cmd := `(Get-Partition -DriveLetter $Env:drivepath | Get-Volume).UniqueId` + cmdEnv := fmt.Sprintf("drivepath=%s", path) + out, err := utils.RunPowershellCmd(cmd, cmdEnv) if err != nil { return "", err } @@ -379,8 +399,9 @@ func getVolumeForDriveLetter(path string) (string, error) { } func writeCache(volumeID string) error { - cmd := fmt.Sprintf("Get-Volume -UniqueId \"%s\" | Write-Volumecache", volumeID) - out, err := utils.RunPowershellCmd(cmd) + cmd := `Get-Volume -UniqueId "$Env:volumeID" | Write-Volumecache` + cmdEnv := fmt.Sprintf("volumeID=%s", volumeID) + out, err := utils.RunPowershellCmd(cmd, cmdEnv) if err != nil { return fmt.Errorf("error writing volume cache. cmd: %s, output: %s, error: %v", cmd, string(out), err) } From 2523e6674dedf3de27f84235efec28555da24664 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 3 Aug 2023 08:46:46 -0700 Subject: [PATCH 2/2] Enable tests than can now run on GH actions Signed-off-by: James Sturtevant --- .github/workflows/windows.yml | 5 ++++- integrationtests/disk_v1_test.go | 4 ---- integrationtests/disk_v1alpha1_test.go | 4 ---- integrationtests/disk_v1beta1_test.go | 4 ---- integrationtests/disk_v1beta2_test.go | 4 ---- integrationtests/disk_v1beta3_test.go | 4 ---- integrationtests/volume_test.go | 10 ++-------- 7 files changed, 6 insertions(+), 29 deletions(-) diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index c218ce7e..33214784 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -20,6 +20,9 @@ jobs: go build -v -a -o ./bin/csi-proxy-api-gen.exe ./cmd/csi-proxy-api-gen - name: Run Windows Integration Tests run: | + # required for running Volume and Disk tests + Install-WindowsFeature -name Hyper-V-PowerShell + # start the CSI Proxy before running tests on windows Start-Job -Name CSIProxy -ScriptBlock { .\bin\csi-proxy.exe @@ -28,7 +31,7 @@ jobs: Write-Output "getting named pipes" [System.IO.Directory]::GetFiles("\\.\\pipe\\") $env:CSI_PROXY_GH_ACTIONS="TRUE" - go test -v -race ./integrationtests/... + go test -timeout 20m -v -race ./integrationtests/... unit_tests: strategy: matrix: diff --git a/integrationtests/disk_v1_test.go b/integrationtests/disk_v1_test.go index 28724c2f..7888216e 100644 --- a/integrationtests/disk_v1_test.go +++ b/integrationtests/disk_v1_test.go @@ -77,8 +77,6 @@ func v1DiskTests(t *testing.T) { }) t.Run("Get/SetDiskState", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) - client, err := diskv1client.NewClient() require.NoError(t, err) @@ -144,8 +142,6 @@ func v1DiskTests(t *testing.T) { }) t.Run("PartitionDisk", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) - var err error client, err := diskv1client.NewClient() require.NoError(t, err) diff --git a/integrationtests/disk_v1alpha1_test.go b/integrationtests/disk_v1alpha1_test.go index d251656d..60e38bd0 100644 --- a/integrationtests/disk_v1alpha1_test.go +++ b/integrationtests/disk_v1alpha1_test.go @@ -34,8 +34,6 @@ func v1alpha1DiskTests(t *testing.T) { }) t.Run("Rescan", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) - client, err := diskv1alpha1client.NewClient() require.NoError(t, err) @@ -47,8 +45,6 @@ func v1alpha1DiskTests(t *testing.T) { }) t.Run("PartitionDisk", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) - var err error client, err := diskv1alpha1client.NewClient() require.NoError(t, err) diff --git a/integrationtests/disk_v1beta1_test.go b/integrationtests/disk_v1beta1_test.go index d0b37dcc..422ef55d 100644 --- a/integrationtests/disk_v1beta1_test.go +++ b/integrationtests/disk_v1beta1_test.go @@ -57,8 +57,6 @@ func v1beta1DiskTests(t *testing.T) { }) t.Run("Get/SetDiskState", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) - client, err := diskv1beta1client.NewClient() require.NoError(t, err) @@ -86,8 +84,6 @@ func v1beta1DiskTests(t *testing.T) { }) t.Run("PartitionDisk", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) - var err error client, err := diskv1beta1client.NewClient() require.NoError(t, err) diff --git a/integrationtests/disk_v1beta2_test.go b/integrationtests/disk_v1beta2_test.go index 8c431b6a..1848b561 100644 --- a/integrationtests/disk_v1beta2_test.go +++ b/integrationtests/disk_v1beta2_test.go @@ -58,8 +58,6 @@ func v1beta2DiskTests(t *testing.T) { }) t.Run("Get/SetDiskState", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) - client, err := diskv1beta2client.NewClient() require.NoError(t, err) @@ -127,8 +125,6 @@ func v1beta2DiskTests(t *testing.T) { }) t.Run("PartitionDisk", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) - var err error client, err := diskv1beta2client.NewClient() require.NoError(t, err) diff --git a/integrationtests/disk_v1beta3_test.go b/integrationtests/disk_v1beta3_test.go index 57e7a262..6b1e9c8a 100644 --- a/integrationtests/disk_v1beta3_test.go +++ b/integrationtests/disk_v1beta3_test.go @@ -77,8 +77,6 @@ func v1beta3DiskTests(t *testing.T) { }) t.Run("Get/SetDiskState", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) - client, err := diskv1beta3client.NewClient() require.NoError(t, err) @@ -144,8 +142,6 @@ func v1beta3DiskTests(t *testing.T) { }) t.Run("PartitionDisk", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) - var err error client, err := diskv1beta3client.NewClient() require.NoError(t, err) diff --git a/integrationtests/volume_test.go b/integrationtests/volume_test.go index afd99b70..d03d00eb 100644 --- a/integrationtests/volume_test.go +++ b/integrationtests/volume_test.go @@ -148,32 +148,26 @@ func TestVolumeAPIs(t *testing.T) { negativeVolumeTests(t) }) - // TODO: These tests will fail on Github Actions because Hyper-V is disabled - // see https://github.com/actions/virtual-environments/pull/2525 + // Github Actions WS 2022 supports hyper-v + // must install management tools manually // these tests should be considered frozen from the API point of view t.Run("v1alpha1Tests", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) v1alpha1VolumeTests(t) }) t.Run("v1beta1Tests", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) v1beta1VolumeTests(t) }) t.Run("v1beta2Tests", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) v1beta2VolumeTests(t) }) t.Run("v1beta3Tests", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) v1beta3VolumeTests(t) }) t.Run("v1Tests", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) v1VolumeTests(t) }) t.Run("v2alpha1Tests", func(t *testing.T) { - skipTestOnCondition(t, isRunningOnGhActions()) v2alpha1VolumeTests(t) }) }