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

Pass Pod information in CSI calls #603

Open
jsafrane opened this Issue Jul 31, 2018 · 27 comments

Comments

Projects
None yet
@jsafrane
Member

jsafrane commented Jul 31, 2018

Feature Description

  • One-line feature description (can be used as a release note): CSI driver may opt-in to receive information about Pod that requested a volume in NodePublish request.

  • Primary contact (assignee): @jsafrane

  • Responsible SIGs: @kubernetes/sig-storage-feature-requests @kubernetes/sig-node-feature-requests

  • Design proposal link (community repo): kubernetes/community#2439

  • Link to e2e and/or unit tests: TBD

  • Reviewer(s) - (for LGTM) recommend having 2+ reviewers (at least one from code-area OWNERS file) agreed to review. Reviewers from multiple companies preferred: @gnufied @msau42

  • Approver (likely from SIG/area to which feature belongs): @saad-ali

  • Feature target (which target equals to which milestone):

    • Alpha release target (x.y): 1.12
    • Beta release target (x.y): 1.13
    • Stable release target (x.y): 1.14

Tagging sig-node since it will probably need a new gRPC API in kubelet. Proposal will follow soon. Proposal has been created.

@justaugustus

This comment has been minimized.

Member

justaugustus commented Jul 31, 2018

Thanks @jsafrane! This has been added to the 1.12 tracking spreadsheet.

/assign @jsafrane
/stage alpha

@zparnold

This comment has been minimized.

Member

zparnold commented Aug 20, 2018

Hey there! @jsafrane I'm the wrangler for the Docs this release. Is there any chance I could have you open up a docs PR against the release-1.12 branch as a placeholder? That gives us more confidence in the feature shipping in this release and gives me something to work with when we start doing reviews/edits. Thanks! If this feature does not require docs, could you please update the features tracking spreadsheet to reflect it?

@justaugustus

This comment has been minimized.

Member

justaugustus commented Sep 5, 2018

@jsafrane --
Any update on docs status for this feature? Are we still planning to land it for 1.12?
At this point, code freeze is upon us, and docs are due on 9/7 (2 days).
If we don't here anything back regarding this feature ASAP, we'll need to remove it from the milestone.

cc: @zparnold @jimangel @tfogo

@msau42

This comment has been minimized.

Member

msau42 commented Sep 5, 2018

@kfox1111

This comment has been minimized.

kfox1111 commented Sep 6, 2018

can we pass through volume.Name too? looks like it is already exposed indirectly to the driver by way of the mount path here:
https://github.com/kubernetes/kubernetes/blob/master/pkg/volume/csi/csi_mounter.go#L79
I'd rather use a passed version then parse it out.

@gnufied

This comment has been minimized.

Member

gnufied commented Sep 6, 2018

This feature is on track for 1.12. I will open placeholder PR for docs shortly.

@kfox1111 you mean PersistentVolume.Name field?

@justaugustus

This comment has been minimized.

Member

justaugustus commented Sep 6, 2018

@gnufied -- Please keep us posted. Docs PR is overdue at this point.

@kfox1111

This comment has been minimized.

kfox1111 commented Sep 6, 2018

@kfox1111

This comment has been minimized.

kfox1111 commented Sep 6, 2018

Skimming the code, I think its adding the line: "csi.storage.k8s.io/pod.volumeName": c.specVolumeID,

@tfogo

This comment has been minimized.

Member

tfogo commented Sep 6, 2018

Hi @gnufied, do you have an update on the Docs PR? Please let us know as soon as you have a PR open.

@gnufied

This comment has been minimized.

Member

gnufied commented Sep 6, 2018

@tfogo now that I think of, traditionally CSI+Kubernetes interface has been documented https://kubernetes-csi.github.io/docs/ rather than on community website. This PR change does not affect any end users and is mostly applicable for CSI plugin authors.

I am wondering if we even need a docs PR to community website. cc @saad-ali

@tfogo

This comment has been minimized.

Member

tfogo commented Sep 6, 2018

That makes sense. In that case, it's totally fine not to have a PR open in k/website. (Though you should make sure the appropriate docs land in k-csi/docs.)

@justaugustus

This comment has been minimized.

Member

justaugustus commented Sep 7, 2018

Thanks for the update!

@saad-ali

This comment has been minimized.

Member

saad-ali commented Sep 7, 2018

Agreed with @gnufied. This change should be documented in kubernetes-csi as it is relevant to CSI driver authors not end users. Tracking that work here.

@ameukam

This comment has been minimized.

Contributor

ameukam commented Oct 5, 2018

Hi folks,
Kubernetes 1.13 is going to be a 'stable' release since the cycle is only 10 weeks. We encourage no big alpha features and only consider adding this feature if you have a high level of confidence it will make code slush by 11/09. Are there plans for this enhancement to graduate to alpha/beta/stable within the 1.13 release cycle? If not, can you please remove it from the 1.12 milestone or add it to 1.13?

We are also now encouraging that every new enhancement aligns with a KEP. If a KEP has been created, please link to it in the original post. Please take the opportunity to develop a KEP.

@kacole2

This comment has been minimized.

Contributor

kacole2 commented Oct 8, 2018

Hi @jsafrane is this enhancement going to graduate to beta in 1.13? Need a confirmation so it can be tracked properly. thanks!

@jsafrane

This comment has been minimized.

Member

jsafrane commented Oct 12, 2018

Yes, we want beta in 1.13

@kacole2

This comment has been minimized.

Contributor

kacole2 commented Oct 12, 2018

/milestone v1.13

@k8s-ci-robot k8s-ci-robot modified the milestones: v1.12, v1.13 Oct 12, 2018

@kacole2 kacole2 added the stage/beta label Oct 16, 2018

@kacole2 kacole2 removed the stage/alpha label Oct 16, 2018

@AishSundar

This comment has been minimized.

AishSundar commented Oct 17, 2018

@jsafrane what work is left for this to land in Beta in 1.13? Is there a list of pending PRs we are tracking for this (code, test and docs)? is kubernetes/community#2273 the only pending PR?

@ameukam

This comment has been minimized.

Contributor

ameukam commented Nov 7, 2018

Hi @jsafrane, I'm Enhancements Shadow for the 1.13 release. As asked by @AishSundar can you please mention the remaining PRs for this enhancement so the Release Team can have a follow up ?

Code slush begins on 11/9 and code freeze is 11/15.

Thank you!

@msau42

This comment has been minimized.

Member

msau42 commented Nov 7, 2018

The pending PRs are:

cc @bertinatto

@AishSundar

This comment has been minimized.

AishSundar commented Nov 12, 2018

@msau42 @jsafrane we are already in slush and fast approaching Code freeze for 1.13 this friday 11/16. Do you think this feature is still on track for 1.13?

I see both the PRs listed above is active review state and I am more concerned about things splling into freeze. We would like for all pending work to be completed early this week (EOD Wednesday), giving us atleast a couple fo days to watch and stabilize CI. Do you think this is feasible?

@AishSundar

This comment has been minimized.

AishSundar commented Nov 12, 2018

Also can you plz point us to the draft Docs PR?

@msau42

This comment has been minimized.

Member

msau42 commented Nov 12, 2018

@msau42

This comment has been minimized.

Member

msau42 commented Nov 14, 2018

I just discussed with @saad-ali and @davidz627 and we decided to not move to beta this release, and instead focus on CSI GA.

I've updated this comment above with all the tasks we need to do for beta.

@AishSundar

This comment has been minimized.

AishSundar commented Nov 14, 2018

@kacole to move this out of tracking for 1.13

@tfogo, @marpaia and @kbarnard10 to remove it from their updates respectively

@kacole2

This comment has been minimized.

Contributor

kacole2 commented Nov 14, 2018

/milestone clear

@k8s-ci-robot k8s-ci-robot removed this from the v1.13 milestone Nov 14, 2018

@kacole2 kacole2 added tracked/no and removed tracked/yes labels Nov 14, 2018

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