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.

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

[Feature blogpost][1.22] CSI Windows Support with CSI Proxy reaches GA #29105

Merged
merged 7 commits into from
Aug 9, 2021
Merged

[Feature blogpost][1.22] CSI Windows Support with CSI Proxy reaches GA #29105

merged 7 commits into from
Aug 9, 2021

Conversation

mauriciopoppe
Copy link
Member

@mauriciopoppe mauriciopoppe commented Jul 24, 2021 •
edited
Loading

@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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 24, 2021
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Jul 24, 2021
@mauriciopoppe mauriciopoppe mentioned this pull request Jul 24, 2021
Closed
@netlify
Copy link

netlify bot commented Jul 24, 2021 •
edited
Loading

✔️ Deploy Preview for kubernetes-io-main-staging ready!

🔨 Explore the source changes: 57e7b78

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/611165e1d0d27a00070c19fc

😎 Browse the preview: https://deploy-preview-29105--kubernetes-io-main-staging.netlify.app

@mauriciopoppe mauriciopoppe force-pushed the feature-blogpost-csi-windows-support branch 2 times, most recently from 906dcf8 to 5092ac7 Compare July 25, 2021 03:58
@mauriciopoppe mauriciopoppe force-pushed the feature-blogpost-csi-windows-support branch from 5092ac7 to 441fb57 Compare July 27, 2021 21:09
@mauriciopoppe
Copy link
Member Author

Original KEP https://github.com/kubernetes/enhancements/tree/master/keps/sig-windows/1122-windows-csi-support
KEP Update for GA kubernetes/enhancements#2737

/sig storage

@k8s-ci-robot k8s-ci-robot added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Jul 27, 2021
@mauriciopoppe mauriciopoppe changed the title [WIP][Feature blogpost] CSI Windows Support with CSI Proxy reaches GA [Feature blogpost] CSI Windows Support with CSI Proxy reaches GA Jul 27, 2021
@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 Jul 27, 2021
@mauriciopoppe mauriciopoppe force-pushed the feature-blogpost-csi-windows-support branch 2 times, most recently from 5adddb3 to 4a22a17 Compare July 27, 2021 21:32
@mauriciopoppe mauriciopoppe changed the title [Feature blogpost] CSI Windows Support with CSI Proxy reaches GA [Feature blogpost][1.22] CSI Windows Support with CSI Proxy reaches GA Jul 27, 2021
@mauriciopoppe
Copy link
Member Author

/milestone 1.22

@k8s-ci-robot
Copy link
Contributor

@mauriciopoppe: You must be a member of the kubernetes/website-milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Website milestone maintainers and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone 1.22

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.

sftim
sftim reviewed Jul 27, 2021
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Some informal / incomplete feedback.

content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
@mauriciopoppe mauriciopoppe force-pushed the feature-blogpost-csi-windows-support branch 5 times, most recently from f81b021 to fb2b792 Compare July 28, 2021 21:22
saad-ali
saad-ali suggested changes Jul 30, 2021
View reviewed changes
content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-07-27-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
@mauriciopoppe mauriciopoppe force-pushed the feature-blogpost-csi-windows-support branch from e29c7b9 to 02ebc79 Compare August 5, 2021 16:17
@jingxu97
Copy link
Contributor

jingxu97 commented Aug 5, 2021

/lgtm

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

LGTM label has been added.

Git tree hash: e9ef5228182d2b49e16d0faa7b46a80ee8ff8d81

chrisnegus
chrisnegus reviewed Aug 6, 2021
View reviewed changes
Copy link
Contributor

@chrisnegus chrisnegus left a comment

Choose a reason for hiding this comment

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

This blog reads really well. Nice job! I have a few small suggestions, none of which are critical.

content/en/blog/_posts/2021-08-05-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-05-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-05-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated

Before we reached GA we wanted to make sure that our API is simple and consistent. We went through an extensive [API review](https://docs.google.com/document/d/1sBP8f_mwV0N_xRRQQGDwHZpU_nTHtpFxdi7LKSUtgX0/edit#heading=h.inwrahdkakje) of the v1beta API groups where we made sure that the CSI Proxy API methods and messages are consistent with the naming conventions defined in the [CSI Spec](https://github.com/container-storage-interface/spec/blob/master/spec.md). As part of this effort we're graduating the [Disk](https://github.com/kubernetes-csi/csi-proxy/blob/master/docs/apis/disk_v1.md), [Filesystem](https://github.com/kubernetes-csi/csi-proxy/blob/master/docs/apis/filesystem_v1.md), [SMB](https://github.com/kubernetes-csi/csi-proxy/blob/master/docs/apis/smb_v1.md) and [Volume](https://github.com/kubernetes-csi/csi-proxy/blob/master/docs/apis/volume_v1.md) API groups to v1.

Windows system APIs to get additional information from the Windows nodes and support to mount iSCSI targets in Windows nodes are available as alpha APIs in the [System API](https://github.com/kubernetes-csi/csi-proxy/tree/v1.0.0/client/api/system/v1alpha1) and the [iSCSI API](https://github.com/kubernetes-csi/csi-proxy/tree/v1.0.0/client/api/iscsi/v1alpha2), these APIs will continue to be improved before we graduate them to v1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Windows system APIs to get additional information from the Windows nodes and support to mount iSCSI targets in Windows nodes are available as alpha APIs in the [System API](https://github.com/kubernetes-csi/csi-proxy/tree/v1.0.0/client/api/system/v1alpha1) and the [iSCSI API](https://github.com/kubernetes-csi/csi-proxy/tree/v1.0.0/client/api/iscsi/v1alpha2), these APIs will continue to be improved before we graduate them to v1.
Windows system APIs, to get additional information from the Windows nodes and support to mount iSCSI targets in Windows nodes, are available as alpha APIs in the [System API](https://github.com/kubernetes-csi/csi-proxy/tree/v1.0.0/client/api/system/v1alpha1) and the [iSCSI API](https://github.com/kubernetes-csi/csi-proxy/tree/v1.0.0/client/api/iscsi/v1alpha2). These APIs will continue to be improved before we graduate them to v1.

I'm had some trouble understanding this sentence. Consider my suggestion above to make it easier for a human to parse, if it represents what you mean.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to:

Additional Windows system APIs to get information from the Windows nodes and support to mount iSCSI targets in Windows nodesac

content/en/blog/_posts/2021-08-05-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2021
@mauriciopoppe
Copy link
Member Author

hey @mrbobbytables, I saw you in the OWNERS file, do you know who can approve/lgtm this PR?

mrbobbytables
mrbobbytables reviewed Aug 9, 2021
View reviewed changes
Copy link
Member

@mrbobbytables mrbobbytables left a comment

Choose a reason for hiding this comment

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

Overall looks good, the last little bit is updating the date and the file paths 👍

content/en/blog/_posts/2021-08-05-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-05-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-05-csi-windows-support-with-csi-proxy-reaches-ga.md Outdated Show resolved Hide resolved
@mrbobbytables
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrbobbytables

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 Aug 9, 2021
@jlbutler
Copy link
Contributor

jlbutler commented Aug 9, 2021

@mauriciopoppe Thanks for this! Can you please update publication date to August 25 from the feature blog schedule (in tracking sheet)? /cc @kunal-kushwaha

@jingxu97
Copy link
Contributor

jingxu97 commented Aug 9, 2021

/lgtm

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

LGTM label has been added.

Git tree hash: eb4cfd7a97651b6ef2ca900b5611e7689ad18570

@k8s-ci-robot k8s-ci-robot merged commit 409a5ef into kubernetes:main Aug 9, 2021
@mauriciopoppe
Copy link
Member Author

mauriciopoppe commented Aug 9, 2021 •
edited
Loading

@jlbutler this got merged right before I saw your comment! I'll create another PR to rollback the changes #29305

@mauriciopoppe mauriciopoppe mentioned this pull request Aug 9, 2021
Closed
@kunal-kushwaha
Copy link
Contributor

@mauriciopoppe Thanks for this! Can you please update publication date to August 25 from the feature blog schedule (in tracking sheet)? /cc @kunal-kushwaha

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sftim sftim sftim left review comments

@jingxu97 jingxu97 jingxu97 left review comments

@chrisnegus chrisnegus chrisnegus left review comments

@mrbobbytables mrbobbytables mrbobbytables left review comments

@saad-ali saad-ali saad-ali requested changes

@kbarnard10 kbarnard10 Awaiting requested review from kbarnard10

@onlydole onlydole Awaiting requested review from onlydole

Assignees

@jingxu97 jingxu97

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/blog Issues or PRs related to the Kubernetes Blog subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@mauriciopoppe @k8s-ci-robot @jingxu97 @mrbobbytables @jlbutler @kunal-kushwaha @saad-ali @chrisnegus @sftim