Refactor Cond.rs

[YJDoc2]

As discussed in #14 , we can rename cond to pipe as it is more understandable. If you won't mind, I would like to refactor it and its occurrences. Reason to open new issue is that it is more than just documentation, so didn't want to add it in #14. you're fine with it, assign me this issue.
Another good reason to do this, as the module comment for it indicates it is a

conditional variable performing busy waiting on lock

but it is actually related to unix pipes, and the name can cause more confusion later on.

[utam0k]

@YJDoc2 I also thought that this refactoring was necessary.
I thought that I could make good use of Read Trait and Write Trait.
I hope you will give it a try.

[YJDoc2]

Hey I was trying to run integration tests before I make any changes, but I am running into some issues :
Initially I could not run the test because of some issues with go :
opencontainers/runtime-tools#718 and
golang/go#30868

after solving those, Now I am having problem, where there are errors while running tests :

Running default/default.t
Running linux_cgroups_devices/linux_cgroups_devices.t
Running linux_cgroups_hugetlb/linux_cgroups_hugetlb.t
failed to create the container
...
[DEBUG src/cgroups/v1/devices.rs:14] 2021-06-11T15:47:46.345001163+05:30 Apply Devices cgroup config
[DEBUG src/cgroups/v1/hugetlb.rs:20] 2021-06-11T15:47:46.345238501+05:30 Apply Hugetlb cgroup config
Error: page size must be in the format of 2^(integer)
Running linux_cgroups_pids/linux_cgroups_pids.t
Running linux_cgroups_memory/linux_cgroups_memory.t
failed to create the container
...

and then several no such file errors like this :

DEBUG src/capabilities.rs:23] 2021-06-11T15:47:48.807657175+05:30 dropping bounding capabilities to [LinuxCapabilityType { cap: CAP_CHOWN }, LinuxCapabilityType { cap: CAP_DAC_OVERRIDE }, LinuxCapabilityType { cap: CAP_FSETID }, LinuxCapabilityType { cap: CAP_FOWNER }, LinuxCapabilityType { cap: CAP_MKNOD }, LinuxCapabilityType { cap: CAP_NET_RAW }, LinuxCapabilityType { cap: CAP_SETGID }, LinuxCapabilityType { cap: CAP_SETUID }, LinuxCapabilityType { cap: CAP_SETFCAP }, LinuxCapabilityType { cap: CAP_SETPCAP }, LinuxCapabilityType { cap: CAP_NET_BIND_SERVICE }, LinuxCapabilityType { cap: CAP_SYS_CHROOT }, LinuxCapabilityType { cap: CAP_KILL }, LinuxCapabilityType { cap: CAP_AUDIT_WRITE }]
[WARN src/capabilities.rs:27] 2021-06-11T15:47:48.807732778+05:30 CAP_PERFMON doesn't support.
[WARN src/capabilities.rs:27] 2021-06-11T15:47:48.807757300+05:30 CAP_CHECKPOINT_RESTORE doesn't support.
[WARN src/capabilities.rs:27] 2021-06-11T15:47:48.807778697+05:30 CAP_BPF doesn't support.
[DEBUG src/process/init.rs:24] 2021-06-11T15:47:48.808027987+05:30 init send to child [1]
[DEBUG src/process/child.rs:58] 2021-06-11T15:47:48.808132984+05:30 child send to parent [0]
[DEBUG src/process/fork.rs:67] 2021-06-11T15:47:48.808263875+05:30 init pid is 77913
[DEBUG src/cgroups/v1/blkio.rs:25] 2021-06-11T15:47:48.808308406+05:30 Apply blkio cgroup config
[DEBUG src/cgroups/v1/memory.rs:29] 2021-06-11T15:47:48.808378659+05:30 Apply Memory cgroup config
Error: No such file or directory (os error 2)
failed to create the container

Can you help me to check this?

[utam0k]

@YJDoc2
It is possible that the environment is causing the failure.
We are considering testing this to handle a variety of environments.
#39

[YJDoc2]

That could be one reason, but I am running Ubuntu 20.04 LTS, same distro as used to run the github action tests, so it should work the same, as action on a PR seem to pass

[YJDoc2]

Hey, I checked it some more, and found out that

[DEBUG src/cgroups/v1/devices.rs:14] 2021-06-11T15:47:46.345001163+05:30 Apply Devices cgroup config
[DEBUG src/cgroups/v1/hugetlb.rs:20] 2021-06-11T15:47:46.345238501+05:30 Apply Hugetlb cgroup config
Error: page size must be in the format of 2^(integer)

This was in fact expected, and not a failed test.
The memory test error :

Running linux_cgroups_memory/linux_cgroups_memory.t
failed to create the container

Is due to the fact that the kernel I am using is not configured with swap limit capabilities, similar to this podman issue :
containers/podman#6365
Rest of the issues might be due to the similar reasons, which I'll try to check

[utam0k]

@YJDoc2 I tried it in the same environment as yours and it failed not only with youki but also with runc and crun for the same reason.
It seems that writing to memory.memsw.limit_in_bytes fails because the kernel CONFIG_MEMCG_SWAP_ENABLED is not set.

[YJDoc2]

Yes, I tried to run it with runc as well yesterday, and that's how I found out there was an issue with my system and not in youki. That is why I have asked in #39 about how we can create a better testing env so that we can reliably run integration tests, waiting for @tsturzl 's reply on that. I think till then I won't do heavy refactoring of this, apart from necessary name changes etc. as currently the only way I can be sure that my changes doesn't break anything is to push a commit and run github action to verify it doesn't break anything. So maybe #39 should have a higher priority before this.

[utam0k]

@YJDoc2
It is easy to ignore the error on the youki side and make it a warning, but I am not sure if youki should do that for this problem. I also looked at the specs, but didn't find anything like that.
To be honest, I think this is a runtime-tools problem. In other words, I think this is a problem that should be handled by CRI.
I apologize for the inconvenience. I'm implementing my youki's integration tests for this, so it's something I'd like to take into account there.
#56

I know it's not a very good move, but how about using /boot/config-$(uname -r) in integration_test.sh to check the configuration and skip the test?

[YJDoc2]

I think for short term we can do that, but for long term we will need to port all test into rust to merge them into cargo test, and then we can have self contained testing, but I think that will be complex, as well as we will need to keep up with changes in OCI runtime tools. I think opening an issue on OCI tools to see what they have to say on the issue of kernel configuration would be a good start to determine long-term solution plan. What is you opinion on opening an issue ?

[utam0k]

@YJDoc2
I believe this will be a significant problem in the future, and @minakawa-daiki is now implementing it for us. I think it will take some time to implement, but we are on our way to a solution.

I think for short term we can do that, but for long term we will need to port all test into rust to merge them into cargo test, and then we can have self contained testing, but I think that will be complex, as well as we will need to keep up with changes in OCI runtime tools.

I agree that creating an issue is a good idea. Let's try it. However, since it is not within our control, we may not be able to expect much response.

I think opening an issue on OCI tools to see what they have to say on the issue of kernel configuration would be a good start to determine long-term solution plan. What is you opinion on opening an issue ?

[YJDoc2]

I was checking out their issues, and quite a few of them seem to be not responded by the repo. Still, I think it will be worth a try. How about about something along the lines of :

Hello, we were using the runtime-tools for testing a new runtime. While running validation tests,
we found that the tests depend on kernel configuration, and if certain configuration is not met 
even runc and crun fails those test on said kernels. Is there any specific 
kernel configuration required by these tests, and if so, is there any document in repo which specifies it? 
Thanks

If you want, you can open the issue, otherwise I'd be happy to, let me know. Thanks

[utam0k]

@YJDoc2
I thoud it would be better to add some examples .
If you don't want to make an issue, I can make one for you, what do you want?

[YJDoc2]

Opened opencontainers/runtime-tools#721 for this, sorry for the delay 😓

[yihuaf]

Is this still relevant now that the code changed so much? Maybe we can close this now?

[utam0k]

There are no problems.