-
Notifications
You must be signed in to change notification settings - Fork 154
cli: Add container inspect
#1832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new bootc container inspect command that outputs container metadata as JSON. Initially, it provides the kernel arguments found in the image. The implementation is clean and follows existing patterns in the codebase. A new test is added to verify the command's output. My main feedback is to make the new test more strict to ensure the output is exactly as expected, improving test robustness.
| assert ($inspect.kargs | any { |k| $k == "kargsd-test=1" }) "expected kargsd-test=1" | ||
| assert ($inspect.kargs | any { |k| $k == "kargsd-othertest=2" }) "expected kargsd-othertest=2" | ||
| assert ($inspect.kargs | any { |k| $k == "testing-kargsd=3" }) "expected testing-kargsd=3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current assertions check for the presence of the expected kernel arguments, but they don't verify the exact list of arguments or their order. This could allow unexpected changes (like additional arguments or different ordering) to go unnoticed. To make the test more robust, I suggest asserting that the kargs array is exactly equal to the expected array.
let expected_kargs = ["kargsd-test=1", "kargsd-othertest=2", "testing-kargsd=3"]
assert ($inspect.kargs == $expected_kargs) "kargs do not match expected values"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It very specifically doesn't check it that way because there's other kargs that get pulled in as part of other pieces of the test suite. Under hack/test-kargs we have:
jeckersb@toolbx bootc> ls hack/test-kargs/ 12/05/2025 04:37:51 PM
╭───┬───────────────────────────────┬──────┬──────┬────────────╮
│ # │ name │ type │ size │ modified │
├───┼───────────────────────────────┼──────┼──────┼────────────┤
│ 0 │ hack/test-kargs/10-test.toml │ file │ 48 B │ a year ago │
│ 1 │ hack/test-kargs/20-test2.toml │ file │ 29 B │ a year ago │
╰───┴───────────────────────────────┴──────┴──────┴────────────╯
jeckersb@toolbx bootc> cat hack/test-kargs/* 12/05/2025 04:38:00 PM
kargs = ["kargsd-test=1", "kargsd-othertest=2"]
kargs = ["testing-kargsd=3"]
But in the final integration image we get other things:
jeckersb@toolbx bootc> podman run --rm -it localhost/bootc-integration bootc container inspect 12/05/2025 04:39:02 PM
{
"kargs": [
"kargsd-test=1",
"kargsd-othertest=2",
"console=ttyS0,115200n8",
"testing-kargsd=3",
"console=hvc0"
]
}
|
One outstanding question is that as-is this only looks at kargs.d, but for the container inspection case do we also want to consider install-config embedded in the container as well? I lean slightly towards no (which is why I omitted it initially) but could pretty easily be persuaded otherwise. |
Yes it's a complicated question. I think there's very few use cases for the install config kargs nowadays and we should generally treat them as legacy. So we can omit them here. |
|
Filed #1839 to track CI failure |
Prints JSON of container metadata/attributes of interest. For now this just renders out the kargs embedded in the container under the kargs.d drop-in. Future ideas for enhancements would be to include kernel version and whether or not the image uses a UKI. Closes: bootc-dev#1827 Signed-off-by: John Eckersberg <jeckersb@redhat.com>
Signed-off-by: John Eckersberg <jeckersb@redhat.com>
Signed-off-by: John Eckersberg <jeckersb@redhat.com>
Prints JSON of container metadata/attributes of interest.
For now this just renders out the kargs embedded in the container
under the kargs.d drop-in. Future ideas for enhancements would be to
include kernel version and whether or not the image uses a UKI.
Closes: #1827
Signed-off-by: John Eckersberg jeckersb@redhat.com