Support feature subcommand

[DarrellTang]

[YJDoc2]

@DarrellTang let me know if any issues!

[DarrellTang]

@DarrellTang let me know if any issues!

Sorry for the delay, had some life events pop up. I've made some changes but I still can't get it to compile. This is the first error:

error[E0277]: the trait bound `Linux: Clone` is not satisfied
   --> crates/liboci-cli/src/features.rs:25:19
    |
25  |     linux: Option<Linux>,
    |                   ^^^^^ the trait `Clone` is not implemented for `Linux`
    |
note: required by a bound in `ArgMatches::remove_one`
   --> /home/dtang/.cargo/registry/src/index.crates.io-6f17d22bba15001f/clap_builder-4.3.0/src/parser/matches/arg_matches.rs:404:32
    |
404 |     pub fn remove_one<T: Any + Clone + Send + Sync + 'static>(&mut self, id: &str) -> Option<T> {
    |                                ^^^^^ required by this bound in `ArgMatches::remove_one`

Not sure what to do with that error? Any suggestions?

[YJDoc2]

Hey @DarrellTang as I have mentioned in the comment here, the structure you are using is for commandline arg parsing. It should not be used to store the data. You should keep that structure as-is and create a separate one to store/move around the features data.

[DarrellTang]

@YJDoc2 Not sure how I closed this PR but I made some more progress. Sorry about the confusion, it finally clicked what you meant by not using that struct and creating a new one.

Some questions now:

• I've got it printing out the json for youki features like the runc features but I'm not sure if the organization of the code is best. I've got the struct in the same file as the features invocation. Suggestions on how to structure this?
• Is there an example unit test I could copy off of for @utam0k's request?

[YJDoc2]

Hey @DarrellTang Apologies for the delay. This looks like a step in right direction. One change I'd strongly suggest is to define constants at the top and define structures before they are used rather than after. Also there seems to be some lint warnings, so you can try running just lint (you'll need to install just ) or you can see the justfile and run commands manually.

Now that you have got the structure ready, we can try to set the values dynamically instead of having them hard-coded. For example, you can use the caps library which is already a dependency of youki, to get the list of supported capabilities, and use that list instead of hard-coding them. Similarly you can try for supported namespaces as well, probably using procfs (not sure about this, might need something else) and so on.

Let me know if you have any issues :+1:

[utam0k]

@DarrellTang May I ask you to check the CI?

[DarrellTang]

@DarrellTang May I ask you to check the CI?

@utam0k I see there are CI runs that require a maintainer to approve but not sure what to do with those? Is there something I should be doing for these? Sorry if these are silly questions. Still learning my way around.

[utam0k]

@utam0k I see there are CI runs that require a maintainer to approve but not sure what to do with those? Is there something I should be doing for these? Sorry if these are silly questions. Still learning my way around.

@DarrellTang I have approved it. You should commit with sign-off at least, please refer to this page: https://github.com/containers/youki/pull/2395/checks?check_run_id=18610534142

[YJDoc2]

@DarrellTang Hello! Are you still working on this? It is fine if you got busy with something else, just let us know so we can close this / re-assign to someone else accordingly. Thanks :)

[DarrellTang]

@DarrellTang Hello! Are you still working on this? It is fine if you got busy with something else, just let us know so we can close this / re-assign to someone else accordingly. Thanks :)

Yes, I'm still planning on working on this. I have to redo my dev setup and got busy but would like to stay assigned unless there's urgency to get it done! Thank you!

[YJDoc2]

Hey @DarrellTang no problem. I'll keep this as it is, and feel free to ask questions or help if needed :+1: In case this is needed urgently, I'll message here and we can discuss about re-assigning then. Thanks!

[DarrellTang]

@utam0k Just figured out the sign off. sorry about the delay.

@YJDoc2 I managed to get capabilities and namespaces pulling dynamically but i'm stuck on what other sections i can do that for. any suggestions for some easy, low hanging fruit?

I looked at cgroups, mount_options, and hooks and couldn't figure out where to pull those from the operating system

[utam0k]

@DarrellTang It seems that it is still missing

@utam0k Just figured out the sign off. sorry about the delay.

[adrianalin]

A rebase is also needed here.

[YJDoc2]

I looked at cgroups, mount_options, and hooks and couldn't figure out where to pull those from the operating system

Hey @DarrellTang , apologies for the delay in response. I think you can take a look at runc's implementation . They also seem to have hardcoded several values such as mount options, we should just validate which of them are applicable for youki.

[utam0k]

@DarrellTang Hi 👋 May I ask you to check the lint errors? You can check them using just lint

[codecov-commenter]

Codecov Report

Merging #2395 (fc12184) into main (04f8f2d) will decrease coverage by 0.72%. Report is 18 commits behind head on main. The diff coverage is 0.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2395      +/-   ##
==========================================
- Coverage   65.50%   64.78%   -0.72%     
==========================================
  Files         133      133              
  Lines       16916    17105     +189     
==========================================
+ Hits        11081    11082       +1     
- Misses       5835     6023     +188     

[utam0k]

@DarrellTang Do you have any trouble? I can help you ;)

[DarrellTang]

@DarrellTang Do you have any trouble? I can help you ;)

Thank you. I know it has taken a long time on this. It seems like my commits are signed now. I also just removed an extra blank line that was causing the linting to fail.

I am still puzzling over what else I can pull programatically. Not sure of a direction so if you have suggestions for what I should look at next, I'd be very happy to take your guidance! :pray:

[YJDoc2]

I am still puzzling over what else I can pull programatically. Not sure of a direction so if you have suggestions for what I should look at next, I'd be very happy to take your guidance!

Hey, so what you can do is to check what things are we hardcoding here, and runc is fetching dynamically, and try to do the same. There are some things which even runc hard-codes, so maybe we can deal with them separately.

[utam0k]

@DarrellTang Friendly reminder 🙏 👀
https://github.com/containers/youki/pull/2395#issuecomment-1980144105

[utam0k]

@musaprg Are you interested in taking over this PR?

[DarrellTang]

@DarrellTang Friendly reminder 🙏 👀

https://github.com/containers/youki/pull/2395#issuecomment-1980144105

I'm sorry for the delay, please feel free to reassign this. My schedule has changed and I haven't had time to continue working on this.

[musaprg]

@utam0k Hi! I'd love to take over the PR, but I need some time to catch things up. I'll submit another PR with cherry-picking the existing commits after that.

[utam0k]

Dup https://github.com/containers/youki/pull/2837