Support feature subcommand #2395

DarrellTang · 2023-09-28T05:49:54Z

No description provided.

utam0k

May I ask you to add the unit tests?

YJDoc2

Hey, so I have added some comments. feel free to ask any questions.
As for actually populating the data into the structures, we can take a look at how runc is doing it.

crates/liboci-cli/src/features.rs

YJDoc2 · 2023-10-12T15:19:06Z

@DarrellTang let me know if any issues!

DarrellTang · 2023-10-28T04:29:21Z

@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 · 2023-10-28T06:20:45Z

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 · 2023-10-31T01:46:07Z

@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 · 2023-11-06T05:34:49Z

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 👍

utam0k · 2023-11-10T12:04:42Z

@DarrellTang May I ask you to check the CI?

DarrellTang · 2023-11-13T00:38:17Z

@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 · 2023-11-14T12:16:41Z

@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 · 2023-12-18T05:23:54Z

@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 · 2023-12-18T06:46:48Z

@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 · 2023-12-18T09:41:45Z

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

DarrellTang · 2024-01-14T19:12:45Z

@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 · 2024-01-19T12:44:50Z

@DarrellTang It seems that it is still missing

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

adrianalin · 2024-01-20T10:34:02Z

A rebase is also needed here.

YJDoc2 · 2024-01-21T09:08:03Z

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.

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

utam0k · 2024-02-04T12:48:15Z

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

codecov-commenter · 2024-02-04T12:48:16Z

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

AkihiroSuda · 2024-02-10T18:47:03Z

crates/youki/src/commands/features.rs

+            );
+            annotations_map.insert(
+                ANNOTATION_RUNC_COMMIT.to_string(),
+                String::from("v1.1.9-0-gccaecfc"),


This is specific to runc.

youki is not intended to use this annotation.

Should there be a similar annotation for youki, i.e. ANNOTATION_YOUKI_COMMIT ?

AkihiroSuda · 2024-02-10T18:47:22Z

crates/youki/src/commands/features.rs

+            let mut annotations_map = HashMap::new();
+            annotations_map.insert(
+                ANNOTATION_LIBSECCOMP_VERSION.to_string(),
+                String::from("2.5.3"),


This should be fetched from libseccomp (if youki uses libseccomp)

it definitely does. what's the best way for me to grab this version number? I apologize, I'm pretty new to rust.

AkihiroSuda · 2024-02-10T18:47:54Z

crates/youki/src/commands/features.rs

+
+    let features = HardFeatures {
+        oci_version_min: Some(String::from("1.0.0")),
+        oci_version_max: Some(String::from("1.0.2-dev")),


Why not 1.1.0 ?

I hardcoded it when i started so I assume it's progressed since then. I don't have any objection to using 1.1.0 but it seems like runc pulls this from the https://github.com/opencontainers/runtime-spec/blob/main/specs-go/version.go . Should I just hardcode for 1.2.0 (which is the latest) or is there a rust runtime-spec equivalent?

Thank you for reviewing this!

utam0k · 2024-02-25T12:22:16Z

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

DarrellTang · 2024-02-28T01:50:49Z

@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! 🙏

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

YJDoc2 · 2024-03-06T06:03:05Z

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 · 2024-06-07T05:44:59Z

@DarrellTang Friendly reminder 🙏 👀
#2395 (comment)

utam0k · 2024-06-28T12:18:57Z

@musaprg Are you interested in taking over this PR?

DarrellTang · 2024-06-28T14:56:03Z

@DarrellTang Friendly reminder 🙏 👀

#2395 (comment)

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 · 2024-06-29T14:21:38Z

@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 · 2024-07-11T11:28:31Z

Dup #2837

YJDoc2 assigned YJDoc2 and unassigned YJDoc2 Sep 28, 2023

YJDoc2 self-requested a review September 28, 2023 06:49

utam0k added the kind/feature label Sep 28, 2023

utam0k requested changes Sep 28, 2023

View reviewed changes

YJDoc2 reviewed Oct 3, 2023

View reviewed changes

crates/liboci-cli/src/features.rs Outdated Show resolved Hide resolved

crates/liboci-cli/src/features.rs Outdated Show resolved Hide resolved

DarrellTang closed this Oct 30, 2023

DarrellTang force-pushed the support-feature-subcommand branch from f9bd988 to 646c103 Compare October 30, 2023 04:36

DarrellTang reopened this Oct 31, 2023

DarrellTang force-pushed the support-feature-subcommand branch 2 times, most recently from df480bf to 5e74538 Compare November 8, 2023 03:17

DarrellTang force-pushed the support-feature-subcommand branch from b152a67 to 44fcf79 Compare November 13, 2023 03:22

DarrellTang force-pushed the support-feature-subcommand branch from 070a86a to c2d8d53 Compare January 14, 2024 19:09

DarrellTang force-pushed the support-feature-subcommand branch 2 times, most recently from f03c7ee to fc12184 Compare January 28, 2024 00:04

DarrellTang added 14 commits January 27, 2024 16:04

try to create new struct for hardcoding

18a85fb

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

change struct name to resolve conflict

e9a81a5

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

fix annotation references

ea18e74

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

set as Strings

1a48f3b

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

use serde

cfe4069

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

pretty print

07ac974

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

clean up names to match runc features output

df9c555

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

rearrange structs and constants

763fa0e

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

fix lint issues

800521d

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

try to source caps dynamically

d973c8c

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

try to source namespaces dynamically

cf4145b

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

fix query_caps

707141a

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

fix match statements

31997ca

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

fix linting issues

fc12184

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

AkihiroSuda reviewed Feb 10, 2024

View reviewed changes

fix extra line for linting

b069ca6

Signed-off-by: Darrell Tang <darrelltang@gmail.com>

musaprg mentioned this pull request Jun 29, 2024

Support feature subcommand #2837

Open

2 tasks

utam0k closed this Jul 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support feature subcommand #2395

Support feature subcommand #2395

DarrellTang commented Sep 28, 2023

utam0k left a comment

YJDoc2 left a comment •

edited

Loading

YJDoc2 commented Oct 12, 2023

DarrellTang commented Oct 28, 2023

YJDoc2 commented Oct 28, 2023

DarrellTang commented Oct 31, 2023

YJDoc2 commented Nov 6, 2023

utam0k commented Nov 10, 2023

DarrellTang commented Nov 13, 2023 •

edited

Loading

utam0k commented Nov 14, 2023

YJDoc2 commented Dec 18, 2023

DarrellTang commented Dec 18, 2023

YJDoc2 commented Dec 18, 2023

DarrellTang commented Jan 14, 2024

utam0k commented Jan 19, 2024

adrianalin commented Jan 20, 2024

YJDoc2 commented Jan 21, 2024

utam0k commented Feb 4, 2024

codecov-commenter commented Feb 4, 2024

AkihiroSuda Feb 10, 2024

DarrellTang Feb 28, 2024

AkihiroSuda Feb 10, 2024

DarrellTang Feb 28, 2024

AkihiroSuda Feb 10, 2024

DarrellTang Feb 28, 2024

utam0k commented Feb 25, 2024

DarrellTang commented Feb 28, 2024

YJDoc2 commented Mar 6, 2024

utam0k commented Jun 7, 2024

utam0k commented Jun 28, 2024

DarrellTang commented Jun 28, 2024

musaprg commented Jun 29, 2024

utam0k commented Jul 11, 2024

Support feature subcommand #2395

Support feature subcommand #2395

Conversation

DarrellTang commented Sep 28, 2023

utam0k left a comment

Choose a reason for hiding this comment

YJDoc2 left a comment • edited Loading

Choose a reason for hiding this comment

YJDoc2 commented Oct 12, 2023

DarrellTang commented Oct 28, 2023

YJDoc2 commented Oct 28, 2023

DarrellTang commented Oct 31, 2023

YJDoc2 commented Nov 6, 2023

utam0k commented Nov 10, 2023

DarrellTang commented Nov 13, 2023 • edited Loading

utam0k commented Nov 14, 2023

YJDoc2 commented Dec 18, 2023

DarrellTang commented Dec 18, 2023

YJDoc2 commented Dec 18, 2023

DarrellTang commented Jan 14, 2024

utam0k commented Jan 19, 2024

adrianalin commented Jan 20, 2024

YJDoc2 commented Jan 21, 2024

utam0k commented Feb 4, 2024

codecov-commenter commented Feb 4, 2024

Codecov Report

AkihiroSuda Feb 10, 2024

Choose a reason for hiding this comment

DarrellTang Feb 28, 2024

Choose a reason for hiding this comment

AkihiroSuda Feb 10, 2024

Choose a reason for hiding this comment

DarrellTang Feb 28, 2024

Choose a reason for hiding this comment

AkihiroSuda Feb 10, 2024

Choose a reason for hiding this comment

DarrellTang Feb 28, 2024

Choose a reason for hiding this comment

utam0k commented Feb 25, 2024

DarrellTang commented Feb 28, 2024

YJDoc2 commented Mar 6, 2024

utam0k commented Jun 7, 2024

utam0k commented Jun 28, 2024

DarrellTang commented Jun 28, 2024

musaprg commented Jun 29, 2024

utam0k commented Jul 11, 2024

YJDoc2 left a comment •

edited

Loading

DarrellTang commented Nov 13, 2023 •

edited

Loading