Project

General

Profile

Bug #42317

mimic: incomplete backport LibCephFS.mount in cephfs.pyx needs it's method signature updated

Added by Rishabh Dave 9 months ago. Updated 9 months ago.

Status:
Resolved
Priority:
Urgent
Assignee:
Category:
-
Target version:
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(FS):
VolumeClient
Labels (FS):
Pull request ID:
Crash signature:

Description

I came across this issue while trying to reproduce the issue here.

On mimic branch ceph_volume_client.py's CephFSVolumeClient.connect() passes an arg to mount (see [1]) -

            log.debug("Premount eviction of {0} completes".format(premount_evict))
        log.debug("CephFS mounting...")
        self.fs.mount(filesystem_name=to_bytes(self.fs_name))

But cephfs.pyx's `LibCephFS.mount` doesn't accept any arg besides `self` (see [2]) -

 def mount(self):

This is due to an incomplete backport - https://github.com/ceph/ceph/commit/1ab18fcf056ef8e4fcb9ed27e64ee4f8a75866dc

How to reproduce

On a latest mimic cluster setup by vstart.sh, run test_volclient.py (attached) to see the error and the traceback.
Running any test from test_volume_client.TestVolumeClient would also work but the real error message would be hidden behind CommandFailedError.

test_volclient.py View (501 Bytes) Rishabh Dave, 10/15/2019 05:53 AM


Related issues

Related to fs - Backport #40896: mimic: ceph_volume_client: fs_name must be converted to string before using it Rejected

History

#1 Updated by Nathan Cutler 9 months ago

Indeed, https://github.com/ceph/ceph/commit/153a8cb025da7a500356d65f80792c1b51de71fe was merged for nautilus and I can't find any mention of it in mimic, so I assume it was never backported.

#2 Updated by Nathan Cutler 9 months ago

@Rishabh Is it enough to cherry-pick 153a8cb025da7a500356d65f80792c1b51de71fe to mimic? I see it applies cleanly.

#3 Updated by Nathan Cutler 9 months ago

  • Related to Backport #40896: mimic: ceph_volume_client: fs_name must be converted to string before using it added

#4 Updated by Nathan Cutler 9 months ago

  • Priority changed from Normal to Urgent

#5 Updated by Patrick Donnelly 9 months ago

  • Target version set to v13.2.7
  • Start date deleted (10/15/2019)
  • Source set to Development

#6 Updated by Rishabh Dave 9 months ago

@Nathan

@Rishabh Is it enough to cherry-pick 153a8cb025da7a500356d65f80792c1b51de71fe to mimic? I see it applies cleanly.

Just cherry pick this commit and not the entire PR, right?

#7 Updated by Nathan Cutler 9 months ago

Rishabh Dave wrote:

Just cherry pick this commit and not the entire PR, right?

Glancing at the code, I don't see any reason why cherry-picking just this one commit and not the entire PR "libcephfs: enable convenient use of multiple filesystems" [1] would cause any problems (when the filesystem argument is empty, it should default to the value of "client_mds_namespace"), but better ask Patrick to be sure.

[1] https://github.com/ceph/ceph/pull/23530

#8 Updated by Patrick Donnelly 9 months ago

!

https://github.com/ceph/ceph/pull/30238

should never have been approved :(. The test failure

http://pulpito.ceph.com/yuriw-2019-10-08_01:35:28-fs-wip-yuri3-testing-2019-10-07-1952-mimic-testing-basic-smithi/4367927/

clearly shows the problem.

#30238 should be reverted. The fix is not needed in Mimic.

#10 Updated by Rishabh Dave 9 months ago

  • Assignee changed from Rishabh Dave to Nathan Cutler

#11 Updated by Nathan Cutler 9 months ago

  • Status changed from New to Resolved

Also available in: Atom PDF