Skip to content

quincy: mds: allow all types of mds caps#52582

Closed
rishabh-d-dave wants to merge 3 commits intoceph:quincyfrom
rishabh-d-dave:wip-62027-quincy
Closed

quincy: mds: allow all types of mds caps#52582
rishabh-d-dave wants to merge 3 commits intoceph:quincyfrom
rishabh-d-dave:wip-62027-quincy

Conversation

@rishabh-d-dave
Copy link
Contributor

backport tracker: https://tracker.ceph.com/issues/62027


backport of #51317
parent tracker: https://tracker.ceph.com/issues/59388

this backport was staged using ceph-backport.sh version 16.0.0.6848
find the latest version at https://github.com/ceph/ceph/blob/master/src/script/ceph-backport.sh

@rishabh-d-dave rishabh-d-dave added this to the quincy milestone Jul 21, 2023
@rishabh-d-dave rishabh-d-dave added the cephfs Ceph File System label Jul 21, 2023
@github-actions github-actions bot added the tests label Jul 21, 2023
@github-actions
Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

MDS caps can contain 5 components: name of a CephFS, a path inside
CephFS, a flag for enabling root squashing mechanism, a UID and list of
GIDs. These 5 components result in 31 combinations, so there can be 31
types of MDS caps. Out of these, the current main branch only allows 11
combinations. This restriction is strange and inappropriate. Ideally,
all combinations should be allowed.

This strange restriction must've been created unintentionally by
previous developers while adding FS name and root squash to MDS caps. A
TODO for a allowing a subset of these combination was also left in
codebase:
https://github.com/ceph/ceph/blob/reef/src/mds/MDSAuthCaps.cc#L69

Fixes: https://tracker.ceph.com/issues/59388
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 0c36929)

Conflicts:
	src/mds/MDSAuthCaps.cc
	- Unlike main branch, on Quincy branch, MDSAuthCaps calls string
	  by writing std::string instead of just string.
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 248e9da)

Conflicts:
	src/test/mds/TestMDSAuthCaps.cc
	- Unlike main branch, on Quincy branch, MDSAuthCaps::parse()
	  method accepts instance of class CephContext as an argument.
Currently we check if the right caps parse successfully and the wrong
caps fail to parse. Increase the test coverage by adding one more
tests -- dump the right caps in to a string after the parsing is
successfully and then re-parse the caps and check if re-parsed caps
dumps to the exact same string.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit bfacee1)
@kotreshhr
Copy link
Contributor

@rishabh-d-dave could you check on make check failure?

@rishabh-d-dave
Copy link
Contributor Author

make check job logs are not available - https://jenkins.ceph.com/job/ceph-pull-requests/129456/

@rishabh-d-dave
Copy link
Contributor Author

jenkins test make check

1 similar comment
@leonid-s-usov
Copy link
Contributor

jenkins test make check

@lxbsz
Copy link
Member

lxbsz commented Aug 2, 2024

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

@github-actions
Copy link

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.

@rishabh-d-dave
Copy link
Contributor Author

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

@vshankar Any update about the QA run for this PR?

@vshankar
Copy link
Contributor

vshankar commented Jan 2, 2025

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

@vshankar
Copy link
Contributor

vshankar commented Jan 5, 2025

@rishabh-d-dave Please help me recall if there was a bug that got introduced due to this change (in main) and those commits need to be additionally backported. Would you know?

@vshankar
Copy link
Contributor

Quincy is EOL.

@vshankar vshankar closed this Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cephfs Ceph File System tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants