Skip to content

feat: Use WMI to implement Volume API to reduce PowerShell overhead (library version) #372

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 18, 2025
Merged

feat: Use WMI to implement Volume API to reduce PowerShell overhead (library version) #372

merged 1 commit into from
Jul 18, 2025
+83,882 −2,285

Conversation

laozc
Copy link
Contributor

@laozc laozc commented Feb 24, 2025 •
edited
Loading

What type of PR is this?
/kind feature

What this PR does / why we need it:
This PR leverages of WMI interfaces to replace the PowerShell functions for Windows related storage operations,
which improve the overall performance of csi-proxy.

This is the same change as #360 to the library branch.

Which issue(s) this PR fixes:
Fixes #193

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 24, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @laozc. 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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 24, 2025
@laozc laozc marked this pull request as ready for review February 24, 2025 15:16
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 24, 2025
@k8s-ci-robot k8s-ci-robot requested a review from humblec February 24, 2025 15:17
@laozc laozc changed the title Use WMI instead of PowerShell for OS operations Use WMI instead of PowerShell for OS operations (library version) Feb 25, 2025
@laozc laozc changed the title Use WMI instead of PowerShell for OS operations (library version) Use WMI to implement Volume API to reduce PowerShell overhead (library version) Apr 18, 2025
@laozc laozc force-pushed the wmi-library branch 2 times, most recently from cc13a71 to 48f468b Compare April 21, 2025 12:48
@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Apr 22, 2025
@laozc laozc changed the title Use WMI to implement Volume API to reduce PowerShell overhead (library version) feat: Use WMI to implement Volume API to reduce PowerShell overhead (library version) May 8, 2025
@mauriciopoppe
Copy link
Member

mauriciopoppe commented May 9, 2025 •
edited
Loading

Thanks a lot for porting the codebase to the library version! For the changes in the library version would they happen after the binary changes?

Another note, feel free to add me as reviewer in your PRs, you can achieve that by writing a comment in a new PR description like:

/cc @mauriciopoppe

@andyzhangx
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 3, 2025
@laozc laozc force-pushed the wmi-library branch 4 times, most recently from 70e3f77 to 24a83d6 Compare July 12, 2025 01:44
@laozc laozc force-pushed the wmi-library branch 12 times, most recently from 624bcb1 to 4400efb Compare July 17, 2025 10:58
@laozc
Copy link
Contributor Author

laozc commented Jul 18, 2025

/cc @mauriciopoppe

andyzhangx
andyzhangx approved these changes Jul 18, 2025
View reviewed changes
Copy link
Member

@andyzhangx andyzhangx left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 18, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, laozc

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 Jul 18, 2025
@k8s-ci-robot k8s-ci-robot merged commit cd2f235 into kubernetes-csi:library-development Jul 18, 2025
7 checks passed
mauriciopoppe
mauriciopoppe reviewed Jul 18, 2025
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mauriciopoppe mauriciopoppe mauriciopoppe left review comments

@andyzhangx andyzhangx andyzhangx approved these changes

@jingxu97 jingxu97 Awaiting requested review from jingxu97

@RaunakShah RaunakShah Awaiting requested review from RaunakShah

@humblec humblec Awaiting requested review from humblec

Assignees

@andyzhangx andyzhangx

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/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@laozc @k8s-ci-robot @mauriciopoppe @andyzhangx