-
Notifications
You must be signed in to change notification settings - Fork 248
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
add doc for TokenRequests and RequiresRepublish fields #383
Conversation
zshihang
commented
Nov 12, 2020
•
edited
Loading
edited
Welcome @zshihang! |
/cc @msau42 |
book/src/csi-driver-object.md
Outdated
@@ -70,6 +75,19 @@ There are four important fields: | |||
The default is `Persistent`, which is the normal PVC/PV mechanism. `Ephemeral` enables | |||
[inline ephemeral volumes](ephemeral-local-volumes.md) in addition (when both | |||
are listed) or instead of normal volumes (when it is the only entry in the list). | |||
* `tokenRequests` | |||
* This field was added in Kubernetes 1.20 and cannot be set when using an older Kubernetes release. | |||
* This field is alpha. |
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.
Can you specify the feature gate that is required as well?
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.
done
book/src/csi-driver-object.md
Outdated
* If this field is specified, Kubelet will plumb down the bound service acocunt tokens of the pod as `volume_context` in the `NodePublishVolume`: | ||
* `"csi.storage.k8s.io/serviceAccount.tokens": {"gcp":{"token":"<token>","expirationTimestamp":"<expiration timestamp in RFC3339>"}}` | ||
* If CSI driver doesn't find token recorded in the `volume_context`, it should return error in `NodePublishVolume` to inform Kubelet to retry. | ||
* Audiences should be distinct, otherwise the validation will fail. If the audience is "", it means the issued token has the same audience as kube-apiserver. |
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.
Is there some general documentation on tokens and audiences that we can point to from here? It may be good to specify what kinds of drivers/use cases this would be used for
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.
Actually this kind of information might be useful under a feature page: https://kubernetes-csi.github.io/docs/features.html, rather than hidden inside this object page, wdyt?
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.
yes, https://tools.ietf.org/html/rfc7519#section-4.1.3. i'll include this in the feature page.
book/src/csi-driver-object.md
Outdated
* Audiences should be distinct, otherwise the validation will fail. If the audience is "", it means the issued token has the same audience as kube-apiserver. | ||
* `requiresRepublish` | ||
* This field was added in Kubernetes 1.20 and cannot be set when using an older Kubernetes release. | ||
* This field is alpha. |
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.
mention the feature gate
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.
done
book/src/csi-driver-object.md
Outdated
* This field is alpha. | ||
* If this field is `true`, Kubelet will periodically call `NodePublishVolume`. This is useful in the following scenarios: | ||
* If the volume mounted by CSI driver is short-lived. | ||
* If CSI driver requires valid service account tokens (enabled by the field `tokenRequests`) repeatedly. |
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.
Can you add that note about how the republish should only update data, and not remount?
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.
done.
cc @mikedanese |
book/src/token-requests.md
Outdated
- **`RequiresRepublish`**: | ||
|
||
- This field is optional. | ||
- If this is true, `NodePublishVolume` will be periodically called. |
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.
Also mention that when used with TokenRequest, the token will be updated when it expires
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.
done
book/src/token-requests.md
Outdated
|
||
```go | ||
"csi.storage.k8s.io/serviceAccount.tokens": { | ||
'audience': { |
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.
Is the value actually "audience" or is it something like "gcp", "vault"?
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.
right. change it to <audience>
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
/retest mdbook hash matched locally. re-run in case transient failure. |
@zshihang: Cannot trigger testing until a trusted user reviews the PR and leaves an 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. |
/ok-to-test |
@zshihang: Cannot trigger testing until a trusted user reviews the PR and leaves an 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. |
/ok-to-test |
/retest |
@zshihang can you try force pushing to retry the ci? |
/lgtm |