KEP-127: Mark as implementable (target phase I for 1.25) #3275
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
the
kind/kep
k8s-ci-robot
added
the
sig/node
|
@mrunalp @SergeyKanzhelev @thockin friendly ping? I'll be away until next week, but it will be great if you can have a look. |
|
/cc |
|
@mrunalp could we get another review? |
|
if I use I've reverted back to using |
|
cc |
|
I reviewed these changes from a Windows perspective and it would require some OS changes to be able to support something like this on Windows and even if those changes were in place things would work differently. |
|
@marosset Perfect, thanks a lot! For reviewers, please note that the CRI changes live all inside the linux-specific part. They are all inside the LinuxPodSandboxConfig field of the PodSandboxConfig message. (in particular PodSandboxConfig.linux.security_context.namespace_options) |
rata
changed the title
k8s-ci-robot
added
size/L
rata
force-pushed
the
rata/userns
branch
2 times, most recently
from
6f047d6 to
fa47967
Compare
k8s-ci-robot
added
the
lgtm
k8s-ci-robot
removed
the
lgtm
| @@ -250,6 +312,56 @@ limit the number of pods using user namespaces to `min(maxPods, 1024)`. This | |||
| leaves us plenty of host UID space free and this limits is probably never hit in | |||
| practice. See UNRESOLVED for more some UNRESOLVED info we still have on this. | |||
|
|
|||
| ##### pkg/volume changes for phase I | |||
There was a problem hiding this comment.
Comment to line 308 (follow up from your response to my comment):
The UID they choose is inside the user namespace, it doesn't affect the mapping picked on the host.
Since we are always allocating the same size, there is no fragmentation to worry about (in facts we just use a bitmap to store at runtime what ranges are allocated).
The picked range is stored in the file /var/lib/kubelet/pods/$POD/userns so the Kubelet can read all the allocated mappings if it restarts
Can you please add that information to the doc - I think it's actually a useful context for people who are not that familiar with details like me.
There was a problem hiding this comment.
Yes, good call. Thanks!
I've kept this as a different commit, as I think it might be way simpler in this case to review as a separate commit. The commit title is: KEP-127: Clarify how IDs will be used and stored
|
@wojtek-t Thanks a lot for the quick and detailed review! I think I addressed all. PTAL (left some comments open that were either unresolved waiting for Sergey or thought it might make the re-review easier if those are more visible. Feel free to close them your self if you consider them solved). |
rata
changed the title
rata
changed the title
k8s-ci-robot
added
the
lgtm
|
@rata - ok, I think it's good enough for Alpha (as I mentioned we will have more discussions for Beta, especially around monitoring and toubleshooting, but I think we're good for Alpha). Please squash the commits and I will approve. |
This adds the needed KEP metadata, CRI changes, etc. Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com> Co-authored-by: Giuseppe Scrivano <gscrivan@redhat.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
k8s-ci-robot
removed
the
lgtm
|
@wojtek-t squashed now, thanks! btw, for the next time, I think you can add the label tide/merge-method-squash and approve. We avoid the forced push, that removes @SergeyKanzhelev lgtm label, so we have to do that dance once more now. (of course, nothing important :)) |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dchen1107, mikebrow, rata, SergeyKanzhelev, wojtek-t 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
With @giuseppe we created a PoC implementation for containerd and CRIO to validate this changes, all turned out to work just fine. Volume support is also working as expected.
This updates the KEP for 1.25 as discussed in SIG-node. Only PRR is missing to mark as implementable.
Signed-off-by: Rodrigo Campos rodrigoca@microsoft.com
Co-authored-by: Giuseppe Scrivano gscrivan@redhat.com
Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com
cc @SergeyKanzhelev @mrunalp @thockin