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

Jump to bottom

nautilus: mgr/volume: subvolume auth_id management and few bug fixes #39292

Merged
merged 20 commits into from Feb 12, 2021
Merged

nautilus: mgr/volume: subvolume auth_id management and few bug fixes #39292

merged 20 commits into from Feb 12, 2021

Conversation

kotreshhr
Copy link
Contributor

@kotreshhr kotreshhr commented Feb 4, 2021 •
edited

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

backport of #36998
parent tracker: https://tracker.ceph.com/issues/40401

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

backport of #38652
parent tracker: https://tracker.ceph.com/issues/48501

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

backport of #38160
parent tracker: https://tracker.ceph.com/issues/44931

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

backport of #38786
parent tracker: https://tracker.ceph.com/issues/44928

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

backport of #39038
parent tracker: https://tracker.ceph.com/issues/48830

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

backport of #39327
parent tracker: https://tracker.ceph.com/issues/49192

Included following commits related to tests as well:

  1. 4ca8aaa
  2. 5945192
  3. The commit 847a7ad is pulled from master for test suite apis. It's part of big PR mon, cephfs: Add auth caps for CephFS fsids #32581

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
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 api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@kotreshhr kotreshhr changed the title Wip mgr backports nautilus nautilus: mgr/volume: subvolume auth_id management and few bug fixes Feb 4, 2021
@kotreshhr kotreshhr requested review from ajarr and batrick and removed request for ajarr February 4, 2021 10:47
@kotreshhr kotreshhr added nautilus-batch-1 nautilus point releases cephfs Ceph File System needs-qa labels Feb 4, 2021
@kotreshhr
Copy link
Contributor Author

http://pulpito.front.sepia.ceph.com/khiremat-2021-02-04_10:50:22-fs-wip-khiremat-mgr-authid-bp-nautilus-5-distro-basic-smithi/

@kotreshhr
Copy link
Contributor Author

jenkins test make check

ajarr and others added 17 commits February 5, 2021 23:56
@ajarr @kotreshhr
mon/MonCap: allow 'profile mgr' to create/update/del auth IDs
913be51 
Signed-off-by: Ramana Raja <rraja@redhat.com>
(cherry picked from commit d3aea55)
@ajarr @kotreshhr
mgr/volumes: Allow/deny auth IDs access to FS subvolumes
d9ddd57 
... via the `ceph fs subvolume authorize/deauthorize` command.

Fixes: https://tracker.ceph.com/issues/40401
Signed-off-by: Ramana Raja <rraja@redhat.com>
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 6c3b754)

Conflicts:
    src/pybind/mgr/volumes/fs/volume.py: subvolume pin is not available
in nautilus.
@ajarr @kotreshhr
qa/tasks/cephfs: test fs subvolume authorize/deauthorize
c98665f 
Fixes: https://tracker.ceph.com/issues/40401
Signed-off-by: Ramana Raja <rraja@redhat.com>
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 7c98dc1)

Conflicts:
    qa/tasks/cephfs/test_volumes.py: Few of the tests are re-organized
        and CLIENTS_REQUIRED should be 2 for these tests which was 1
        for existing tests. Resolved the same.
@kotreshhr
doc/mgr/volumes: Document authorize/deauthorize cli commands
48b2259 
Fixes: https://tracker.ceph.com/issues/40401
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 1426c23)
@kotreshhr
mgr/volumes: Filter inherited snapshots while listing snapshots
535976b 
Filter inherited snapshots resulted as part of a snapshot
at ancestor level while listing snapshots of a subvolume
and subvolumegroup

Also, fail the snapshot info on inherited snapshot.

Fixes: https://tracker.ceph.com/issues/48501
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit bd49b64)

Conflicts:
    qa/tasks/cephfs/test_volumes.py: Few of the tests are re-organized,
      hence the conflicts. Resolved the same.
@kotreshhr
mgr/volumes: Persist auth and subvolume metadata
f5c64b8 
1. Subvolume create and delete operations create and delete subvolume
   metadata file respectively.
2. Subvolume authorize creates the auth meta file and persists the
   required metadata on subvolume metadata file and auth metdata file
   on disk. Subvolume deauthorize clears the required metadata on
   both metadata files.

Fixes: https://tracker.ceph.com/issues/44931
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 04d876c)
@kotreshhr
mgr/volumes: Add tenant_id option to subvolume authorize
228a5c0 
Fixes: https://tracker.ceph.com/issues/44931
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 39acfcc)
@kotreshhr
mgr/volumes: Add subvolume authorized_list command
c02b948 
Fixes: https://tracker.ceph.com/issues/44931
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 1abec3d)
@kotreshhr
mgr/volumes: Disallow authorize existing auth_id
e027783 
This patch disallow the mgr plugin to authorize the auth_id
which is not created via mgr plugin. Those auth_ids could be
created by other means for other use cases which should not be modified
via mgr plugin.

Fixes: https://tracker.ceph.com/issues/44931
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit c028904)
@kotreshhr
mgr/volumes: Preserve existing caps while authorize/deauthorize auth-id
0fff2f7 
Authorize/Deauthorize used to overwrite the caps of auth-id which would
end up deleting existing caps. This patch fixes the same by retaining
the existing caps by appending or deleting the new caps as needed.

Fixes: https://tracker.ceph.com/issues/44931
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 2dece3b)
@kotreshhr
mgr/volumes: Optionally authorize existing auth-ids
89cb03b 
Optionally allow authorizing auth-ids not created by mgr plugin
via the option 'allow_existing_id'. This can help existing deployers
of manila to disallow/allow authorization of pre-created auth IDs
via a manila driver config that sets 'allow_existing_id' to False/True.

Fixes: https://tracker.ceph.com/issues/44931
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 713270d)
@kotreshhr
mgr/volumes: Update the 'volumes' key to 'subvolumes' in auth metadat…
b280c88 
…a file

The older auth metadata files created by CephVolumeClient stores the
authorized subvolumes using the 'volumes' key as the notion of
'subvolumes' brought in by mgr/volumes. Hence, this would be tranparently
updated to 'subvolumes' and newer auth metadata files would store them
with 'subvolumes' key.

Also fails the deauthorize if the auth-id doesn't exist.

Fixes: https://tracker.ceph.com/issues/44931
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 5f32eb1)
@kotreshhr
qa: Add tests for list auth-ids of a subvolume
5f9e2e0 
Fixes: https://tracker.ceph.com/issues/44931
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 1b98e63)

Conflicts:
    qa/tasks/cephfs/test_volumes.py: Few of the tests are re-organized,
     hence the conflicts. Resolved the same.
@kotreshhr
doc/mgr/volumes: Document 'fs subvolume authorized_list' cli
d077503 
Fixes: https://tracker.ceph.com/issues/44931
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 9f9f8ad)
@kotreshhr
mgr/volumes: Evict clients based on auth-IDs and subvolume mounted
8fa1284 
Add subvolume evict command which evicts the subvolume mounts
which are mounted using particular auth-ID.

Fixes: https://tracker.ceph.com/issues/44928
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 269adcc)

Conflicts:
    qa/tasks/cephfs/test_volumes.py: Few of the tests are re-organized,
      hence the conflicts. Resolved the same.
@kotreshhr
ceph_volume_client: Fix failure of test_idempotency
113893e 
With the test environment, 'args must be encodeable
 as a bytearray' error is seen for 'ceph_mds_command'.
Hence removed tuple and passed the JSON formatted string.

Fixes: https://tracker.ceph.com/issues/48830
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 1c6c172)
@rishabh-d-dave @kotreshhr
qa/cephfs: add methods to read/write on CephFS mounts
3b36899 
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 3f0284f)

Conflicts:
    qa/tasks/cephfs/mount.py: get_file and IP module is not present in
        nautilus
ajarr
ajarr requested changes Feb 11, 2021
View reviewed changes
Copy link
Contributor

@ajarr ajarr left a comment

Choose a reason for hiding this comment

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

Nice work, Kotresh.

Do we need to add 113893e that fixes test_volume_client as part of this PR? We can add that as a different PR but include in the same testing branch, or are the commits intertwined?

The PR to fix mgr/volume tests in master has not merged yet. We'll need to backport the merged commit here instead of 3aa0bc9

@ajarr
Copy link
Contributor

ajarr commented Feb 11, 2021

@kotreshhr do you think the following commits also need to be backported,
4ca8aaa
5945192

@kotreshhr
Copy link
Contributor Author

Nice work, Kotresh.

Do we need to add 113893e that fixes test_volume_client as part of this PR? We can add that as a different PR but include in the same testing branch, or are the commits intertwined?

This is already part of this batch.

The PR to fix mgr/volume tests in master has not merged yet. We'll need to backport the merged commit here instead of 3aa0bc9

Yes, you are right. I have included that to see if teuthology run looks fine as for some reason those cases are hitting in nautilus branch consistently but I am currently stuck at this crash[1]. I will pull the commit after it's merged in master.

[1] https://tracker.ceph.com/issues/49213

@kotreshhr
Copy link
Contributor Author

@kotreshhr do you think the following commits also need to be backported,
4ca8aaa
5945192

Good to have these. I will include them.

@ajarr
Copy link
Contributor

ajarr commented Feb 12, 2021 •
edited

@kotreshhr please update the branch with the mgr/volumes test fixes that were merged in master,
ceph/ceph-ci@4952d21

Also, as mentioned in the tracker ticket https://tracker.ceph.com/issues/49213#note-2, the MON crash is observed during filesystem removal. Can you try skipping or removing that test_volume_rm test in the testing branch and run teuthology?

@ajarr
Copy link
Contributor

ajarr commented Feb 12, 2021

@yuriw I suggest that you remove this from your testing branch. @kotreshhr needs to update this PR. I think it's better if kotresh runs the teuthology tests for this PR.

kotreshhr and others added 3 commits February 12, 2021 15:23
@kotreshhr
qa: Fix a few mgr/volume test cases
038e94e 
Recovering dirty auth metadata file might not retain the order,
fixed the comparison in 'test_recover_auth_metadata_during_authorize'
and 'test_recover_auth_metadata_during_deauthorize'.

Fixes: https://tracker.ceph.com/issues/49192
Signed-off-by: Kotresh HR <khiremat@redhat.com>
(cherry picked from commit 4952d21)
@batrick @kotreshhr
qa: unmount volumes before removal
9056e39 
To avoid potential failures/hangs in umount.

Fixes: https://tracker.ceph.com/issues/23718
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
(cherry picked from commit 4ca8aaa)
@batrick @kotreshhr
qa: leave one standby available to avoid warning
89aeece 
Four file systems will use all MDS and generate this warning:

	2020-11-02T03:48:33.407 INFO:teuthology.orchestra.run.smithi003.stdout:2020-11-02T03:24:21.817337+0000 mon.a (mon.0) 481 : cluster [WRN] Health check failed: insufficient standby MDS daemons available (MDS_INSUFFICIENT_STANDBY)

Fixes: https://tracker.ceph.com/issues/23718
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
(cherry picked from commit 5945192)
@kotreshhr
Copy link
Contributor Author

kotreshhr commented Feb 12, 2021 •
edited

Finally all the tests passed. Thanks @ajarr, adding two test fixes 4ca8aaa, 5945192 helped. Did not have to skip test_volume_rm test case.

Teuthology Results:

http://pulpito.front.sepia.ceph.com/khiremat-2021-02-12_11:06:00-fs-wip-khiremat-mgr-authid-bp-9-nautilus-distro-basic-smithi/

@kotreshhr
Copy link
Contributor Author

@kotreshhr please update the branch with the mgr/volumes test fixes that were merged in master,
ceph/ceph-ci@4952d21

Done

Also, as mentioned in the tracker ticket https://tracker.ceph.com/issues/49213#note-2, the MON crash is observed during filesystem removal. Can you try skipping or removing that test_volume_rm test in the testing branch and run teuthology?

Adding those test cases helped. Didn't have to skip the test case.

@ajarr ajarr merged commit 1d5ad07 into ceph:nautilus Feb 12, 2021
@batrick
Copy link
Member

batrick commented Mar 13, 2021

follow-up: #40095

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajarr ajarr ajarr approved these changes

@batrick batrick Awaiting requested review from batrick

@fullerdj fullerdj Awaiting requested review from fullerdj

Assignees
No one assigned
Labels
cephfs Ceph File System nautilus-batch-1 nautilus point releases
Projects
None yet
Milestone
No milestone
5 participants
@kotreshhr @ajarr @batrick @yuriw @rishabh-d-dave