mgr/volumes: Keep mon caps if auth key has remaining mds/osd caps by ebocchi · Pull Request #59435 · ceph/ceph

ebocchi · 2024-08-26T11:40:24Z

Fixes: https://tracker.ceph.com/issues/67708
Signed-off-by: Enrico Bocchi enrico.bocchi@cern.ch

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

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)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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
jenkins test rook e2e

batrick · 2024-08-29T20:37:42Z

jenkins test make check

batrick · 2024-08-29T20:37:44Z

jenkins test make check arm64

mchangir

LGTM

mchangir · 2024-09-04T03:43:59Z

@ebocchi please add a test in qa/tasks/cephfs/test_volumes.py to confirm that the fix works

ebocchi · 2024-10-16T09:47:52Z

@mchangir Thanks for looking into this PR.
I have added the test as you requested.

mchangir · 2024-10-18T09:49:57Z

@mchangir Thanks for looking into this PR. I have added the test as you requested.

Thanks Enrico, the test looks good.
Now, we need to get the test exercised in teuthology.

vshankar · 2024-10-29T04:54:34Z

This PR is under test in https://tracker.ceph.com/issues/68744.

vshankar · 2024-11-05T05:31:26Z

qa/tasks/cephfs/test_volumes.py

+
+        # check mon caps for authid
+        expected_mon_caps = 'allow r'
+        full_caps = self._raw_cmd("auth", "get", "client.alice", "--format=json-pretty")


@ebocchi - this call needs to be wrapped around json.loads(), else accessing the json would fail as it fails in

https://pulpito.ceph.com/vshankar-2024-10-30_07:41:03-fs-wip-vshankar-testing-20241029.045257-debug-testing-default-smithi/7973229/

024-10-31T01:31:12.006 INFO:tasks.cephfs_test_runner:====================================================================== 2024-10-31T01:31:12.006 INFO:tasks.cephfs_test_runner:ERROR: test_subvolume_deauthorize_with_shared_key (tasks.cephfs.test_volumes.TestSubvolumes) 2024-10-31T01:31:12.006 INFO:tasks.cephfs_test_runner:That mon caps are preserved when one cephx key authorized on multiple 2024-10-31T01:31:12.006 INFO:tasks.cephfs_test_runner:---------------------------------------------------------------------- 2024-10-31T01:31:12.006 INFO:tasks.cephfs_test_runner:Traceback (most recent call last): 2024-10-31T01:31:12.006 INFO:tasks.cephfs_test_runner: File "/home/teuthworker/src/git.ceph.com_ceph-c_73c971683afc1322bab0582d2b64477919ac88b7/qa/tasks/cephfs/test_volumes.py", line 2855, in test_subvolume_deauthorize_with_shared_key 2024-10-31T01:31:12.006 INFO:tasks.cephfs_test_runner: self.assertEqual(expected_mon_caps, full_caps[0]['caps']['mon']) 2024-10-31T01:31:12.006 INFO:tasks.cephfs_test_runner:TypeError: string indices must be integers 2024-10-31T01:31:12.006 INFO:tasks.cephfs_test_runner: 2024-10-31T01:31:12.006 INFO:tasks.cephfs_test_runner:----------------------------------------------------------------------

Sorry, missed that.
It should be fixed with 8c28c1e.

I'll put this to test. The commit can be squashed later when the change is ready to merge. Thx @ebocchi

vshankar · 2024-11-12T10:57:14Z

cc @mchangir for QA testing.

mchangir · 2024-11-25T09:52:54Z

This PR is under test in https://tracker.ceph.com/issues/69038.

github-actions · 2025-01-24T10:02:03Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2025-03-25T11:01:27Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

ceph-jenkins · 2025-03-25T11:01:31Z

Can one of the admins verify this patch?

vshankar · 2025-03-25T11:23:35Z

This PR is under test in https://tracker.ceph.com/issues/69038.

@mchangir I see https://tracker.ceph.com/issues/69038#note-2 and I guess the review is done and this is ready to merge, yes?

github-actions · 2025-05-24T14:01:23Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

vshankar · 2025-05-25T11:38:17Z

jenkins retest this please

vshankar · 2025-05-29T13:41:29Z

This PR is under test in https://tracker.ceph.com/issues/71496.

* refs/pull/59435/head: mgr/volumes: Fix json.loads for test on mon caps mgr/volumes: Add test for mon caps if auth key has remaining mds/osd caps mgr/volumes: Keep mon caps if auth key has remaining mds/osd caps Reviewed-by: Venky Shankar <vshankar@redhat.com> Reviewed-by: Milind Changire <mchangir@redhat.com>

vshankar

https://tracker.ceph.com/projects/cephfs/wiki/QA_main_2025#wip-vshankar-testing-20250618123141-debug

vshankar · 2025-06-20T06:59:05Z

jenkins retest this please

vshankar · 2025-06-20T09:12:34Z

jenkins test make check arm64

vshankar · 2025-06-20T10:06:50Z

jenkins test api

* refs/pull/59435/head: mgr/volumes: Fix json.loads for test on mon caps mgr/volumes: Add test for mon caps if auth key has remaining mds/osd caps mgr/volumes: Keep mon caps if auth key has remaining mds/osd caps Reviewed-by: Milind Changire <mchangir@redhat.com> Reviewed-by: Venky Shankar <vshankar@redhat.com>

Signed-off-by: Enrico Bocchi <enrico.bocchi@cern.ch>

…caps Signed-off-by: Enrico Bocchi <enrico.bocchi@cern.ch>

Signed-off-by: Enrico Bocchi <enrico.bocchi@cern.ch>

vshankar · 2025-06-23T07:23:20Z

rebased and push to get the jenkins tests working.

github-actions bot added cephfs Ceph File System pybind labels Aug 26, 2024

ebocchi force-pushed the keep-mon-caps-for-remaining-subvolumes branch from b164f6c to 221a91b Compare August 26, 2024 11:44

batrick requested review from a team, kotreshhr and mchangir August 29, 2024 20:37

batrick added the needs-review label Aug 29, 2024

mchangir approved these changes Sep 4, 2024

View reviewed changes

ebocchi force-pushed the keep-mon-caps-for-remaining-subvolumes branch from 221a91b to 44f21dc Compare October 16, 2024 09:42

github-actions bot added the tests label Oct 16, 2024

ebocchi force-pushed the keep-mon-caps-for-remaining-subvolumes branch from 44f21dc to ee27143 Compare October 16, 2024 09:45

vshankar assigned mchangir Oct 17, 2024

vshankar self-assigned this Oct 22, 2024

vshankar approved these changes Oct 28, 2024

View reviewed changes

vshankar added the wip-vshankar-testing label Oct 28, 2024

vshankar reviewed Nov 5, 2024

View reviewed changes

vshankar removed the wip-vshankar-testing label Nov 5, 2024

gregsfortytwo added the needs-qa label Nov 11, 2024

github-actions bot added the stale label Jan 24, 2025

vshankar removed the stale label Jan 24, 2025

github-actions bot added the stale label Mar 25, 2025

github-actions bot removed the stale label Mar 25, 2025

github-actions bot added the stale label May 24, 2025

vshankar removed the stale label May 25, 2025

vshankar added the wip-vshankar-testing label May 25, 2025

vshankar approved these changes Jun 20, 2025

View reviewed changes

vshankar removed the wip-vshankar-testing label Jun 20, 2025

vshankar added the ready-to-merge label Jun 22, 2025

ebocchi added 3 commits June 23, 2025 12:52

mgr/volumes: Keep mon caps if auth key has remaining mds/osd caps

0882bbe

Signed-off-by: Enrico Bocchi <enrico.bocchi@cern.ch>

mgr/volumes: Add test for mon caps if auth key has remaining mds/osd …

403d541

…caps Signed-off-by: Enrico Bocchi <enrico.bocchi@cern.ch>

mgr/volumes: Fix json.loads for test on mon caps

b008ef9

Signed-off-by: Enrico Bocchi <enrico.bocchi@cern.ch>

vshankar force-pushed the keep-mon-caps-for-remaining-subvolumes branch from 8c28c1e to b008ef9 Compare June 23, 2025 07:23

vshankar merged commit d78ffd1 into ceph:main Jun 25, 2025
13 checks passed

vshankar removed the ready-to-merge label Jun 25, 2025

This was referenced Aug 27, 2025

tentacle: mgr/volumes: Keep mon caps if auth key has remaining mds/osd caps #65262

Merged

squid: mgr/volumes: Keep mon caps if auth key has remaining mds/osd caps #65296

Open

reef: mgr/volumes: Keep mon caps if auth key has remaining mds/osd caps #65297

Merged

Conversation

ebocchi commented Aug 26, 2024

Contribution Guidelines

Checklist

Uh oh!

batrick commented Aug 29, 2024

Uh oh!

batrick commented Aug 29, 2024

Uh oh!

mchangir left a comment

Choose a reason for hiding this comment

Uh oh!

mchangir commented Sep 4, 2024

Uh oh!

ebocchi commented Oct 16, 2024

Uh oh!

mchangir commented Oct 18, 2024

Uh oh!

vshankar commented Oct 29, 2024

Uh oh!

vshankar Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

ebocchi Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

vshankar Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

vshankar commented Nov 12, 2024

Uh oh!

mchangir commented Nov 25, 2024

Uh oh!

github-actions bot commented Jan 24, 2025

Uh oh!

github-actions bot commented Mar 25, 2025

Uh oh!

ceph-jenkins commented Mar 25, 2025

Uh oh!

vshankar commented Mar 25, 2025

Uh oh!

github-actions bot commented May 24, 2025

Uh oh!

vshankar commented May 25, 2025

Uh oh!

vshankar commented May 29, 2025

Uh oh!

vshankar left a comment

Choose a reason for hiding this comment

Uh oh!

vshankar commented Jun 20, 2025

Uh oh!

vshankar commented Jun 20, 2025

Uh oh!

vshankar commented Jun 20, 2025

Uh oh!

vshankar commented Jun 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants