Skip to content

mgr/dashboard: CephFS statfs REST API#52218

Merged
nizamial09 merged 1 commit intoceph:mainfrom
YiteGu:cephfs-get-statfs
Jul 25, 2023
Merged

mgr/dashboard: CephFS statfs REST API#52218
nizamial09 merged 1 commit intoceph:mainfrom
YiteGu:cephfs-get-statfs

Conversation

@YiteGu
Copy link
Member

@YiteGu YiteGu commented Jun 28, 2023

Introduce statfs for the CephFS REST API controller, we can get statfs of the specified path by it, it returns the used bytes and the number of files used.

Fixes: https://tracker.ceph.com/issues/61883

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@YiteGu YiteGu requested a review from a team as a code owner June 28, 2023 03:49
@YiteGu YiteGu requested review from Pegonzal and cloudbehl and removed request for a team June 28, 2023 03:49
@YiteGu
Copy link
Member Author

YiteGu commented Jun 28, 2023

For example:

curl -X GET "0.0.0.0:8080/api/cephfs/1/statfs?path=/" -H  "Accept: application/vnd.ceph.api.v1.0+json"
{"used_bytes": 0, "used_files": 1}

@YiteGu
Copy link
Member Author

YiteGu commented Jun 30, 2023

Hi @cloudbehl have a code review

@YiteGu
Copy link
Member Author

YiteGu commented Jul 3, 2023

@dparmar18 help me code review if you have time

@dparmar18
Copy link
Contributor

Would be good to have a feature tracker for this

@dparmar18
Copy link
Contributor

We have ls_dir in src/pybind/mgr/dashboard/services/cephfs.py that looks similar to this because the doctext in that method says "List directories of specified path with additional information."

Did you check if the "additional information" mentioned contains bytes/files usage stats?

@dparmar18
Copy link
Contributor

If we decide to move with this patch then we need to verify that this works as intended, I'm unsure how dashboard tests works but definitely we need a test case here.

@YiteGu
Copy link
Member Author

YiteGu commented Jul 3, 2023

Would be good to have a feature tracker for this

Created: https://tracker.ceph.com/issues/61883

@YiteGu
Copy link
Member Author

YiteGu commented Jul 3, 2023

We have ls_dir in src/pybind/mgr/dashboard/services/cephfs.py that looks similar to this because the doctext in that method says "List directories of specified path with additional information."

Did you check if the "additional information" mentioned contains bytes/files usage stats?
my test fs:

# mount | grep cephfs
ceph-fuse on /root/cephfs_mount type fuse.ceph-fuse (rw,nosuid,nodev,relatime,user_id=0,group_id=0,allow_other)
# tree /root/cephfs_mount
/root/cephfs_mount
├── dir1
├── file1
└── file2

cephfs_mount]# ll -h
总用量 4.1M
drwxr-xr-x. 2 root root    0 7月   3 18:56 dir1
-rw-r--r--. 1 root root    0 7月   3 18:56 file1
-rw-r--r--. 1 root root 4.0M 7月   3 19:04 file2

ls_dir

# curl -X GET "0.0.0.0:8080/api/cephfs/1/ls_dir?path=/" -H  "Accept: application/vnd.ceph.api.v1.0+json" 
[{"name": "dir1", "path": "/dir1", "parent": "/", "snapshots": [], "quotas": {"max_bytes": 0, "max_files": 0}}]

statfs

curl -X GET "0.0.0.0:8080/api/cephfs/1/statfs?path=/" -H  "Accept: application/vnd.ceph.api.v1.0+json" 
{"used_bytes": 4194304, "used_files": 4}

@YiteGu
Copy link
Member Author

YiteGu commented Jul 3, 2023

If we decide to move with this patch then we need to verify that this works as intended, I'm unsure how dashboard tests works but definitely we need a test case here.

I have not found a test case about ls_dir, get quota and set quota, should we write a test case for these API in another PR?

@dparmar18
Copy link
Contributor

If we decide to move with this patch then we need to verify that this works as intended, I'm unsure how dashboard tests works but definitely we need a test case here.

I have not found a test case about ls_dir, get quota and set quota, should we write a test case for these API in another PR?

Interesting; usually it's always a good idea to test whatever code is being added. I'm surprised ls_dir and other stuff isn't tested.

@dparmar18 dparmar18 requested a review from a team July 3, 2023 18:26
@YiteGu YiteGu force-pushed the cephfs-get-statfs branch 2 times, most recently from 37bd667 to 1d98985 Compare July 4, 2023 11:08
@YiteGu
Copy link
Member Author

YiteGu commented Jul 4, 2023

@dparmar18 updated:

# curl -X GET "0.0.0.0:8080/api/cephfs/1/statfs?path=/" -H  "Accept: application/vnd.ceph.api.v1.0+json" 
{"used_bytes": 4194304, "used_files": 2, "used_subdirs": 2}

@dparmar18
Copy link
Contributor

@dparmar18 updated:

# curl -X GET "0.0.0.0:8080/api/cephfs/1/statfs?path=/" -H  "Accept: application/vnd.ceph.api.v1.0+json" 
{"used_bytes": 4194304, "used_files": 2, "used_subdirs": 2}

I think just files and subdirs would make better sense than having "used" as prefix. i.e.

{"used_bytes": 4194304, "files": 2, "subdirs": 2}

@dparmar18
Copy link
Contributor

dparmar18 commented Jul 4, 2023

@dparmar18 updated:

# curl -X GET "0.0.0.0:8080/api/cephfs/1/statfs?path=/" -H  "Accept: application/vnd.ceph.api.v1.0+json" 
{"used_bytes": 4194304, "used_files": 2, "used_subdirs": 2}

I think just files and subdirs would make better sense than having "used" as prefix. i.e.

{"used_bytes": 4194304, "files": 2, "subdirs": 2}

at this point i think we can just mimic this #52218 (comment) and keep it as simple as bytes, files or subdirs

@YiteGu YiteGu force-pushed the cephfs-get-statfs branch from 1d98985 to bbb4aa4 Compare July 4, 2023 11:54
@YiteGu
Copy link
Member Author

YiteGu commented Jul 4, 2023

@dparmar18 updated:

# curl -X GET "0.0.0.0:8080/api/cephfs/1/statfs?path=/" -H  "Accept: application/vnd.ceph.api.v1.0+json" 
{"used_bytes": 4194304, "used_files": 2, "used_subdirs": 2}

I think just files and subdirs would make better sense than having "used" as prefix. i.e.

{"used_bytes": 4194304, "files": 2, "subdirs": 2}

at this point i think we can just mimic this #52218 (comment) and keep it as simple as bytes, files or subdirs

# curl -X GET "0.0.0.0:8080/api/cephfs/1/statfs?path=/" -H  "Accept: application/vnd.ceph.api.v1.0+json" 
{"used_bytes": 4194304, "files": 2, "subdirs": 2}

@dparmar18
Copy link
Contributor

@dparmar18 updated:

# curl -X GET "0.0.0.0:8080/api/cephfs/1/statfs?path=/" -H  "Accept: application/vnd.ceph.api.v1.0+json" 
{"used_bytes": 4194304, "used_files": 2, "used_subdirs": 2}

I think just files and subdirs would make better sense than having "used" as prefix. i.e.

{"used_bytes": 4194304, "files": 2, "subdirs": 2}

at this point i think we can just mimic this #52218 (comment) and keep it as simple as bytes, files or subdirs

# curl -X GET "0.0.0.0:8080/api/cephfs/1/statfs?path=/" -H  "Accept: application/vnd.ceph.api.v1.0+json" 
{"used_bytes": 4194304, "files": 2, "subdirs": 2}

@YiteGu i think just "bytes" instead of "used_bytes" would be better IMO just like what we have in rstat

# ceph daemon /var/run/ceph/ceph-mds.rook-cephfs-b.asok dump inode 1  | grep -A 7 -w rstat
    "rstat": {
        "version": 198,
        "rbytes": 23555211429,
        "rfiles": 36,
        "rsubdirs": 6,
        "rsnaps": 0,
        "rctime": "2023-01-11T14:47:27.131023+0000"
    },

@dparmar18
Copy link
Contributor

It's usually good to cleanup the changes made in a test case; you add some files to the fs; i think you should also make sure to delete them upon exit

@dparmar18
Copy link
Contributor

some doc failures

<openapi>:1: ERROR: Unexpected indentation.
/home/jenkins-build/build/workspace/ceph-pr-docs/doc/mgr/ceph_api/index.rst:353: WARNING: Block quote ends without a blank line; unexpected unindent.

https://jenkins.ceph.com/job/ceph-pr-docs/85022/console

@YiteGu YiteGu force-pushed the cephfs-get-statfs branch from a5adc8f to 63d9ec4 Compare July 11, 2023 03:13
@YiteGu
Copy link
Member Author

YiteGu commented Jul 11, 2023

It's usually good to cleanup the changes made in a test case; you add some files to the fs; i think you should also make sure to delete them upon exit

We don't seem to have an API for delete a file yet

@YiteGu YiteGu force-pushed the cephfs-get-statfs branch from 63d9ec4 to 62cd4ed Compare July 11, 2023 05:03
@dparmar18
Copy link
Contributor

It's usually good to cleanup the changes made in a test case; you add some files to the fs; i think you should also make sure to delete them upon exit

We don't seem to have an API for delete a file yet

you can use unlink from cython binding

@YiteGu
Copy link
Member Author

YiteGu commented Jul 11, 2023

unlink

got it

@YiteGu YiteGu force-pushed the cephfs-get-statfs branch from 62cd4ed to 85d0f10 Compare July 11, 2023 07:38
@YiteGu YiteGu force-pushed the cephfs-get-statfs branch from 85d0f10 to 8478c29 Compare July 11, 2023 09:47
Copy link
Contributor

@dparmar18 dparmar18 left a comment

Choose a reason for hiding this comment

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

fs changes lgtm

@YiteGu
Copy link
Member Author

YiteGu commented Jul 13, 2023

Hi, @nizamial09 help me check this make check fail about run_tox_mgr_dashboard_openapi :)

@dparmar18
Copy link
Contributor

From make check log

openapi-check: commands[1] /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/dashboard> diff openapi.yaml /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/dashboard/.tox/openapi-check/tmp/openapi.yaml
1941c1941,1985
<   /api/cephfs/{fs_id}/write_to_file:
---
>   /api/cephfs/{fs_id}/snapshot:
>     delete:
>       description: "\n        Remove a snapshot.\n        :param fs_id: The filesystem\
>         \ identifier.\n        :param path: The path of the directory.\n        :param\
>         \ name: The name of the snapshot.\n        "
>       parameters:
>       - in: path
>         name: fs_id
>         required: true
>         schema:
>           type: string
>       - in: query
>         name: path
>         required: true
>         schema:
>           type: string
>       - in: query
>         name: name
>         required: true
>         schema:
>           type: string
>       responses:
>         '202':
>           content:
>             application/vnd.ceph.api.v1.0+json:
>               type: object
>           description: Operation is still executing. Please check the task queue.
>         '204':
>           content:
>             application/vnd.ceph.api.v1.0+json:
>               type: object
>           description: Resource deleted.
>         '400':
>           description: Operation exception. Please check the response body for details.
>         '401':
>           description: Unauthenticated access. Please login first.
>         '403':
>           description: Unauthorized access. Please check your permissions.
>         '500':
>           description: Unexpected error. Please check the response body for the stack
>             trace.
>       security:
>       - jwt: []
>       tags:
>       - Cephfs
1943,1946c1987,1991
<       description: "\n        Write some data to the specified path.\n        :param\
<         \ fs_id: The filesystem identifier.\n        :param path: The path of the\
<         \ file to write.\n        :param buf: The str to write to the buf.\n     \
<         \   "
---
>       description: "\n        Create a snapshot.\n        :param fs_id: The filesystem\
>         \ identifier.\n        :param path: The path of the directory.\n        :param\
>         \ name: The name of the snapshot. If not specified, a name using the\n   \
>         \     current time in RFC3339 UTC format will be generated.\n        :return:\
>         \ The name of the snapshot.\n        :rtype: str\n        "
1958c2003
<                 buf:
---
>                 name:
1964d2008
<               - buf
2044c2088
<   /api/cephfs/{fs_id}/snapshot:
---
>   /api/cephfs/{fs_id}/tree:
2046,2048c2090,2091
<       description: "\n        Remove a snapshot.\n        :param fs_id: The filesystem\
<         \ identifier.\n        :param path: The path of the directory.\n        :param\
<         \ name: The name of the snapshot.\n        "
---
>       description: "\n        Remove a directory.\n        :param fs_id: The filesystem\
>         \ identifier.\n        :param path: The path of the directory.\n        "
2060,2064d2102
<       - in: query
<         name: name
<         required: true
<         schema:
<           type: string
2090,2094c2128,2129
<       description: "\n        Create a snapshot.\n        :param fs_id: The filesystem\
<         \ identifier.\n        :param path: The path of the directory.\n        :param\
<         \ name: The name of the snapshot. If not specified, a name using the\n   \
<         \     current time in RFC3339 UTC format will be generated.\n        :return:\
<         \ The name of the snapshot.\n        :rtype: str\n        "
---
>       description: "\n        Create a directory.\n        :param fs_id: The filesystem\
>         \ identifier.\n        :param path: The path of the directory.\n        "
2106,2107d2140
<                 name:
<                   type: string
2137c2170
<   /api/cephfs/{fs_id}/tree:
---
>   /api/cephfs/{fs_id}/unlink:
2139,2140c2172,2174
<       description: "\n        Remove a directory.\n        :param fs_id: The filesystem\
<         \ identifier.\n        :param path: The path of the directory.\n        "
---
>       description: "\n        Removes a file, link, or symbolic link.\n        :param\
>         \ fs_id: The filesystem identifier.\n        :param path: The path of the\
>         \ file or link to unlink.\n        "
2175a2210
>   /api/cephfs/{fs_id}/write_to_file:
2177,2178c2212,2215
<       description: "\n        Create a directory.\n        :param fs_id: The filesystem\
<         \ identifier.\n        :param path: The path of the directory.\n        "
---
>       description: "\n        Write some data to the specified path.\n        :param\
>         \ fs_id: The filesystem identifier.\n        :param path: The path of the\
>         \ file to write.\n        :param buf: The str to write to the buf.\n     \
>         \   "
2189a2227,2228
>                 buf:
>                   type: string
2193a2233
>               - buf
openapi-check: exit 1 (0.00 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/dashboard> diff openapi.yaml /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/dashboard/.tox/openapi-check/tmp/openapi.yaml pid=649450
  openapi-check: FAIL code 1 (61.58=setup[59.00]+cmd[2.58,0.00] seconds)
  evaluation failed :( (61.71 seconds)

@nizamial09
Copy link
Member

Hi, @nizamial09 help me check this make check fail about run_tox_mgr_dashboard_openapi :)

you can generate a new openapi spec using the command tox -e openapi-fix from the src/pybind/mgr/dashboard folder.
https://docs.ceph.com/en/latest/dev/developer_guide/dash-devel/#rest-api-documentation

@YiteGu YiteGu force-pushed the cephfs-get-statfs branch 2 times, most recently from dd5dd1e to ea38205 Compare July 13, 2023 11:00
@YiteGu
Copy link
Member Author

YiteGu commented Jul 13, 2023

From make check log

openapi-check: commands[1] /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/dashboard> diff openapi.yaml /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/dashboard/.tox/openapi-check/tmp/openapi.yaml
1941c1941,1985
<   /api/cephfs/{fs_id}/write_to_file:
---
>   /api/cephfs/{fs_id}/snapshot:
>     delete:
>       description: "\n        Remove a snapshot.\n        :param fs_id: The filesystem\
>         \ identifier.\n        :param path: The path of the directory.\n        :param\
>         \ name: The name of the snapshot.\n        "
>       parameters:
>       - in: path
>         name: fs_id
>         required: true
>         schema:
>           type: string
>       - in: query
>         name: path
>         required: true
>         schema:
>           type: string
>       - in: query
>         name: name
>         required: true
>         schema:
>           type: string
>       responses:
>         '202':
>           content:
>             application/vnd.ceph.api.v1.0+json:
>               type: object
>           description: Operation is still executing. Please check the task queue.
>         '204':
>           content:
>             application/vnd.ceph.api.v1.0+json:
>               type: object
>           description: Resource deleted.
>         '400':
>           description: Operation exception. Please check the response body for details.
>         '401':
>           description: Unauthenticated access. Please login first.
>         '403':
>           description: Unauthorized access. Please check your permissions.
>         '500':
>           description: Unexpected error. Please check the response body for the stack
>             trace.
>       security:
>       - jwt: []
>       tags:
>       - Cephfs
1943,1946c1987,1991
<       description: "\n        Write some data to the specified path.\n        :param\
<         \ fs_id: The filesystem identifier.\n        :param path: The path of the\
<         \ file to write.\n        :param buf: The str to write to the buf.\n     \
<         \   "
---
>       description: "\n        Create a snapshot.\n        :param fs_id: The filesystem\
>         \ identifier.\n        :param path: The path of the directory.\n        :param\
>         \ name: The name of the snapshot. If not specified, a name using the\n   \
>         \     current time in RFC3339 UTC format will be generated.\n        :return:\
>         \ The name of the snapshot.\n        :rtype: str\n        "
1958c2003
<                 buf:
---
>                 name:
1964d2008
<               - buf
2044c2088
<   /api/cephfs/{fs_id}/snapshot:
---
>   /api/cephfs/{fs_id}/tree:
2046,2048c2090,2091
<       description: "\n        Remove a snapshot.\n        :param fs_id: The filesystem\
<         \ identifier.\n        :param path: The path of the directory.\n        :param\
<         \ name: The name of the snapshot.\n        "
---
>       description: "\n        Remove a directory.\n        :param fs_id: The filesystem\
>         \ identifier.\n        :param path: The path of the directory.\n        "
2060,2064d2102
<       - in: query
<         name: name
<         required: true
<         schema:
<           type: string
2090,2094c2128,2129
<       description: "\n        Create a snapshot.\n        :param fs_id: The filesystem\
<         \ identifier.\n        :param path: The path of the directory.\n        :param\
<         \ name: The name of the snapshot. If not specified, a name using the\n   \
<         \     current time in RFC3339 UTC format will be generated.\n        :return:\
<         \ The name of the snapshot.\n        :rtype: str\n        "
---
>       description: "\n        Create a directory.\n        :param fs_id: The filesystem\
>         \ identifier.\n        :param path: The path of the directory.\n        "
2106,2107d2140
<                 name:
<                   type: string
2137c2170
<   /api/cephfs/{fs_id}/tree:
---
>   /api/cephfs/{fs_id}/unlink:
2139,2140c2172,2174
<       description: "\n        Remove a directory.\n        :param fs_id: The filesystem\
<         \ identifier.\n        :param path: The path of the directory.\n        "
---
>       description: "\n        Removes a file, link, or symbolic link.\n        :param\
>         \ fs_id: The filesystem identifier.\n        :param path: The path of the\
>         \ file or link to unlink.\n        "
2175a2210
>   /api/cephfs/{fs_id}/write_to_file:
2177,2178c2212,2215
<       description: "\n        Create a directory.\n        :param fs_id: The filesystem\
<         \ identifier.\n        :param path: The path of the directory.\n        "
---
>       description: "\n        Write some data to the specified path.\n        :param\
>         \ fs_id: The filesystem identifier.\n        :param path: The path of the\
>         \ file to write.\n        :param buf: The str to write to the buf.\n     \
>         \   "
2189a2227,2228
>                 buf:
>                   type: string
2193a2233
>               - buf
openapi-check: exit 1 (0.00 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/dashboard> diff openapi.yaml /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/dashboard/.tox/openapi-check/tmp/openapi.yaml pid=649450
  openapi-check: FAIL code 1 (61.58=setup[59.00]+cmd[2.58,0.00] seconds)
  evaluation failed :( (61.71 seconds)

make check completed

@nizamial09
Copy link
Member

jenkins test dashboard cephadm

Copy link
Member

@nizamial09 nizamial09 left a comment

Choose a reason for hiding this comment

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

Thanks for addressing all the comments @YiteGu

one more thing: you'll need to attach the tracker to the commit with a fixes message.

Fixes: <tracker-url>

Introduce statfs for the CephFS REST API controller, we
can easily get statfs of the specified path by it, it returns
the used bytes, used files and used subdirs.

Fixes: https://tracker.ceph.com/issues/61883
Signed-off-by: Yite Gu <yitegu0@gmail.com>
@YiteGu YiteGu force-pushed the cephfs-get-statfs branch from ea38205 to bcbf91d Compare July 14, 2023 09:00
@YiteGu
Copy link
Member Author

YiteGu commented Jul 14, 2023

Fixes: https://tracker.ceph.com/issues/61883

fixes message done

@pereman2
Copy link
Contributor

jenkins test make check

@avanthakkar
Copy link
Contributor

jenkins test windows

Copy link
Contributor

@avanthakkar avanthakkar left a comment

Choose a reason for hiding this comment

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

LGTM!

@nizamial09 nizamial09 merged commit 7f26596 into ceph:main Jul 25, 2023
@nizamial09
Copy link
Member

thank you @YiteGu

@nizamial09
Copy link
Member

@YiteGu should this be backported to reef and quincy? if so can you update the tracker? https://tracker.ceph.com/issues/61883

@YiteGu
Copy link
Member Author

YiteGu commented Jul 26, 2023

@YiteGu should this be backported to reef and quincy? if so can you update the tracker? https://tracker.ceph.com/issues/61883

  1. ok, I am pleasure to do backport work, this my first time :)
  2. I seem don't have permission add backport to tracker

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