add ratime mount_setattr test

[lengrongfu]

Fixes: #1534

[lengrongfu]

# Start group mounts_recursive
1 / 17 : ratime_test : not ok
        container stderr was not empty, found : ERROR libcontainer::rootfs::mount: failed to mount Mount { destination: "/ratime", typ: None, source: Some("/tmp/ratime_dir"), options: Some(["rbind", "ratime"]) }: syscall
ERROR libcontainer::process::container_init_process: failed to prepare rootfs err=Mount(Syscall(Nix(EINVAL)))
ERROR libcontainer::process::container_intermediate_process: failed to initialize container process: failed to prepare rootfs
ERROR libcontainer::process::container_main_process: failed to wait for init ready: failed to receive. "waiting for init ready". BrokenChannel
ERROR libcontainer::container::builder_impl: failed to run container process err=Channel(ReceiveError { msg: "waiting for init ready", source: BrokenChannel })
ERROR youki: error in executing command: failed to receive. "waiting for init ready". BrokenChannel

ratime can't mount, need to check whether there is any problem with the implementation.

[codecov-commenter]

Codecov Report

Merging #2658 (7d1f5bd) into main (18f3e0b) will increase coverage by 0.00%.
Report is 4 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2658   +/-   ##
=======================================
  Coverage   65.21%   65.21%           
=======================================
  Files         133      133           
  Lines       16981    16981           
=======================================
+ Hits        11074    11075    +1     
+ Misses       5907     5906    -1     

[YJDoc2]

Hey @lengrongfu , I have also encountered somewhat similar errors in other mount tests when I was running locally. The Root Cause for them was that the /tmp dir is not mounted with the expected attributes by default, and thus when we create the container dir for testing there, it does not have the correct mount attributes and test fails.

Can you check if this issue also has same RC?

[lengrongfu]

Hey @lengrongfu , I have also encountered somewhat similar errors in other mount tests when I was running locally. The Root Cause for them was that the /tmp dir is not mounted with the expected attributes by default, and thus when we create the container dir for testing there, it does not have the correct mount attributes and test fails.

Can you check if this issue also has same RC?

ok, i will to check.

[lengrongfu]

@YJDoc2 I found that it has nothing to do with the /tmp directory. It may be related to the mount option.

[YJDoc2]

@YJDoc2 I found that it has nothing to do with the /tmp directory. It may be related to the mount option.

Hey, yes that's what I meant : the /tmp directory might not be mounted with the options we need. You can try checking cat /proc/mounts output and corresponding docs to see if that provides any help

[YJDoc2]

Hey @lengrongfu , did you find out anything on this?

[lengrongfu]

Hey @lengrongfu , did you find out anything on this?

Sorry, I didn't find any more information.

[YJDoc2]

Hey, @lengrongfu , the PR looks good, one very minor nitpick .

Also may I ask you for one change : Instead of adding the ratime bugfix in libcontainer in this PR, can you open a separate one just containing that fix, and then rebase this on top of it after merge? The reason I'm asking is so that in changelog, it gets properly added as a bug fix, and does not hide under tests, as this PR will go under test category.

[YJDoc2]

Suggested change

	if one_metadata.atime() == two_metadata.atime() {
	if one_metadata.atime() >= two_metadata.atime() {

Original == check should be enough, but we can be extra cautious and expect the new atime to be strictly greater than old, as we are sleeping in between.

[YJDoc2]

Suggested change

	"update access time for file {:?}, expected update",
	"expected old access time to be less than new access time after update for file {:?}, found old : {old}, new {new} ",

[lengrongfu]

I'm not sure if this is correct, because runc implementation is similar. but runc can't success running the test.
https://github.com/opencontainers/runc/blob/18c313be729dd02b17934af41e32116a28b4b3bf/libcontainer/specconv/spec_linux.go#L125

[YJDoc2]

I'm not sure if this is correct, because runc implementation is similar. but runc can't success running the test.
https://github.com/opencontainers/runc/blob/18c313be729dd02b17934af41e32116a28b4b3bf/libcontainer/specconv/spec_linux.go#L125

Yes, you are right. From what I checked it seems that the mount options are dependent on the specific flag being ORed or not ORed . for if for time we are ORing the flag SET_ATIME (suppose) , then for noatime we simply do not OR that flag in the argument. I believe our setup regarding this particular part is similar to runc, so the original code should be correct.

What do you think?

[YJDoc2]

@lengrongfu ping!

[lengrongfu]

I'm thinking, maybe mount doesn't have a ratime option, because setting "rnoatime" => Ok(MountAttrOption::MountAttrNoatime(false, MOUNT_ATTR_NOATIME)) is to use non-updated access time, and "ratime" => Ok(MountAttrOption::MountAttrNoatime(true, MOUNT_ATTR_NOATIME)) is to use updated access time, but for the file system, the access time can be updated by default, so does the mount option ratime not exist?

[utam0k]

@lengrongfu Yes, you're right. ratime doesn't exist in mount(2). This is an original option in OCI Runtime Spec.
https://github.com/opencontainers/runtime-spec/blob/5d5d92197d1491500490c44129156fab2154276e/config.md#linux-mount-options

[lengrongfu]

@utam0k So, we should don't need this test PR?

[utam0k]

@utam0k So, we should don't need this test PR?

No, we need this test.

[YJDoc2]

Hey @lengrongfu , apologies , we haven't been able to provide attention to this. If you're still willing to work on this, I'll go over this again and see what can be done.