KEP-3857: Recursive Read-only (RRO) mounts #3858
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
the
size/XL
AkihiroSuda
force-pushed
the
rro
branch
2 times, most recently
from
478118b
to
761883a
Compare
AkihiroSuda
requested review from
thockin
and removed request for
derekwaynecarr and
dchen1107
|
@thockin |
|
Did you add something about status? #3858 (comment) """ We also probably need a way for nodes to indicate that they do support it, so a node status field? Not sure if there is precedent for this. @mrunalp @bobbypage might know. |
PTAL: #3858 (comment) |
k8s-ci-robot
added
size/XXL
Revert PR 9713, as it appeared to break the compatibility too much kubernetes/enhancements#3858 (comment) This reverts commit b2f254f. > Conflicts: > internal/cri/opts/spec_linux_opts.go Signed-off-by: Akihiro Suda <[email protected]>
I am curious how that decision was reached? I suspect you are right, in general (that almost always this is what people want) and yet to that tiny fraction of users who actually depend on the non-recursive behavior, you just broke their app. How do other projects think about that tradeoff? Kube is erring on the side of not breaking users (as did Linux by making RRO distinct from RO), but that has costs, too. |
Signed-off-by: Akihiro Suda <[email protected]>
|
@thockin For Docker, we can consider reverting the behavior when a client speaks an old API: |
Revert PR 9713, as it appeared to break the compatibility too much kubernetes/enhancements#3858 (comment) This reverts commit b2f254f. > Conflicts: > internal/cri/opts/spec_linux_opts.go Signed-off-by: Akihiro Suda <[email protected]>
I'm not in a position to say what Docker should do - I was asking a question, not offering an opinion :) |
Revert PR 9713, as it appeared to break the compatibility too much kubernetes/enhancements#3858 (comment) This reverts commit b2f254f. > Conflicts: > internal/cri/opts/spec_linux_opts.go Signed-off-by: Akihiro Suda <[email protected]>
|
/assign @johnbelamaric |
Revert PR 9713, as it appeared to break the compatibility too much kubernetes/enhancements#3858 (comment) This reverts commit b2f254f. > Conflicts: > internal/cri/opts/spec_linux_opts.go Signed-off-by: Akihiro Suda <[email protected]>
| that might indicate a serious problem? | ||
| --> | ||
|
|
||
| Look for an event saying indicating RRO is not supported by the runtime. |
There was a problem hiding this comment.
events are not easy for a cluster admin to observe. metrics are much better. So, a few things I can imagine we might need to look for:
- crashing kubelet (especially when the field is set)
- pod failures / errors from the runtime
- pod rejections
Imagine you enable this on a cluster with 100 different users. How do you know if it's causing a problem? If it is causing a problem for some users, but other users are relying on it, can you roll it back?
Now imagine you have 10,000 clusters. You enable this on 100 clusters. You want to know if you should enable it on more clusters. How can you tell if users are using it? How can you tell if it is working?
That's what we're trying to get to with some of these questions.
Note these are non-blocking for alpha.
| Yes, the feature is used if the following `jq` command prints non-zero number: | ||
|
|
||
| ```bash | ||
| kubectl get pods -A -o json | jq '[.items[].spec.containers[].volumeMounts[]? | select(.recursiveReadOnly)] | length' |
There was a problem hiding this comment.
see comment above discussing metrics and the "fleet admin" point of view
|
/approve Added some comments on the PRR sections that are for beta, but they are non-blocking for alpha. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AkihiroSuda, johnbelamaric, mrunalp, thockin 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 |
k8s-ci-robot
added
the
approved
|
/lgtm |
k8s-ci-robot
added
the
lgtm
Revert PR 9713, as it appeared to break the compatibility too much kubernetes/enhancements#3858 (comment) This reverts commit b2f254f. > Conflicts: > internal/cri/opts/spec_linux_opts.go Signed-off-by: Akihiro Suda <[email protected]>
Related:
nerdctl run -v /foo:/bar:rro,rprivatecontainerd/nerdctl#511MOUNT_ATTR_RDONLY+AT_RECURSIVEutil-linux/util-linux#1501