Add test for cgroups_relative_memory

[omprakaash]

Adds an integration test for cgroups_relative_memory. The only difference from the non-relative test seems to be the cgroup_path of the specified spec while testing.

[codecov-commenter]

Codecov Report

Merging #2686 (44e7b59) into main (04f8f2d) will decrease coverage by 0.37%. Report is 64 commits behind head on main. The diff coverage is 0.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2686      +/-   ##
==========================================
- Coverage   65.50%   65.13%   -0.37%     
==========================================
  Files         133      133              
  Lines       16916    17012      +96     
==========================================
  Hits        11081    11081              
- Misses       5835     5931      +96     

[lengrongfu]

LGTM

[YJDoc2]

Hey, as @omprakaash had mentioned in discord we seem to be skipping a check for memory cgroups validation, as done here in runtime-tools. @tsturzl do you have any idea, why we might be intentionally skipping this, or we might have just missed it originally?

If so, @omprakaash may I ask you to add that in both this and the other test? (You can do it in separate PR, we can merge this one before it)

[tsturzl]

@YJDoc2 Hmm, if you compare any of the integration tests to runtime-tools it seems like we're missing a similar step a few cgroups integration test I think I might have assumed there was something in check_container_created that was checking the container against the spec somehow. It's been 3 years, so I can't recall my thinking from back then.

A simple solution would be to just read the respective cgroups files and comparing them. It looks like cpu, memory, and network are all missing these checks.

There is a lot of opportunity here to create a read/write abstraction for cgroups files. Right now we just have a constant for each file name and use a write_cgroup_file utility function to write to a given path. If reading (or writing) cgroups file is going to become more useful outside the context of libcgroups crate then maybe we want a generalized abstraction to handle read/write? This would clean up the implementation and unit tests, as well as provide obvious utility to these integration tests in checking cgroups values without having to implement a lot of boilerplate. It's something I thought about introducing to make it easier to more easily swap out the kernel IO API being used which is something I've experimented with in the past. I don't know if that effort is too large to block fixing the issues with the current cgroups integration tests, but it's something I'd be interested in working on if no one else is immediately interested.

[omprakaash]

I could just implement the additional checks for this test with current existing methods in this PR. Can the new abstractions be part of a separate PR later on ?

[tsturzl]

@omprakaash that's probably a better immediate solution. If we can just implement something similar to what runtime tools is doing.

[YJDoc2]

I think I have some changes to request in this, I'll try to get to a full review soon.

[omprakaash]

Thank you for the review ! Will make the required changes.

[YJDoc2]

@omprakaash ping! There are still some review changes pending, so pinging you

[omprakaash]

Sorry for the delay, finally got a chance to work on this again.

[utam0k]

@omprakaash There seems to be a conflict. Please need to rebase.