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

service: print more error message #1388

Merged
merged 1 commit into from
Aug 4, 2023

Conversation

WeiZhang555
Copy link
Contributor

Some error messages were swallowed which makes user confused, for example, for RAFSv6, we need to set blobcache config in localfs.json (following docs tutorial), before modification, the error message indicates nothing:

ERROR [src/bin/nydusd/main.rs:525] Failed in starting daemon: Invalid
argument (os error 22)

After this modification, we get clearer error message:

ERROR [/src/fusedev.rs:595] service mount error: RAFS failed to handle
request, Configure("Rafs v6 must have local blobcache configured")

Relevant Issue (if applicable)

Details

Please describe the details of PullRequest.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation Update (if none of the other choices apply)

Checklist

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@WeiZhang555 WeiZhang555 requested a review from a team as a code owner August 3, 2023 06:34
@WeiZhang555 WeiZhang555 requested review from bergwolf, imeoer and jiangliu and removed request for a team August 3, 2023 06:34
@anolis-bot
Copy link
Collaborator

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

@anolis-bot
Copy link
Collaborator

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

Test CaseTest Result
build rust golang image❌ FAIL

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

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Merging #1388 (880adbb) into master (43c737d) will decrease coverage by 0.26%.
The diff coverage is 0.00%.

❗ Current head 880adbb differs from pull request most recent head b09acfa. Consider uploading reports for the commit b09acfa to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1388      +/-   ##
==========================================
- Coverage   46.31%   46.05%   -0.26%     
==========================================
  Files         122      122              
  Lines       38071    38077       +6     
  Branches    38071    38077       +6     
==========================================
- Hits        17632    17536      -96     
- Misses      19513    19621     +108     
+ Partials      926      920       -6     
Files Changed Coverage Δ
service/src/fusedev.rs 0.00% <0.00%> (ø)

... and 8 files with indirect coverage changes

@WeiZhang555
Copy link
Contributor Author

More details:

Following tutorial: https://github.com/dragonflyoss/image-service/blob/master/docs/tutorial.md#build-nydus-image-from-directory-source

When I tried to start a nydusd with this localfs.json:

{
  "device": {
    "backend": {
      "type": "localfs",
      "config": {
        "dir": "/<YOUR-WORK-PATH>/nydus-image/blobs"
      }
    }
  },
  "mode": "direct"
}

The nydusd errors out with this message:

image

It saids "Invalid arguments" which confused me a lot, after some debugging, I found that RAFSv6 needs to set blob cache config, e.g.

{
  "device": {
    "backend": {
      "type": "localfs",
      "config": {
        "dir": "/<YOUR-WORK-PATH>/nydus-image/blobs"
      }
    },
    "cache": {
      "type": "blobcache",
      "config": {
        "work_dir": "/var/lib/nydus/cache"
      }
    }
  },
  "mode": "direct"
}

A better error message should tell us the configuration is error, so I just add a little bit new code to reveal the inner error.

Final result:

image

The docs should be updated too, I'll append another commit.

@anolis-bot
Copy link
Collaborator

@WeiZhang555 , 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/88755

@anolis-bot
Copy link
Collaborator

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

Test CaseTest Result
build rust golang image✅ SUCCESS
compile nydusd✅ SUCCESS
compile ctr remote✅ SUCCESS
compile nydus snapshotter✅ SUCCESS
run container with rafs✅ SUCCESS
run container with zran✅ SUCCESS
run container with rafs and compile linux✅ SUCCESS

Congratulations, your test job passed!

Some error messages were swallowed which makes user confused, for
example, for RAFSv6, we need to set blobcache config in `localfs.json`
(following docs tutorial), before modification, the error message
indicates nothing:

```
ERROR [src/bin/nydusd/main.rs:525] Failed in starting daemon: Invalid
argument (os error 22)
```

After this modification, we get clearer error message:

```
ERROR [/src/fusedev.rs:595] service mount error: RAFS failed to handle
request, Configure("Rafs v6 must have local blobcache configured")
```

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

@WeiZhang555 , 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/89086

@anolis-bot
Copy link
Collaborator

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

Test CaseTest Result
build rust golang image✅ SUCCESS
compile nydusd✅ SUCCESS
compile ctr remote✅ SUCCESS
compile nydus snapshotter✅ SUCCESS
run container with rafs✅ SUCCESS
run container with zran✅ SUCCESS
run container with rafs and compile linux✅ SUCCESS

Congratulations, your test job passed!

Copy link
Member

@adamqqqplay adamqqqplay left a comment

Choose a reason for hiding this comment

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

Very detailed analysis, thanks for your work!

@jiangliu jiangliu merged commit f419349 into dragonflyoss:master Aug 4, 2023
21 of 22 checks passed
@WeiZhang555 WeiZhang555 deleted the improve-error-message branch August 7, 2023 02:10
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.

5 participants