-
Notifications
You must be signed in to change notification settings - Fork 150
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
Migrate to csi proxy v2 alpha #1071
Migrate to csi proxy v2 alpha #1071
Conversation
Hi @alexander-ding. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
/cc @mauriciopoppe |
/ok-to-test There's an optional presubmit job for Windows, please look for it in the test-infra codebase |
It was not /test pull-gcp-compute-persistent-disk-csi-driver-e2e-win2019 :( |
/test pull-gcp-compute-persistent-disk-csi-driver-e2e-windows-2019 |
I haven't run /test pull-gcp-compute-persistent-disk-csi-driver-e2e-windows-2019 in a while so it might have drifted from the CI one, I'd take that one and compare it with the presubmit |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/assign @mattcary
securityContext: | ||
windowsOptions: | ||
hostProcess: true | ||
runAsUserName: "NT AUTHORITY\\SYSTEM" # TODO: customize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove TODO
Dockerfile.Windows
Outdated
@@ -24,6 +24,8 @@ RUN cd /code/ && GOARCH=$(echo $TARGETPLATFORM | cut -f2 -d '/') GCE_PD_CSI_STAG | |||
|
|||
FROM ${BASE_IMAGE} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A followup item is to use the new base image that sig-windows created in https://static.sched.com/hosted_files/kccncna2022/c6/HPC-talk.pdf
Dockerfile.Windows
Outdated
@@ -24,6 +24,8 @@ RUN cd /code/ && GOARCH=$(echo $TARGETPLATFORM | cut -f2 -d '/') GCE_PD_CSI_STAG | |||
|
|||
FROM ${BASE_IMAGE} | |||
LABEL description="PD CSI driver" | |||
ENV PATH="${PATH};c:\\Windows\\System32\\WindowsPowerShell\\v1.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might not be needed after the new base image
The last run https://testgrid.k8s.io/provider-gcp-compute-persistent-disk-csi-driver#windows-2019-presubmit-gcp-compute-persistent-disk-csi-driver is red but here it's green, probably you can check again after removing the TODO |
One more thing, I created a few tracking issues a while ago in this repo too, I think we should use those kubernetes-csi/csi-proxy#217 tracks the issues in this repo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the release note it should say something along the lines of:
Migrated Windows CSI node plugin to HostProcess containers
Action required: Ensure that the k8s version is 1.23+ and set the security policies
in the cluster to enable the HostProcess container feature only to system workloads
53baf47
to
e6b3ba1
Compare
New changes are detected. LGTM label has been removed. |
/test pull-gcp-compute-persistent-disk-csi-driver-e2e-windows-2019 |
Thanks for the PR @alexander-ding ! /approve I'll let @mauriciopoppe give the final lgtm when all the comments are fixed. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexander-ding, mattcary 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 |
@alexander-ding: PR needs rebase. 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. |
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
/remove-lifecycle rotten |
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
@k8s-triage-robot: Closed this PR. In response to this:
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. |
/reopen |
@mauriciopoppe: Reopened this PR. In response to this:
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. |
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
@k8s-triage-robot: Closed this PR. In response to this:
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. |
What type of PR is this?
What this PR does / why we need it:
Migrates GCP Compute PD CSI Driver to use CSI Proxy v2. Besides updating the Go package code to use the new library, we also update the deployment files to run the driver as HostProcess containers. See the linked issue.
Which issue(s) this PR fixes:
Fixes #1070
Special notes for your reviewer:
This is a work in progress. We still need to update documentation to reflect the increased minimal Kubernetes version.
Does this PR introduce a user-facing change?: