Skip to content

rgw: metadata refactoring#29118

Merged
cbodley merged 118 commits intoceph:masterfrom
cbodley:wip-rgw-metadata-servicification
Aug 13, 2019
Merged

rgw: metadata refactoring#29118
cbodley merged 118 commits intoceph:masterfrom
cbodley:wip-rgw-metadata-servicification

Conversation

@cbodley
Copy link
Contributor

@cbodley cbodley commented Jul 18, 2019

rebase of #28679 with some additional cleanup

@cbodley cbodley requested review from adamemerson and dang July 18, 2019 19:25
@cbodley cbodley force-pushed the wip-rgw-metadata-servicification branch 4 times, most recently from 595c080 to 8469c77 Compare July 19, 2019 20:39
@cbodley
Copy link
Contributor Author

cbodley commented Jul 26, 2019

cc @yehudasa

@cbodley
Copy link
Contributor Author

cbodley commented Jul 29, 2019

some radosgw-admin failures, and lots of valgrind issues to go through: http://pulpito.ceph.com/cbodley-2019-07-26_18:05:13-rgw-wip-cbodley-testing-distro-basic-smithi/

@cbodley
Copy link
Contributor Author

cbodley commented Jul 29, 2019

from qa/tasks/radosgw_admin.py, the creation of user2=fud fails during # create a second user to link the bucket to, because it finds an access_key2 object pointing at user1=foo. this key is added to user1 in # TESTCASE 'add-keys','key','create','w/valid info','succeeds' and removed in # TESTCASE 'rm-key','key','rm','newly added key','succeeds, key is removed', but it looks like the rados object doesn't get deleted

cbodley
cbodley commented Jul 29, 2019
View reviewed changes
yehudasa added 17 commits July 29, 2019 15:20
@yehudasa @cbodley
rgw: header cleanup: remove code from rgw_rados.h
c8e5d5e 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: more headers cleanups
9dfe243 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: object expirer: move code around
744ded1 
move code out of RGWRados, refactor a bit to use rados svc.

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: more headers cleanup
268caad 
Dependency reduction

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: move manifest code around, initial tier separation
80ff837 
cascading changes, and minor improvements.

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: svc/mfa: new service for mfa functionality
91f1278 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: switch to using svc.mfa
16a4051 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: svc_cls: replace svc_mfa with a more generic svc_cls
56ee875 
Consolidate objclass util services.

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: move rgw_compression_info_from_attrset() to a better home
c063b59 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: move admin socket cache code into svc_sys_obj_cache
8c64f12 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: no need to pass in RGWRados to rgw_get_system_obj()
e689d5d 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: svc_bucket: initial work
9369650 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: use bucket service for get_bucket_info
01c08bd 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: header cleanup
6a0f387 
move implementation out of header.

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: move helper code out of rgw_rados.cc
ba2706e 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: svc bucket: set bucket instance info
488583a 
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa @cbodley
rgw: initial meta manager refactoring
830cc73 
I didn't intend for it to be such a big commit, and it's not
even compiling yet. This changes the structure of how
the metadata manager and handlers work.
The idea is to be able to relatively easily hook in different
meta backends (or same backends with different handling -- such
as the otp).
Added new services for meta, meta backend, and meta backend sysobj
implementation.
The meta backend service is responsible for the final data storage,
and updating the meta log (log might be split later on, but at the
moment it keeping it together for simplicity).
The handlers themselves are the ones responsible for reading or
modifying the metadata. This means that they need to call the
meta backend service instead of calling the utility functions.
The utility functions need to call the handlers, and not the other
way around. Handlers can have utility methods to assist.

Left to do: get everything actually compiling and implemented. The
structure is there, now need to fill in the gaps.

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@cbodley
Copy link
Contributor Author

cbodley commented Jul 29, 2019

the valgrind leak was because RGWSI_MetaBackend_Handler::Op doesn't have a virtual destructor, so the derived Op_ManagedCtx destructor isn't called to delete the allocated Context. since Op doesn't have any other virtual methods, i decided against making its destructor virtual. instead, i pushed a commit that uses the public Op_ManagedCtx directly

@cbodley
Copy link
Contributor Author

cbodley commented Jul 31, 2019

next failure is in radosgw_admin_rest on test case # try creating an object with the first user which should cause an error after 'bucket link' with another user:

   File "qa/tasks/radosgw_admin_rest.py", line 520, in task
    assert denied

@cbodley
rgw: RGWBucket::link clears acl buffer before re-encoding
dd0e60d 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
cbodley
cbodley commented Jul 31, 2019
View reviewed changes
src/rgw/rgw_bucket.cc Outdated

// ...and encode the acl
attrs[RGW_ATTR_ACL].clear();
policy.encode(attrs[RGW_ATTR_ACL]);
Copy link
Contributor Author

@cbodley cbodley Jul 31, 2019

Choose a reason for hiding this comment

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

this was missing the call to clear(), so re-encoding the policy was appending to the old one and not taking effect

Copy link
Member

@yehudasa yehudasa Aug 1, 2019

Choose a reason for hiding this comment

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

@cbodley looks good, but maybe can do it like this:
auto& attr = atts[RGW_ATTR_ACL]; attr.clear(); policy.encode(attr);

to avoid a double lookup?

@cbodley cbodley mentioned this pull request Aug 1, 2019
Merged
3 tasks
@cbodley
Copy link
Contributor Author

cbodley commented Aug 2, 2019

http://pulpito.ceph.com/cbodley-2019-08-01_13:07:49-rgw-wip-cbodley-testing-distro-basic-smithi/ looks like a pass. i'll start rebasing this on top of #28813 so we can merge both

@cbodley cbodley mentioned this pull request Aug 7, 2019
Merged
@cbodley cbodley force-pushed the wip-rgw-metadata-servicification branch from dd0e60d to 8f912a6 Compare August 9, 2019 15:14
@cbodley
Copy link
Contributor Author

cbodley commented Aug 9, 2019

i squashed everything into a single commit (because individual commits did not compile on their own), and rebased on top of #29540

@yehudasa
Copy link
Member

yehudasa commented Aug 9, 2019

i squashed everything into a single commit (because individual commits did not compile on their own), and rebased on top of #29540

@cbodley Then we'll lose all history that I worked really hard to retain when rebasing it the first time. How about doing a git merge instead of rebase?

@mattbenjamin
Copy link
Contributor

mattbenjamin commented Aug 9, 2019

@yehudasa how much of the history is history just of the evolution of metadata refactor in your checkout?

@yehudasa
Copy link
Member

yehudasa commented Aug 9, 2019

@mattbenjamin I can't really say. History can have value, even if not immediately clear. Squashing everything and applying a single commit is technically the same as running git merge. Is there a reason to lose all the history?

@cbodley cbodley force-pushed the wip-rgw-metadata-servicification branch from 8f912a6 to 3e7dc95 Compare August 9, 2019 19:25
cbodley
cbodley commented Aug 12, 2019
View reviewed changes
@cbodley
rgw: RGWUserCtl::PutParams::attrs defaults to nullptr
2bf1031 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley cbodley force-pushed the wip-rgw-metadata-servicification branch from f5d6064 to 96cbc98 Compare August 12, 2019 19:59
@cbodley
Merge branch 'wip-rgw-metadata-servicification'
75e1ec8 
 Conflicts:
	src/rgw/rgw_auth.cc
	src/rgw/rgw_auth_registry.h
	src/rgw/rgw_auth_s3.h
	src/rgw/rgw_bucket.cc
	src/rgw/rgw_bucket.h
	src/rgw/rgw_data_sync.h
	src/rgw/rgw_frontend.h
	src/rgw/rgw_log.h
	src/rgw/rgw_main.cc
	src/rgw/rgw_rados.cc
	src/rgw/rgw_rados.h
	src/rgw/rgw_rest_s3.h
	src/rgw/rgw_rest_sts.h
	src/rgw/rgw_swift_auth.h
	src/rgw/rgw_user.cc
	src/rgw/rgw_user.h
	src/rgw/services/svc_sys_obj_core.h
@cbodley cbodley force-pushed the wip-rgw-metadata-servicification branch from 96cbc98 to 75e1ec8 Compare August 13, 2019 14:25
@cbodley
Copy link
Contributor Author

cbodley commented Aug 13, 2019

mostly passed in http://pulpito.ceph.com/cbodley-2019-08-13_12:16:43-rgw-wip-cbodley-testing-distro-basic-smithi/

some 403 Forbidden failures from radosgw-admin tests because i missed a couple pieces of 'user rename' in the last rebase. i added those back and pushed the branch after verifying the fix locally. i'd like to merge this as is to unblock other work; any objections @yehudasa @dang?

@yehudasa
Copy link
Member

yehudasa commented Aug 13, 2019

@cbodley no objections

@cbodley cbodley merged commit a3039be into ceph:master Aug 13, 2019
@cbodley cbodley deleted the wip-rgw-metadata-servicification branch August 13, 2019 16:57
This was referenced Aug 20, 2019
Merged
Merged
@votdev
Copy link
Member

votdev commented Aug 20, 2019

Please run the Dashboard QA test suites before next time to ensure RGW changes do not break the Dashboard or inform the Dashboard team and open a tracker issue with the changes that needs to be adapted in the Dashboard.

@cbodley
Copy link
Contributor Author

cbodley commented Aug 20, 2019

@votdev i don't think it's reasonable to run every rgw pr through an extra teuthology suite. i'd prefer to move the dashboard's functional test coverage of rgw admin apis into qa/tasks/radosgw_admin_rest.py which runs as part of the rgw suite

cbodley
cbodley commented Aug 22, 2019
View reviewed changes
src/rgw/rgw_bucket.cc
return -EINVAL;
}
rgw_bucket old_bucket = bucket;
bucket.tenant = tenant;
Copy link
Contributor Author

@cbodley cbodley Aug 22, 2019

Choose a reason for hiding this comment

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

oops, somehow this change snuck into the rebase

@cbodley cbodley mentioned this pull request Aug 22, 2019
Merged
3 tasks
@cbodley cbodley mentioned this pull request Dec 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yehudasa yehudasa yehudasa left review comments

@dang dang dang approved these changes

@adamemerson adamemerson Awaiting requested review from adamemerson

Assignees

No one assigned

Labels

DNM rgw wip-cbodley-testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@cbodley @yehudasa @mattbenjamin @votdev @dang