[WIP] Convert Disk APIs implementation to syscalls

[pradeep-hegde]

What type of PR is this?
/kind feature

What this PR does / why we need it:
Attempt to convert Disk APIs implementation to syscall alternatives

Which issue(s) this PR fixes:

Special notes for your reviewer:
Its WIP change to attempt to convert PowerShell commands to syscalls for Disk APIs. Few APIs of syscall are not running as expected. Reviews and suggestions are welcomed

Does this PR introduce a user-facing change?:

NONE

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pradeep-hegde
To complete the pull request process, please assign jsafrane after the PR has been reviewed.
You can assign the PR to them by writing /assign @jsafrane in a comment when ready.

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

Needs approval from an approver in each of these files:

• OWNERS

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]

Hi @pradeep-hegde. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[xing-yang]

/assign @jingxu97

[xing-yang]

/assign @mauriciopoppe

[xing-yang]

/assign @ddebroy

[mauriciopoppe]

/ok-to-test

[mauriciopoppe]

I see a few TODOs in the PR, is it still a WIP?

[pradeep-hegde]

I see a few TODOs in the PR, is it still a WIP?

Yes, it's WIP. Without those fixes, APIs won't be ready with the new implementation. I am unsure how to fix those and still looking for a solution to it. So raised PR to get initial reviews and suggestions for those TODOs

[mauriciopoppe]

I'm not familiar with Windows syscalls

• how did you get the code? did you translate it from an existing codebase in a different language?
• none of these functions are used in the API, how do we make sure that they're working?

[pradeep-hegde]

• how did you get the code? did you translate it from an existing codebase in a different language?

I referred to a few existing DiskAPIs in CSI proxy

csi-proxy/pkg/os/disk/api.go

Line 180 in 713bbca

func (DiskAPI) GetDiskNumber(disk syscall.Handle) (uint32, error) {

There is Windows Doc for all supported Disk APIs through DeviceIoControl function with ioControlCodes
https://docs.microsoft.com/en-us/windows/win32/fileio/disk-management-control-codes
@ddebroy helped to get some of the windows docs for ioControlCodes

• none of these functions are used in the API, how do we make sure that they're working?

Yes, I am trying to have similar functions of Disk APIs with syscall implementation. Before raising PR, I tried to unit test each syscall in Windows Server 2019 to validate the functions
I am not sure if there are unit tests that can validate PowerShell alternative functions. I can give it a try.
Idea is to have the same APIs working with a new implementation. Do you think it's better to create a new API instead?

[mauriciopoppe]

Idea is to have the same APIs working with a new implementation. Do you think it's better to create a new API instead?

Agreed on using a new impl with the existing APIs, creating new APIs is painful and I don't think it's worth it at this point, you could change the impl call pkg/server/disk/server.go or forward the call through the existing APIs in pkg/os/disk/api.go

I am not sure if there are unit tests that can validate PowerShell alternative functions. I can give it a try.

We have a few integration tests for disks that run in pull-kubernetes-csi-csi-proxy-integration, when the job runs after the impl is changed please look at the output, while we tried to have a good coverage I'm not sure about an exact number, please look at integrationtests/disk_v1_test.go

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.