Conversation
6af7953 to
b14b3c5
Compare
|
Hello @saku3, This should be done in terms of implementation. I'm just curious what I should add the integration tests. I have one test where it just checks if a container with a time namespace can be created at all. |
d24abea to
5250fc2
Compare
|
Works on my machine :(, using the same binary as the one built by the ci above. Weird fluke? Logs aren't helpful. |
|
Thank you for the implementation. The |
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
b0128d9 to
72e02b4
Compare
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
|
Integration tests are done, it should be worth checking this out now. |
|
@CheatCodeSam |
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
|
Good to go |
|
@saku3 Could I ask you to review this PR as the first reviewer? |
|
Sure, I can review it. |
saku3
left a comment
There was a problem hiding this comment.
Thank you for creating this PR.
I’ve left a few comments, so I’d appreciate it if you could take a look.
Also, based on my local testing, the behavior of exec didn’t seem to work correctly, so could you please check it?
$ youki run -b tutorial a
$ cat /proc/self/timens_offsets
monotonic 172800 33
boottime 604800 67
another terminal
$ youki exec a cat /proc/self/timens_offsets
monotonic 0 0
boottime 0 0
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
81a0e64 to
578211f
Compare
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
cac6e6c to
041368b
Compare
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
20f38c0 to
5859de4
Compare
Signed-off-by: Carson Weeks <mail@carsonweeks.com>
|
Works on my machine, :( This is ready for re-review |
There was a problem hiding this comment.
In dark mode, the black color blends into the background and is hard to see.
| pub fn validate_time_namespace(spec: &Spec) -> Result<(), LibcontainerError> { | ||
| let linux = match spec.linux() { | ||
| Some(l) => l, | ||
| None => return Ok(()), | ||
| }; | ||
|
|
||
| match linux.time_offsets() { | ||
| Some(s) if !s.is_empty() => s, | ||
| _ => return Ok(()), | ||
| }; | ||
|
|
||
| linux | ||
| .namespaces() | ||
| .as_ref() | ||
| .and_then(|nss| nss.iter().find(|ns| ns.typ() == LinuxNamespaceType::Time)) | ||
| .ok_or(LibcontainerError::InvalidSpec( | ||
| crate::error::ErrInvalidSpec::TimeNamespace, | ||
| ))?; | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
| pub fn validate_time_namespace(spec: &Spec) -> Result<(), LibcontainerError> { | |
| let linux = match spec.linux() { | |
| Some(l) => l, | |
| None => return Ok(()), | |
| }; | |
| match linux.time_offsets() { | |
| Some(s) if !s.is_empty() => s, | |
| _ => return Ok(()), | |
| }; | |
| linux | |
| .namespaces() | |
| .as_ref() | |
| .and_then(|nss| nss.iter().find(|ns| ns.typ() == LinuxNamespaceType::Time)) | |
| .ok_or(LibcontainerError::InvalidSpec( | |
| crate::error::ErrInvalidSpec::TimeNamespace, | |
| ))?; | |
| Ok(()) | |
| } | |
| pub fn validate_time_namespace(spec: &Spec) -> Result<(), LibcontainerError> { | |
| if let Some(linux) = spec.linux() { | |
| if linux.time_offsets().is_some() { | |
| linux | |
| .namespaces() | |
| .as_ref() | |
| .and_then(|nss| nss.iter().find(|ns| ns.typ() == LinuxNamespaceType::Time)) | |
| .ok_or(LibcontainerError::InvalidSpec( | |
| crate::error::ErrInvalidSpec::TimeNamespace, | |
| ))?; | |
| } | |
| } | |
| Ok(()) | |
| } |
There was a problem hiding this comment.
Thank you, I was trying to figure out a good way to do this
| // This must be done from the parent process since CAP_SYS_TIME is required. | ||
| if let Some(time_offsets) = time_offsets { | ||
| main_receiver.wait_for_time_offset_request()?; | ||
| setup_time_offsets(&time_offsets, intermediate_pid)?; |
There was a problem hiding this comment.
What if the main process is running rootless?
There was a problem hiding this comment.
Also, I'm not sure why the intermediate process doesn't writes the offset directly.
There was a problem hiding this comment.
Also, I'm not sure why the intermediate process doesn't writes the offset directly.
It follows runc’s implementation: even if we switch user namespaces, the owner of /proc/self/timens_offsets does not change, so the intermediate process cannot write to it and the write is done in the main process.
|
@CheatCodeSam From what I can see, it looks like only a small number of changes are needed. |
|
Hey, sorry I started a new job. I'll fix it this week |
@CheatCodeSam |
Description
Adding the time namespace.
Type of Change
Testing
Related Issues
Fixes #3259
Additional Context