qa/cephfs: clean up evicted client in 4-compat_client.yaml by rishabh-d-dave · Pull Request #46988 · ceph/ceph

rishabh-d-dave · 2022-07-06T10:21:37Z

Related PR - PR #45036

4-compat_client.yaml in creates two clients and evicts one of them. The
evicted client is not cleaned up later, that is it's left unmounted and
the mount point is left undeleted. This doesn't cause failure during
final teardown for main branch but with PR #45036 it does lead to
failure every time.

PR #45036 changes the fact that CephFS code in directory "qa" depends on
value of attribute "is_mounted" to check if a CephFS has been unmounted
or not. Instead, it runs "findmnt" command to check if the client was
actually unmounted.

Operating on a CephFS mountpoint after the client has been evicted
causes the operation to hang. Thus with PR #45036 the final teardown
for teuthology job fails every time.

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

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. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

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

rishabh-d-dave · 2022-07-06T10:23:11Z

jenkins test api

rishabh-d-dave · 2022-07-06T10:24:09Z

@vshankar @lxbsz The fix and the issue have been verified by running the failing job again.

http://pulpito.front.sepia.ceph.com/rishabh-2022-07-06_08:49:09-fs-wip-vshankar-testing-20220527-073645-distro-default-smithi/
http://pulpito.front.sepia.ceph.com/rishabh-2022-07-06_05:23:51-fs-wip-vshankar-testing-20220527-073645-distro-default-smithi

vshankar · 2022-07-06T12:52:02Z

qa/suites/fs/upgrade/featureful_client/upgraded_client/tasks/4-compat_client.yaml

    clients:
      client.0: False
      client.1: True
+# cleanup evicted client so there's no trouble later.


nit: add a comment that only client.0 is upgraded and client.1 is evicted by the mds due to missing feature compat set.

Done - https://github.com/ceph/ceph/pull/46988/files#diff-12f10acf04fc223775779597acad35c68ffd3e591bc4eb79df53a4b224dceabcR12.

rishabh-d-dave · 2022-07-06T13:26:18Z

Tested PR #45036 and PR #46988. Ran fine - http://pulpito.front.sepia.ceph.com/rishabh-2022-07-06_11:37:52-fs-wip-vshankar-testing-20220527-073645-distro-default-smithi/. This PR is ready for QA now.

lxbsz · 2022-07-07T02:17:24Z

qa/suites/fs/upgrade/featureful_client/upgraded_client/tasks/4-compat_client.yaml

+      - cat mntpt.txt
+      - sudo umount -f $(cat mntpt.txt)
+      - sudo rmdir $(cat mntpt.txt)
+      - rm mntpt.txt


How about doing this in clients_evicted() in tasks/fs.py instead ? Then it should be very simple by:

mount.umount_wait()

Could it lead to a problem for teuthology jobs that wants to keep evicted clients as it is around for some time?

There are only two places using this, I didn't see it could lead potential issues.

And from my understanding the fs.clients_evicted: is where needs the evicted clients as it is around for some time. So after this or in this to unmount them should be fine.

Okay, I'll make the change in that case.

You'd only need to make sure that the findmnt (from your changes) does not interleave anywhere, which then could lead to a test hang.

Done. PTAL.

rishabh-d-dave · 2022-07-07T19:35:18Z

Tested new change, it work fine - http://pulpito.front.sepia.ceph.com/rishabh-2022-07-07_17:52:35-fs-main-distro-default-smithi/

rishabh-d-dave · 2022-07-07T19:52:03Z

jenkins test make check

rishabh-d-dave · 2022-07-07T20:07:48Z

jenkins test api

rishabh-d-dave · 2022-07-08T03:35:19Z

jenkins test api

rishabh-d-dave · 2022-07-08T07:38:35Z

https://jenkins.ceph.com/job/ceph-api/40322/

rishabh-d-dave · 2022-07-08T07:38:40Z

jenkins test api

vshankar · 2022-08-10T14:23:28Z

@rishabh-d-dave ping?

rishabh-d-dave · 2022-08-10T16:59:01Z

Spoke to @rishabh-d-dave today. Seems like the pending issue with this PR is to identify if the client has been blocklisted and then (force) unmount it. I suggested that the we save the client address after a successful mount (mount.py) and then during class destruction (or in some helper) check if the address is part of the blocklisted client and then do the needful.

Does that make sense?

Elaborating on the problem: The Python code hangs when a blocked client is being operated on even when timeout option is passed to run() method, timeout option is passed to findmnt command and/or timeout command is used to kill findmnt or any other command. All of these methods have been tried in previous version of this PR. (One more option was tried besides these: run umount -fl <mntpt> for blocked client's mountpoint from within the 4-compat_client.yaml)

The way around this is (which is currently on this PR) to set wait=False and treat client as blocked when no response is received for X seconds. This approach too is not so nice since it involves waiting for X to 2X or 3X seconds (depending on how many times is_stuck() was called) for every mount.

Noting the client socket in class constructor and checking if it is blocked in class destructor should be a better way to deal with this issue. I'll try this out and post the result.

Add a note explaining the reason behind the eviction of "client.1" during this test. Signed-off-by: Rishabh Dave <ridave@redhat.com>

Signed-off-by: Rishabh Dave <ridave@redhat.com>

github-actions · 2022-08-19T14:12:46Z

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

rishabh-d-dave · 2022-08-19T15:28:55Z

Tested this PR individually, testing was successful - http://pulpito.front.sepia.ceph.com/rishabh-2022-08-19_14:15:36-fs-wip-rishabh-client-evict-distro-default-smithi/.

Adding it for QA run.

Before unmounting check if the client has been evicted and, if so, run "umount -f -l" for the mount point of the client and cleanup the mount right after it. Attempting to unmount, cleanup or operate in any way over mount point of a evicted client will hang the operation (and thereby our Python code too). Lazy-force unmount prevents such hangs for our Python code and also frees the mount point. This commit also adds code to gather session info for kernel mounts after mounting is successful. This is a necessity since network address of session is needed to check if it is blocked by Ceph cluster. Fixes: https://tracker.ceph.com/issues/56476 Signed-off-by: Rishabh Dave <ridave@redhat.com>

rishabh-d-dave · 2022-08-19T15:31:25Z

In last make check job flake8 had failed -

flake8 run-test: commands[0] | flake8 --select=F,E9 --exclude=venv,.tox
./tasks/cephfs/fuse_mount.py:15:1: F401 'tasks.cephfs.filesystem.Filesystem' imported but unused

Fixed now.

rishabh-d-dave · 2022-08-22T05:29:06Z

jenkins test api

rishabh-d-dave · 2022-08-22T05:29:17Z

jenkins test make check arm64

rishabh-d-dave · 2022-08-22T05:29:25Z

jenkins test windows

rishabh-d-dave · 2022-08-26T12:56:23Z

QA was successful - https://tracker.ceph.com/projects/cephfs/wiki/Main#2022-Aug-26.

The QA job that this PR fixes didn't got executed during QA run, so I ran it myself. The job ran successfully - http://pulpito.front.sepia.ceph.com/rishabh-2022-08-26_12:11:39-fs-wip-rishabh-testing-2022Aug19-distro-default-smithi/detail.

Waiting on CI now.

rishabh-d-dave · 2022-09-01T12:37:53Z

jenkins test api

rishabh-d-dave · 2022-09-01T12:38:01Z

jenkins test make check arm64

rishabh-d-dave · 2022-09-01T12:38:14Z

jenkins test windows

rishabh-d-dave · 2022-09-02T12:19:17Z

#46988 (comment)

Requested changes were added.

rishabh-d-dave requested review from a team, lxbsz and vshankar July 6, 2022 10:21

github-actions bot added the cephfs Ceph File System label Jul 6, 2022

rishabh-d-dave changed the title ~~qa/cephfs: improvements for xfstests_dev.py~~ qa/cephfs: clean up evicted client in 4-compat_client.yaml Jul 6, 2022

rishabh-d-dave added tests needs-review labels Jul 6, 2022

rishabh-d-dave mentioned this pull request Jul 6, 2022

qa/cephfs: upgrade is_mounted() in mount.py #45036

Merged

14 tasks

vshankar reviewed Jul 6, 2022

View reviewed changes

rishabh-d-dave force-pushed the qa-suites-upgraded-client branch from 49e51ba to 9ee8f27 Compare July 6, 2022 13:30

lxbsz reviewed Jul 7, 2022

View reviewed changes

rishabh-d-dave force-pushed the qa-suites-upgraded-client branch 4 times, most recently from c25d3e1 to 41938cc Compare July 7, 2022 17:41

rishabh-d-dave requested review from lxbsz and vshankar July 7, 2022 19:35

rishabh-d-dave force-pushed the qa-suites-upgraded-client branch from 84a2841 to f3857ec Compare July 7, 2022 20:05

lxbsz approved these changes Jul 8, 2022

View reviewed changes

vshankar approved these changes Jul 8, 2022

View reviewed changes

rishabh-d-dave removed the needs-review label Jul 8, 2022

rishabh-d-dave force-pushed the qa-suites-upgraded-client branch 2 times, most recently from 604c3ce to d8416c8 Compare August 17, 2022 15:26

rishabh-d-dave force-pushed the qa-suites-upgraded-client branch 2 times, most recently from f51e1ec to 79bb6a7 Compare August 18, 2022 09:05

rishabh-d-dave added 2 commits August 19, 2022 19:40

qa/suites/fs: add a note in 4-compat_client.yaml

1255356

Add a note explaining the reason behind the eviction of "client.1" during this test. Signed-off-by: Rishabh Dave <ridave@redhat.com>

qa/cephfs: add a method to check if CephFS mounts have hanged

b79853f

Signed-off-by: Rishabh Dave <ridave@redhat.com>

github-actions bot added the needs-rebase label Aug 19, 2022

rishabh-d-dave force-pushed the qa-suites-upgraded-client branch from 79bb6a7 to 737f978 Compare August 19, 2022 14:13

github-actions bot removed the needs-rebase label Aug 19, 2022

rishabh-d-dave requested a review from batrick August 19, 2022 15:28

rishabh-d-dave force-pushed the qa-suites-upgraded-client branch from 737f978 to c279b47 Compare August 19, 2022 15:30

rishabh-d-dave added the wip-rishabh-testing Rishabh's testing label label Aug 19, 2022

rishabh-d-dave removed the needs-qa label Sep 2, 2022

rishabh-d-dave merged commit d5ab255 into ceph:main Sep 2, 2022

rishabh-d-dave deleted the qa-suites-upgraded-client branch November 29, 2022 08:25

Conversation

rishabh-d-dave commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

rishabh-d-dave commented Jul 6, 2022

Uh oh!

rishabh-d-dave commented Jul 6, 2022

Uh oh!

vshankar Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

rishabh-d-dave Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

rishabh-d-dave commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lxbsz Jul 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rishabh-d-dave Jul 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lxbsz Jul 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lxbsz Jul 7, 2022

Choose a reason for hiding this comment

Uh oh!

rishabh-d-dave Jul 7, 2022

Choose a reason for hiding this comment

Uh oh!

vshankar Jul 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rishabh-d-dave Jul 7, 2022

Choose a reason for hiding this comment

Uh oh!

rishabh-d-dave commented Jul 7, 2022

Uh oh!

rishabh-d-dave commented Jul 7, 2022

Uh oh!

rishabh-d-dave commented Jul 7, 2022

Uh oh!

rishabh-d-dave commented Jul 8, 2022

Uh oh!

rishabh-d-dave commented Jul 8, 2022

Uh oh!

rishabh-d-dave commented Jul 8, 2022

Uh oh!

vshankar commented Aug 10, 2022

Uh oh!

rishabh-d-dave commented Aug 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 19, 2022

Uh oh!

rishabh-d-dave commented Aug 19, 2022

Uh oh!

rishabh-d-dave commented Aug 19, 2022

Uh oh!

rishabh-d-dave commented Aug 22, 2022

Uh oh!

rishabh-d-dave commented Aug 22, 2022

Uh oh!

rishabh-d-dave commented Aug 22, 2022

Uh oh!

rishabh-d-dave commented Aug 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rishabh-d-dave commented Sep 1, 2022

Uh oh!

rishabh-d-dave commented Sep 1, 2022

Uh oh!

rishabh-d-dave commented Jul 6, 2022 •

edited

Loading

rishabh-d-dave commented Jul 6, 2022 •

edited

Loading

lxbsz Jul 7, 2022 •

edited

Loading

rishabh-d-dave Jul 7, 2022 •

edited

Loading

lxbsz Jul 7, 2022 •

edited

Loading

vshankar Jul 7, 2022 •

edited

Loading

rishabh-d-dave commented Aug 10, 2022 •

edited

Loading

rishabh-d-dave commented Aug 26, 2022 •

edited

Loading