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

ceph_volume_client: allow volumes without namespace isolation #21808

Merged
merged 2 commits into from May 8, 2018

Conversation

ajarr
Copy link
Contributor

@ajarr ajarr commented May 3, 2018

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

Signed-off-by: Ramana Raja rraja@redhat.com

@ajarr ajarr requested a review from batrick May 3, 2018 22:11
@ajarr ajarr added cephfs Ceph File System pending-discussion labels May 3, 2018
@ajarr
Copy link
Contributor Author

ajarr commented May 3, 2018

With this patch, if you authorize an ID, client.john, 'read-write' access to a volume1, and then authorize client.john,'read' access to volume2 that belongs to the same data pool. client.john's OSD caps of data pool would be set to 'read'. The latest changes to OSD data pool caps takes effect. I think the OSD data pool caps should pick the more permissive caps, here 'read-write'.

@batrick , what do you think?

@batrick
Copy link
Member

batrick commented May 4, 2018

Use case for posterity: kubernetes-retired/external-storage#674

@batrick
Copy link
Member

batrick commented May 4, 2018

With this patch, if you authorize an ID, client.john, 'read-write' access to a volume1, and then authorize client.john,'read' access to volume2 that belongs to the same data pool. client.john's OSD caps of data pool would be set to 'read'. The latest changes to OSD data pool caps takes effect.

Ah, I think this is what you were trying to tell me in standup (sorry, my brain doesn't work that well in the morning!).

I think the OSD data pool caps should pick the more permissive caps, here 'read-write'.

Given the circumstances, I agree that this is the best option.

@batrick batrick removed their request for review May 4, 2018 00:53
@batrick
Copy link
Member

batrick commented May 4, 2018

@ajarr this will need a QA test too.

if namespace_isolated:
namespace = "{0}{1}".format(self.pool_ns_prefix, volume_path.volume_id)
log.info("create_volume: {0}, using rados namespace {1} to isolate data.".format(volume_path, namespace))
self.fs.setxattr(path, 'ceph.dir.layout.pool_namespace', namespace, 0)
Copy link

Choose a reason for hiding this comment

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

@ajarr If namespace is isolated, we set ceph.dir.layout.pool_namespace on path, which will also set ceph.dir.layout.pool (inherited from ancestor) on it. If namespace is not isolated, should we configure ceph.dir.layout.pool on volume path intentionally? I'm afraid if ancestor pool changed, we may have permission issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cofyc , I looked at kubernetes-retired/external-storage@2913195
makes sense. thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cofyc done.

@ajarr
Copy link
Contributor Author

ajarr commented May 4, 2018

With this patch, if you authorize an ID, client.john, 'read-write' access to a volume1, and then authorize client.john,'read' access to volume2 that belongs to the same data pool. client.john's OSD caps of data pool would be set to 'read'. The latest changes to OSD data pool caps takes effect. I think the OSD data pool caps should pick the more permissive caps, here 'read-write'.

Implemented this.

@ajarr
Copy link
Contributor Author

ajarr commented May 4, 2018

this will need a QA test too.

I will add that tomorrow.

@ajarr ajarr changed the title [DNM] ceph_volume_client: allow volumes without namespace isolation ceph_volume_client: allow volumes without namespace isolation May 7, 2018
@ajarr
Copy link
Contributor Author

ajarr commented May 7, 2018

this will need a QA test too.

done

@batrick
Copy link
Member

batrick commented May 7, 2018

Let me know when you're ready to have this go through another round of QA @ajarr

ajarr and others added 2 commits May 8, 2018 20:15
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
@ajarr
Copy link
Contributor Author

ajarr commented May 8, 2018

@batrick it's ready

@batrick batrick merged commit 3838674 into ceph:master May 8, 2018
batrick added a commit that referenced this pull request May 8, 2018
* refs/pull/21808/head:
	qa: ignore version in auth metadata comp
	ceph_volume_client: allow volumes without namespace isolation

Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>
@cofyc
Copy link

cofyc commented May 23, 2018

@batrick @ajarr hi, will this be cherry-picked into luminos branch?

@batrick
Copy link
Member

batrick commented May 24, 2018

@cofyc
Copy link

cofyc commented May 24, 2018

Thanks!

@ajarr ajarr deleted the wip-23695 branch June 30, 2019 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cephfs Ceph File System
Projects
None yet
3 participants