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

CSIServiceAccountToken going GA #2664

Merged
merged 1 commit into from
May 12, 2021
Merged

CSIServiceAccountToken going GA #2664

merged 1 commit into from
May 12, 2021

Conversation

zshihang
Copy link
Contributor

/cc @msau42

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 29, 2021
msau42
msau42 reviewed May 7, 2021
View reviewed changes
keps/sig-storage/1855-csi-driver-service-account-token/kep.yaml
status: implementable
stage: beta
latest-milestone: "v1.21"
stage: stable
Copy link
Member

Choose a reason for hiding this comment

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

In the README.md, can you also update the Implementation history?

For scalability, have we tested in a large cluster if the 1 token request per second is ok?

Copy link
Contributor Author

@zshihang zshihang May 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@liggitt do we have load test for bound tokens?

for this feature, the load is # of pods that have csi drivers * # of token requests in csi driver spec.

Copy link
Member

Choose a reason for hiding this comment

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

I think large numbers of pods with unique tokens is covered by existing scale tests, but @wojtek-t @mm4tt can confirm

Copy link

Choose a reason for hiding this comment

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

@marseel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done for implementation history.

re scalability, current scale test for access tokens is using secret-based tokens, which is different from the bound tokens here. however, since we enable the BoundServiceAccountTokenVolume by default since 1.21, every pod will have one projected token mounted. then the scale test for pods should cover this.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for confirming!

@msau42
Copy link
Member

msau42 commented May 7, 2021

/assign @deads2k

@deads2k
Copy link
Contributor

deads2k commented May 7, 2021

PRR still looks good (https://github.com/kubernetes/enhancements/blob/master/keps/sig-storage/1855-csi-driver-service-account-token/README.md#scalability)

/approve

@JamesLaverack
Copy link
Member

xref #2047

@JamesLaverack JamesLaverack mentioned this pull request May 11, 2021
Closed
20 tasks
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 12, 2021
@msau42
Copy link
Member

msau42 commented May 12, 2021

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, msau42, zshihang

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 May 12, 2021
@k8s-ci-robot k8s-ci-robot merged commit 8167f6d into kubernetes:master May 12, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.22 milestone May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@mm4tt mm4tt mm4tt left review comments

@msau42 msau42 msau42 left review comments

Assignees

@deads2k deads2k

@msau42 msau42

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/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.22
Development

Successfully merging this pull request may close these issues.
7 participants
@zshihang @msau42 @deads2k @JamesLaverack @k8s-ci-robot @liggitt @mm4tt