Skip to content
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

nydus-image: refactor subcommand "inspect" #675

Merged
merged 2 commits into from
Sep 20, 2022

Conversation

YushuoEdge
Copy link
Contributor

The code for "nydus-image inspect" are refactored using standard API
in RAFS.

Signed-off-by: YushuoEdge [email protected]

@anolis-bot
Copy link
Collaborator

@YushuoEdge , a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/18269

@anolis-bot
Copy link
Collaborator

@YushuoEdge , The CI test is completed, please check result:

Test CaseTest Result
merge-target-branch✅ SUCCESS
build-docker-image✅ SUCCESS
compile-nydus✅ SUCCESS
compile-ctr-remote✅ SUCCESS
compile-nydus-snapshotter✅ SUCCESS
start-nydus-snapshotter-config-containerd✅ SUCCESS
run-container-with-nydus-image✅ SUCCESS

Congratulations, your test job passed!

@liubogithub
Copy link
Collaborator

@changweige can you please take a look?

@changweige
Copy link
Contributor

@changweige can you please take a look?

Sure. will take a glance

@changweige
Copy link
Contributor

I am experiencing this refactored tool but got the below error, which I think chunk info should be correctly shown about its blob id and chunk index in one blob.

Shall we continue reviewing the PR until they are fixed?

For symlink file:

Inspecting Rafs :> stat run
[2022-08-17 09:49:51.162030 +08:00] ERROR [error/src/error.rs:21] Error:
	"invalid chunk info"
	at rafs/src/metadata/direct_v6.rs:956
	note: enable `RUST_BACKTRACE=1` env to display a backtrace
Failed in executing command, No such file or directory (os error 2)

For regular file:

Inspecting Rafs :> stat btmp
[2022-08-17 09:52:57.778269 +08:00] ERROR [/home/gechangwei/.cargo/registry/src/mirrors.sjtug.sjtu.edu.cn-4f7dbcce21e258a2/log-panics-2.1.0/src/lib.rs:130] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: NotFound, error: "No such file or directory (os error 2): rafs/src/metadata/direct_v6.rs:753" }': rafs/src/metadata/direct_v6.rs:1141
   0: backtrace::capture::Backtrace::create
   1: backtrace::capture::Backtrace::new

@YushuoEdge
Copy link
Contributor Author

changweige

Sorry for that. I did not find the problem when I did test before. Maybe something went wrong when I rebase to master branch, please allow me to check again and revise.

@anolis-bot
Copy link
Collaborator

@YushuoEdge , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/19319

@anolis-bot
Copy link
Collaborator

@YushuoEdge , The CI test is completed, please check result:

Test CaseTest Result
merge-target-branch✅ SUCCESS
build-docker-image✅ SUCCESS
compile-nydus✅ SUCCESS
compile-ctr-remote✅ SUCCESS
compile-nydus-snapshotter✅ SUCCESS
start-nydus-snapshotter-config-containerd✅ SUCCESS
run-container-with-nydus-image✅ SUCCESS

Congratulations, your test job passed!

@changweige
Copy link
Contributor

Please rebase your PR to get cargo deny passed

@anolis-bot
Copy link
Collaborator

@YushuoEdge , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/19487

@anolis-bot
Copy link
Collaborator

@YushuoEdge , The CI test is completed, please check result:

Test CaseTest Result
merge-target-branch✅ SUCCESS
build-docker-image✅ SUCCESS
compile-nydus✅ SUCCESS
compile-ctr-remote✅ SUCCESS
compile-nydus-snapshotter✅ SUCCESS
start-nydus-snapshotter-config-containerd✅ SUCCESS
run-container-with-nydus-image✅ SUCCESS

Congratulations, your test job passed!

Copy link
Member

@bergwolf bergwolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup! Thanks! Is it possible to add some test cases in smoke? There are pre-built bootstrap files under tests/texture/bootstrap/ which can be used to test inspect as well.

@@ -882,7 +492,6 @@ impl Executor {
prefetch: Show prefetch table
chunk OFFSET: List basic info of a single chunk together with a list of files that share it
icheck INODE: Show path of the inode and basic information
index INDEX: Show information about a file by its index
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed index is unneeded and it's fine to just drop it.

@@ -896,7 +505,7 @@ impl Prompt {
print!("Inspecting Rafs :> ");
std::io::stdout().flush().unwrap();

let mut input = String::new();
let mut input: String = String::new();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks unnecessary.

@imeoer
Copy link
Collaborator

imeoer commented Sep 16, 2022

Updates: @YushuoEdge is working on smoke testing and fixing. :)

The code for "nydus-image inspect" are refactored using standard API
in RAFS.

Signed-off-by: YushuoEdge <[email protected]>
@anolis-bot
Copy link
Collaborator

@YushuoEdge , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/21637

@anolis-bot
Copy link
Collaborator

@YushuoEdge , The CI test is completed, please check result:

Test CaseTest Result
merge-target-branch✅ SUCCESS
build-docker-image✅ SUCCESS
compile-nydus❌ FAIL

Sorry, your test job failed. Please get the details in the link.

@anolis-bot
Copy link
Collaborator

@YushuoEdge , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/21638

@YushuoEdge
Copy link
Contributor Author

YushuoEdge commented Sep 18, 2022

update:
The request_mode output of cmd stats, prefetch, blobs are used in smoke test.
More tests result can refer to: https://docs.google.com/document/d/115r90YmZmRPKxEz6WEfGHD6F6eylv4lQDe-7xd_pTUI/edit?usp=sharing

@anolis-bot
Copy link
Collaborator

@YushuoEdge , The CI test is completed, please check result:

Test CaseTest Result
merge-target-branch✅ SUCCESS
build-docker-image✅ SUCCESS
compile-nydus✅ SUCCESS
compile-ctr-remote✅ SUCCESS
compile-nydus-snapshotter✅ SUCCESS
start-nydus-snapshotter-config-containerd✅ SUCCESS
run-container-with-nydus-image❌ FAIL

Sorry, your test job failed. Please get the details in the link.

We use the output result of request_mode for verification.

In ./tests/texture/image-inspect/*.result, there stores the output
result of executing cmd
`nydus-image inspect -B ./tests/texture/bootstrap/image_v2.boot -R *`
with `nydus-image` that has not been refactored.

Then we load the result and compare them with result using the
refactored `nydus-image` to do the smoke test.

Signed-off-by: YushuoEdge <[email protected]>
@anolis-bot
Copy link
Collaborator

@YushuoEdge , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/21641

@anolis-bot
Copy link
Collaborator

@YushuoEdge , The CI test is completed, please check result:

Test CaseTest Result
merge-target-branch✅ SUCCESS
build-docker-image✅ SUCCESS
compile-nydus✅ SUCCESS
compile-ctr-remote✅ SUCCESS
compile-nydus-snapshotter✅ SUCCESS
start-nydus-snapshotter-config-containerd✅ SUCCESS
run-container-with-nydus-image❌ FAIL

Sorry, your test job failed. Please get the details in the link.

@imeoer
Copy link
Collaborator

imeoer commented Sep 19, 2022

update: The request_mode output of cmd stats, prefetch, blobs are used in smoke test. More tests result can refer to: https://docs.google.com/document/d/115r90YmZmRPKxEz6WEfGHD6F6eylv4lQDe-7xd_pTUI/edit?usp=sharing

Thanks for adding the test case in smoke testing, it is difficult for us to cover all the tests of inspect command, a few basic checks are enough, what do you think? cc @changweige @bergwolf

Copy link
Member

@bergwolf bergwolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@bergwolf bergwolf merged commit 8aaeca9 into dragonflyoss:master Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants