Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename /internal to pkg, nested /internal/server/<group>/internal to impl #152

Merged
merged 4 commits into from
Jun 16, 2021

Conversation

mauriciopoppe
Copy link
Member

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind cleanup

What this PR does / why we need it:

For the upcoming move to Windows privileged containers we need to make some changes in the file structure of the codebase, internal folders can't be imported from other projects so we need to rename them, the new names are:

  • /internal to /pkg
  • /internal/server/<group>/internal to /pkg/server/<group>/impl

I also changed the code generator to create the generated files at the folders above

Does this PR introduce a user-facing change?:

NONE

/cc @jingxu97 @ddebroy

@k8s-ci-robot k8s-ci-robot requested review from ddebroy and jingxu97 June 15, 2021 17:30
@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 15, 2021
@mauriciopoppe
Copy link
Member Author

/hold

I'm going to run the E2E tests before marking this as ready

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 15, 2021
@mauriciopoppe mauriciopoppe force-pushed the rename-internal branch 3 times, most recently from e462ac2 to 63bc544 Compare June 15, 2021 21:02
@mauriciopoppe
Copy link
Member Author

All the integration tests passed! I made some changes that won't be in this PR that will be part of another PR, the changes to make the integration tests work are in 63bc544

PS C:\Users\mauriciopoppe\go\src\github.com\kubernetes-csi\csi-proxy> go test -v .\integrationtests\
=== RUN   TestAPIGroups
=== RUN   TestAPIGroups/happy_path_with_v1alpha1
=== RUN   TestAPIGroups/overflow_with_v1alpha1
=== RUN   TestAPIGroups/happy_path_with_v1alpha2
=== RUN   TestAPIGroups/overflow_with_v1alpha2
=== RUN   TestAPIGroups/with_v1
--- PASS: TestAPIGroups (120.11s)
    --- PASS: TestAPIGroups/happy_path_with_v1alpha1 (0.06s)
    --- PASS: TestAPIGroups/overflow_with_v1alpha1 (0.02s)
    --- PASS: TestAPIGroups/happy_path_with_v1alpha2 (0.02s)
    --- PASS: TestAPIGroups/overflow_with_v1alpha2 (0.01s)
    --- PASS: TestAPIGroups/with_v1 (0.01s)
=== RUN   TestNewAPIGroup
    csi_api_gen_test.go:24: Skipping csi-api-generator test (ref 139#)
--- SKIP: TestNewAPIGroup (0.00s)
=== RUN   TestDiskAPIGroup
=== RUN   TestDiskAPIGroup/v1beta3Tests
=== RUN   TestDiskAPIGroup/v1beta3Tests/ListDiskIDs,ListDiskLocations
    disk_v1beta3_test.go:40: diskIDsResponse=diskIDs:{key:0 value:{page83:"Google  persistent-disk-0" serial_number:"                    "}}
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c hostname"
    disk_v1beta3_test.go:72: listDiskLocationsResponse=disk_locations:{key:0 value:{Adapter:"0" Target:"1" LUNID:"0"}}
=== RUN   TestDiskAPIGroup/v1beta3Tests/Get/SetDiskState
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\lib\\kubelet\\plugins\\testplugin-61.csi.io\\mount-61"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:
\\var\\lib\\kubelet\\plugins\\testplugin-61.csi.io\\disk-61.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path
C:\\var\\lib\\kubelet\\plugins\\testplugin-61.csi.io\\disk-61.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C
:\\var\\lib\\kubelet\\plugins\\testplugin-61.csi.io\\disk-61.vhdx).DiskNumber"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Initialize-Disk -Number 1 -PartitionStyle GPT"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-Partition -DiskNumber 1 -UseMaximumSize"
    volume_test.go:129: Checking that the size is inside the bounds: 1052770304 < (actual) 1073741824 < 1073741824
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Disk -Number 1 | Set-Disk -IsOffline $true"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Disk -Number 1 | Select-Object -ExpandProperty IsOffline"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Disk -Number 1 | Select-Object -ExpandProperty IsOffline"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Dismount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-61.csi.io\\disk-61.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rm C:\\var\\lib\\kubelet\\plugins\\testplugin-61.csi.io\\disk-61.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\lib\\kubelet\\plugins\\testplugin-61.csi.io\\mount-61"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\lib\\kubelet\\plugins\\testplugin-61.csi.io\\"
=== RUN   TestDiskAPIGroup/v1beta3Tests/PartitionDisk
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\lib\\kubelet\\plugins\\testplugin-21.csi.io\\mount-29"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:
\\var\\lib\\kubelet\\plugins\\testplugin-21.csi.io\\disk-69.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path
C:\\var\\lib\\kubelet\\plugins\\testplugin-21.csi.io\\disk-69.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C
:\\var\\lib\\kubelet\\plugins\\testplugin-21.csi.io\\disk-69.vhdx).DiskNumber"
=== RUN   TestDiskAPIGroup/v1beta2Tests
=== RUN   TestDiskAPIGroup/v1beta2Tests/ListDiskIDs,ListDiskLocations
    disk_v1beta2_test.go:37: diskIDsResponse=diskIDs:<key:"0" value:<identifiers:<key:"page83" value:"Google  persistent-disk-0" > identifiers:<key:"serialNumber" value:"                    " > > > diskIDs:<key:"1" value:<identifiers:<key:"page83" value:"4d534654202020205d069acaddc11b41bb89b51fd6e51461" > identifiers:<key:"serialNumber" value:"" > > >
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c hostname"
    disk_v1beta2_test.go:69: listDiskLocationsResponse=disk_locations:<key:"0" value:<Adapter:"0" Target:"1" LUNID:"0" > >
=== RUN   TestDiskAPIGroup/v1beta2Tests/Get/SetDiskState
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\lib\\kubelet\\plugins\\testplugin-672.csi.io\\mount-672"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-672.csi.io\\disk-672.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path
C:\\var\\lib\\kubelet\\plugins\\testplugin-672.csi.io\\disk-672.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C
:\\var\\lib\\kubelet\\plugins\\testplugin-672.csi.io\\disk-672.vhdx).DiskNumber"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Initialize-Disk
-Number 2 -PartitionStyle GPT"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-Partition -DiskNumber 2 -UseMaximumSize"
    volume_test.go:129: Checking that the size is inside the bounds: 1052770304 < (actual) 1073741824 < 1073741824
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Disk -Number 2 | Set-Disk -IsOffline $true"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Disk -Number 2 | Select-Object -ExpandProperty IsOffline"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Disk -Number 2 | Select-Object -ExpandProperty IsOffline"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Dismount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-672.csi.io\\disk-672.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rm C:\\var\\lib\\kubelet\\plugins\\testplugin-672.csi.io\\disk-672.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\lib\\kubelet\\plugins\\testplugin-672.csi.io\\mount-672"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\lib\\kubelet\\plugins\\testplugin-672.csi.io\\"
=== RUN   TestDiskAPIGroup/v1beta2Tests/PartitionDisk
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\l
ib\\kubelet\\plugins\\testplugin-53.csi.io\\mount-6"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:
\\var\\lib\\kubelet\\plugins\\testplugin-53.csi.io\\disk-51.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-53.csi.io\\disk-51.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-53.csi.io\\disk-51.vhdx).DiskNumber"
=== RUN   TestDiskAPIGroup/v1beta1Tests
=== RUN   TestDiskAPIGroup/v1beta1Tests/ListDiskIDs,ListDiskLocations
    disk_v1beta1_test.go:36: diskIDsResponse=diskIDs:<key:"0" value:<identifiers:<key:"page83" value:"Google  persistent-disk-0" > identifiers:<key:"serialNumber" value:"                    " > > > diskIDs:<key:"1" value:<identifiers:<key:"page83" value:"4d534654202020205d069acaddc11b41bb89b51fd6e51461" > identifiers:<key:"serialNumber" value:"" > > > diskID
s:<key:"2" value:<identifiers:<key:"page83" value:"4d534654202020202c1c1fb5bf4bb94a99468ed09179f5f4" > identifiers:<key:
"serialNumber" value:"" > > >
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c hostname"
    disk_v1beta1_test.go:68: listDiskLocationsResponse=disk_locations:<key:"0" value:<Adapter:"0" Target:"1" LUNID:"0" >
 >
=== RUN   TestDiskAPIGroup/v1beta1Tests/Get/SetDiskState
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\lib\\kubelet\\plugins\\testplugin-543.csi.io\\mount-543"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-543.csi.io\\disk-543.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-543.csi.io\\disk-543.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-543.csi.io\\disk-543.vhdx).DiskNumber"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Initialize-Disk -Number 3 -PartitionStyle GPT"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-Partition -DiskNumber 3 -UseMaximumSize"
    volume_test.go:129: Checking that the size is inside the bounds: 1052770304 < (actual) 1073741824 < 1073741824
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Dismount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-543.csi.io\\disk-543.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rm C:\\var\\lib\\kubelet\\plugins\\testplugin-543.csi.io\\disk-543.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\lib\\kubelet\\plugins\\testplugin-543.csi.io\\mount-543"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\lib\\kubelet\\plugins\\testplugin-543.csi.io\\"
=== RUN   TestDiskAPIGroup/v1beta1Tests/PartitionDisk
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\lib\\kubelet\\plugins\\testplugin-80.csi.io\\mount-87"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-80.csi.io\\disk-70.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-80.csi.io\\disk-70.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-80.csi.io\\disk-70.vhdx).DiskNumber"
=== RUN   TestDiskAPIGroup/v1alpha1Tests
=== RUN   TestDiskAPIGroup/v1alpha1Tests/ListDiskLocations
    disk_v1alpha1_test.go:30: listDiskLocationsResponse=disk_locations:<key:"0" value:<Adapter:"0" Target:"1" LUNID:"0" > >
=== RUN   TestDiskAPIGroup/v1alpha1Tests/Rescan
=== RUN   TestDiskAPIGroup/v1alpha1Tests/PartitionDisk
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\lib\\kubelet\\plugins\\testplugin-68.csi.io\\mount-74"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-68.csi.io\\disk-53.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-68.csi.io\\disk-53.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-68.csi.io\\disk-53.vhdx).DiskNumber"
--- PASS: TestDiskAPIGroup (180.74s)
    --- PASS: TestDiskAPIGroup/v1beta3Tests (59.55s)
        --- PASS: TestDiskAPIGroup/v1beta3Tests/ListDiskIDs,ListDiskLocations (5.58s)
        --- PASS: TestDiskAPIGroup/v1beta3Tests/Get/SetDiskState (36.98s)
        --- PASS: TestDiskAPIGroup/v1beta3Tests/PartitionDisk (17.00s)
    --- PASS: TestDiskAPIGroup/v1beta2Tests (58.18s)
        --- PASS: TestDiskAPIGroup/v1beta2Tests/ListDiskIDs,ListDiskLocations (4.98s)
        --- PASS: TestDiskAPIGroup/v1beta2Tests/Get/SetDiskState (36.30s)
        --- PASS: TestDiskAPIGroup/v1beta2Tests/PartitionDisk (16.89s)
    --- PASS: TestDiskAPIGroup/v1beta1Tests (41.62s)
        --- PASS: TestDiskAPIGroup/v1beta1Tests/ListDiskIDs,ListDiskLocations (4.93s)
        --- PASS: TestDiskAPIGroup/v1beta1Tests/Get/SetDiskState (19.79s)
        --- PASS: TestDiskAPIGroup/v1beta1Tests/PartitionDisk (16.89s)
    --- PASS: TestDiskAPIGroup/v1alpha1Tests (21.39s)
        --- PASS: TestDiskAPIGroup/v1alpha1Tests/ListDiskLocations (2.32s)
        --- PASS: TestDiskAPIGroup/v1alpha1Tests/Rescan (2.24s)
        --- PASS: TestDiskAPIGroup/v1alpha1Tests/PartitionDisk (16.83s)
=== RUN   TestFilesystemAPIGroup
=== RUN   TestFilesystemAPIGroup/v1beta2FilesystemTests
=== RUN   TestFilesystemAPIGroup/v1beta2FilesystemTests/PathExists_positive
=== RUN   TestFilesystemAPIGroup/v1beta2FilesystemTests/IsMount
=== RUN   TestFilesystemAPIGroup/v1beta1FilesystemTests
=== RUN   TestFilesystemAPIGroup/v1beta1FilesystemTests/PathExists_positive
=== RUN   TestFilesystemAPIGroup/v1beta1FilesystemTests/IsMount
=== RUN   TestFilesystemAPIGroup/v1alpha1FilesystemTests
=== RUN   TestFilesystemAPIGroup/v1alpha1FilesystemTests/PathExists_positive
=== RUN   TestFilesystemAPIGroup/v1alpha1FilesystemTests/IsMount
--- PASS: TestFilesystemAPIGroup (0.68s)
    --- PASS: TestFilesystemAPIGroup/v1beta2FilesystemTests (0.24s)
        --- PASS: TestFilesystemAPIGroup/v1beta2FilesystemTests/PathExists_positive (0.18s)
        --- PASS: TestFilesystemAPIGroup/v1beta2FilesystemTests/IsMount (0.07s)
    --- PASS: TestFilesystemAPIGroup/v1beta1FilesystemTests (0.20s)
        --- PASS: TestFilesystemAPIGroup/v1beta1FilesystemTests/PathExists_positive (0.09s)
        --- PASS: TestFilesystemAPIGroup/v1beta1FilesystemTests/IsMount (0.11s)
    --- PASS: TestFilesystemAPIGroup/v1alpha1FilesystemTests (0.23s)
        --- PASS: TestFilesystemAPIGroup/v1alpha1FilesystemTests/PathExists_positive (0.11s)
        --- PASS: TestFilesystemAPIGroup/v1alpha1FilesystemTests/IsMount (0.12s)
=== RUN   TestIscsiAPIGroup
    utils.go:146: Skipping test
--- SKIP: TestIscsiAPIGroup (0.00s)
=== RUN   TestSmbAPIGroup
=== RUN   TestSmbAPIGroup/v1alpha1SmbTests
=== RUN   TestSmbAPIGroup/v1beta1SmbTests
=== RUN   TestSmbAPIGroup/v1beta2SmbTests
--- PASS: TestSmbAPIGroup (31.72s)
    --- PASS: TestSmbAPIGroup/v1alpha1SmbTests (13.09s)
    --- PASS: TestSmbAPIGroup/v1beta1SmbTests (9.27s)
    --- PASS: TestSmbAPIGroup/v1beta2SmbTests (9.37s)
=== RUN   TestGetBIOSSerialNumber
=== RUN   TestGetBIOSSerialNumber/GetBIOSSerialNumber
    system_test.go:31: The serial number is GoogleCloud-6E36B9EB2F74E25C3AB5037CC8BD3AD4
--- PASS: TestGetBIOSSerialNumber (0.43s)
    --- PASS: TestGetBIOSSerialNumber/GetBIOSSerialNumber (0.43s)
=== RUN   TestServiceCommands
=== RUN   TestServiceCommands/GetService
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Stop-Service -Name \"MSiSCSI\""
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Service -Name \"MSiSCSI\" | Select-Object -ExpandProperty Status"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Service -Nam
e \"MSiSCSI\" | Select-Object DisplayName, Status, StartType | ConvertTo-Json"
=== RUN   TestServiceCommands/Stop/Start_Service
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Stop-Service -Na
me \"MSiSCSI\""
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Service -Name \"MSiSCSI\" | Select-Object -ExpandProperty Status"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Service -Name \"MSiSCSI\" | Select-Object -ExpandProperty Status"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Get-Service -Name \"MSiSCSI\" | Select-Object -ExpandProperty Status"
--- PASS: TestServiceCommands (7.02s)
    --- PASS: TestServiceCommands/GetService (1.73s)
    --- PASS: TestServiceCommands/Stop/Start_Service (5.28s)
=== RUN   TestVolumeAPIs
=== RUN   TestVolumeAPIs/NegativeDiskTests
=== RUN   TestVolumeAPIs/NegativeVolumeTests
=== RUN   TestVolumeAPIs/v1alpha1Tests
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\lib\\kubelet\\plugins\\testplugin-257.csi.io\\mount-257"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-257.csi.io\\disk-257.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-257.csi.io\\disk-257.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-257.csi.io\\disk-257.vhdx).DiskNumber"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Initialize-Disk
-Number 5 -PartitionStyle GPT"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-Partition -D
iskNumber 5 -UseMaximumSize"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Resize-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-257.csi.io\\disk-257.vhdx -SizeBytes 2147483648"
    volume_v1alpha1_test.go:78: Attempt to resize volume from sizeBytes=1073741824 to sizeBytes=1610612736
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Dismount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-257.csi.io\\disk-257.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rm C:\\var\\lib\\kubelet\\plugins\\testplugin-257.csi.io\\disk-257.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\l
ib\\kubelet\\plugins\\testplugin-257.csi.io\\mount-257"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\l
ib\\kubelet\\plugins\\testplugin-257.csi.io\\"
=== RUN   TestVolumeAPIs/v1beta1Tests
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\lib\\kubelet\\plugins\\testplugin-401.csi.io\\mount-401"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-401.csi.io\\disk-401.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-401.csi.io\\disk-401.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-401.csi.io\\disk-401.vhdx).DiskNumber"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Initialize-Disk -Number 5 -PartitionStyle GPT"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-Partition -D
iskNumber 5 -UseMaximumSize"
    volume_test.go:129: Checking that the size is inside the bounds: 1052770304 < (actual) 1056894976 < 1073741824
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Resize-VHD -Path
 C:\\var\\lib\\kubelet\\plugins\\testplugin-401.csi.io\\disk-401.vhdx -SizeBytes 2147483648"
    volume_v1beta1_test.go:100: Attempt to resize volume from sizeBytes=1056894976 to sizeBytes=1585342464
    volume_test.go:129: Checking that the size is inside the bounds: 1564370944 < (actual) 1585340416 < 1585342464
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Dismount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-401.csi.io\\disk-401.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rm C:\\var\\lib\\kubelet\\plugins\\testplugin-401.csi.io\\disk-401.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\l
ib\\kubelet\\plugins\\testplugin-401.csi.io\\mount-401"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\l
ib\\kubelet\\plugins\\testplugin-401.csi.io\\"
=== RUN   TestVolumeAPIs/v1beta2Tests
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\lib\\kubelet\\plugins\\testplugin-814.csi.io\\mount-814"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-814.csi.io\\disk-814.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-814.csi.io\\disk-814.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-814.csi.io\\disk-814.vhdx).DiskNumber"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Initialize-Disk -Number 5 -PartitionStyle GPT"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-Partition -DiskNumber 5 -UseMaximumSize"
    volume_test.go:129: Checking that the size is inside the bounds: 1052770304 < (actual) 1056894976 < 1073741824
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Resize-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-814.csi.io\\disk-814.vhdx -SizeBytes 2147483648"
    volume_v1beta2_test.go:99: Attempt to resize volume from sizeBytes=1056894976 to sizeBytes=1585342464
    volume_test.go:129: Checking that the size is inside the bounds: 1564370944 < (actual) 1585340416 < 1585342464
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Dismount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-814.csi.io\\disk-814.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rm C:\\var\\lib\\kubelet\\plugins\\testplugin-814.csi.io\\disk-814.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\lib\\kubelet\\plugins\\testplugin-814.csi.io\\mount-814"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\lib\\kubelet\\plugins\\testplugin-814.csi.io\\"
=== RUN   TestVolumeAPIs/v1beta3Tests
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c mkdir C:\\var\\lib\\kubelet\\plugins\\testplugin-423.csi.io\\mount-423"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-423.csi.io\\disk-423.vhdx -SizeBytes 1073741824"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Mount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-423.csi.io\\disk-423.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c (Get-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-423.csi.io\\disk-423.vhdx).DiskNumber"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Initialize-Disk -Number 5 -PartitionStyle GPT"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c New-Partition -DiskNumber 5 -UseMaximumSize"
    volume_v1beta3_test.go:73: VolumeId \\?\Volume{4186403c-da61-48b7-80a0-6067f81fc215}\
    volume_test.go:129: Checking that the size is inside the bounds: 1052770304 < (actual) 1056894976 < 1073741824
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Resize-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-423.csi.io\\disk-423.vhdx -SizeBytes 2147483648"
    volume_v1beta3_test.go:112: Attempt to resize volume from sizeBytes=1056894976 to sizeBytes=1585342464
    volume_test.go:129: Checking that the size is inside the bounds: 1564370944 < (actual) 1585340416 < 1585342464
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c Dismount-VHD -Path C:\\var\\lib\\kubelet\\plugins\\testplugin-423.csi.io\\disk-423.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rm C:\\var\\lib\\kubelet\\plugins\\testplugin-423.csi.io\\disk-423.vhdx"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\lib\\kubelet\\plugins\\testplugin-423.csi.io\\mount-423"
    utils.go:160: Executing command: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe /c rmdir C:\\var\\lib\\kubelet\\plugins\\testplugin-423.csi.io\\"
--- PASS: TestVolumeAPIs (202.10s)
    --- PASS: TestVolumeAPIs/NegativeDiskTests (0.00s)
    --- PASS: TestVolumeAPIs/NegativeVolumeTests (10.01s)
    --- PASS: TestVolumeAPIs/v1alpha1Tests (41.64s)
    --- PASS: TestVolumeAPIs/v1beta1Tests (49.48s)
    --- PASS: TestVolumeAPIs/v1beta2Tests (48.31s)
    --- PASS: TestVolumeAPIs/v1beta3Tests (52.66s)
PASS
ok      github.com/kubernetes-csi/csi-proxy/integrationtests    543.205s

@mauriciopoppe
Copy link
Member Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 15, 2021
@jingxu97
Copy link
Contributor

/lgtm

@ddebroy could you confirm this change is ok?

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 15, 2021
@ddebroy
Copy link
Contributor

ddebroy commented Jun 16, 2021

Thanks a lot @mauriciopoppe for the PR and the changes. I have one suggestion: will it be possible to switch the impl directory to apis please? This allows the clients of the versioned APIs to specify an import with a path of apis rather than impl. Following is the rough structure we can move towards for direct API binding with privileged container support:

  1. Add a New() method to server_generated.go files (like pkg/server/disk/apis/v1beta3/server_generated.go):
import github.com/kubernetes-csi/csi-proxy/pkg/server/disk/apis
import "osdiskapis" github.com/kubernetes-csi/csi-proxy/pkg/os/disk
...
// new direct API invocation handler
func New() apis.VersionedAPI {
    srv, _ := NewServer(osdiskapis.New())
    return &versionedAPI{
        apiGroupServer: srv,
    }
}
...
// same as today for gRPC registered interface
func NewVersionedServer(apiGroupServer apis.ServerInterface) apis.VersionedAPI {	
    return &versionedAPI{		
        apiGroupServer: apiGroupServer,	
    }
}
  1. Now one of the integration tests - say Get/SetDiskState in integrationtests/disk_v1beta3_test.go - can invoke a versioned API like diskv1beta3apis.GetDiskStats directly (i.e. skipping gRPC) with the following:
import github.com/kubernetes-csi/csi-proxy/pkg/server/disk/apis/v1beta3
...
diskv1beta3apis := v1beta3.New()

@mauriciopoppe
Copy link
Member Author

mauriciopoppe commented Jun 16, 2021

I understand what you mean, in the solution you propose we'd still use the client APIs to create the request objects and we'd no longer use the gRPC server, some tradeoffs I imagine:

  • we'd still need the code in client, if there are changes to the client API we might need to create a new API version e.g. v2beta1 in the client code, in the conversion functions, etc.
  • we'd still be using conversion functions which are a really useful layer in the client-server approach but if csi-proxy becomes a library I don't think the conversion functions are needed anymore

Another approach is to use pkg/server/disk/ directly instead, we'd no longer need the conversion functions, the driver would control which version of the csi-proxy library it'd want to include using the go.mod file instead e.g.

// driver's go.mod
// importing csi-proxy instead of csi-proxy/client
require (
	github.com/kubernetes-csi/csi-proxy v1.0.0
)
// driver's usage e.g. PD CSI
import (
	disktypes "github.com/kubernetes-csi/csi-proxy/pkg/server/disk/impl"
	diskimpl "github.com/kubernetes-csi/csi-proxy/pkg/server/disk"
)

func (mounter *CSIProxyMounterV1) GetDevicePath(deviceName string, partition string, volumeKey string) (uint32, error) {
	listDiskIDsRequest := &disktypes.ListDiskIDsRequest{}
	listDiskIDsResponse, err := diskimpl.ListDiskIDs(context.Background(), listDiskIDsRequest)
	// ...
}

Something to note in this approach is that the concept of version is gone, from what I saw the API handler layer is using the version only for validation and the implementation inside pkg/os is not doing anything with it anyways, some tradeoffs of this approach:

  • there'd be another layer introduced in the client-server approach
// in the client-server approach the call stack would be
client -> 
  pkg/server/disk/apis/v1beta3/conversion_generated.go (conversion from versioned requests to internal structs) -> 
  pkg/server/disk/server.go (version checks only?) ->
  pkg/server/disk/api.go (new file, we'd move the code of server.go minus the version checks)

// the library approach would be
client -> 
  pkg/server/disk/api.go (new file, we'd move the code of server.go minus the version checks)
  • we'd need to provide API compatibility in the github.com/kubernetes-csi/csi-proxy/pkg/server/disk/impl layer, it already has some structs that we no longer need
  • we'd no longer use protobufs, this is nice for API compatibility changes but I'm not sure if protobufs were used this way in this project

cc @jingxu97

@ddebroy
Copy link
Contributor

ddebroy commented Jun 16, 2021

Discussed a little more with Mauricio and Jing.

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ddebroy, mauriciopoppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 16, 2021
@k8s-ci-robot k8s-ci-robot merged commit 4d2de61 into kubernetes-csi:master Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants