-
Notifications
You must be signed in to change notification settings - Fork 90
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
fs/volume_client: exercise the configurable prefix and ns_prefix. #975
Conversation
@@ -196,7 +206,9 @@ def test_idempotency(self): | |||
guest_entity = "guest" | |||
group_id = "grpid" | |||
volume_id = "volid" | |||
self._volume_client_python(self.mount_b, dedent(""" | |||
self._volume_client_python(self.mount_b, | |||
volume_prefix, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
volume_prefix not defined here, or later in test_data_isolated (it was defined in test_lifecycle)
7bd9a99
to
e678b43
Compare
Done |
@@ -23,7 +23,7 @@ def _volume_client_python(self, client, script): | |||
log = logging.getLogger("ceph_volume_client") | |||
log.addHandler(logging.StreamHandler()) | |||
log.setLevel(logging.DEBUG) | |||
vc = CephFSVolumeClient("manila", "{conf_path}", "ceph") | |||
vc = CephFSVolumeClient("manila", "{conf_path}", "ceph", prefix, ns_prefix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefix and ns_prefix aren't getting passed into this format string
@jcsp ,fixed. It's a bit ugly , we will pass None into the constructor explicitly..but I don't have good idea not passing anything when prefix not provided. |
@@ -14,7 +14,7 @@ class TestVolumeClient(CephFSTestCase): | |||
# the VolumeClient, one for mounting the created shares | |||
CLIENTS_REQUIRED = 3 | |||
|
|||
def _volume_client_python(self, client, script): | |||
def _volume_client_python(self, client, script, prefix=None, ns_prefix=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion , maybe change 'prefix' to 'vol_prefix' for clarity?
I think it'd be good to check if namespace_prefix and vol_prefix are set to default values if they are not explicitly passed. |
And also maybe mention the tracker issue in the commit message, |
Depends on ceph/ceph#8771 |
@ajarr ,make sense, will add a test. |
ec53f34
to
5654048
Compare
@ajarr ,would you mind take another look? |
guest_entity = "guest" | ||
DEFAULT_VOL_PREFIX = "/volumes" | ||
DEFAULT_NS_PREFIX = "fsvolumens_" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, you'd need to set up auth credentials for the client 'mount_b' such that it's a handle for driving the volumeclient.
Please add the following lines,
self.mount_b.umount_wait() self._configure_vc_auth(self.mount_b, "manila")
@ajarr Hmm, thanks, fixed |
NS_PREFIX = "fsvolumens_" | ||
namespace = "{0}{1}".format(NS_PREFIX, volume_id) | ||
ns_in_attr = self.mount_a.getfattr(os.path.join("volumes", group_id, volume_id), "ceph.dir.layout.pool_namespace") | ||
pool_name = self.mount_a.getfattr(os.path.join(volume_prefix, group_id, volume_id), "ceph.dir.layout.pool") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/volume_prefix/"myprefix"/
Also ensure namespace_prefix and vol_prefix are set to default values if they are not explicitly passed. Fixes #15417 Signed-off-by: Xiaoxi Chen <xiaoxchen@ebay.com>
The tests passed in my local setup. The code looks good to me. |
Signed-off-by: Xiaoxi Chen xiaoxchen@ebay.com