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

rgw: add suport for Swift-at-root dependent features of Swift API #10280

Merged
merged 6 commits into from Aug 29, 2016

Conversation

rzarzynski
Copy link
Contributor

This pull request brings:

  • a temporary, provisional fix for the Swift API-at-root issue that is small enough to be backported to Jewel.
  • implementation of several parts of Swift API that has to be placed in the root of URL hierarchy (/crossdomain.xml, /healthcheck and /info by @pritha-srivastava adapted to this solution).

Placing Swift-at root-definitely needs an ultimate solution as the temporary, proposed one exhibits severe side effect: inability to deploy RadosGW in multi-site configuration. This is because some S3 operations working in system_request mode are necessary for that to operate.

Derogates: #9900, #10069, #10213 and - only if Pritha agrees - #9582.
CC: @pritha-srivastava, @yehudasa, @cbodley.

Tempest and S3 haven't found any regression here. The test for/info passes as well though it's really simple - check whether response code is 200 OK and body is a properly formatted JSON.

rzarzynski and others added 6 commits July 14, 2016 15:46
This patch fixes to the support for placing the Swift API in the root
of URL hierarchy. Unfortunately, the whole concept exhibits a severe side
effect: inability to deploy RadosGW in multi-site configuration.

The sole reason behind this fix is the fact we claimed in documentation
that the feature is available.

Fixes: http://tracker.ceph.com/issues/16673
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
Implementation for swift /info API.

Signed-off-by: Pritha Srivastava <prsrivas@redhat.com>
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>

Conflicts:
	src/rgw/rgw_common.h
	src/rgw/rgw_main.cc
	src/rgw/rgw_rest_swift.cc
	src/rgw/rgw_rest_swift.h
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
@rzarzynski rzarzynski force-pushed the wip-rgw-swift-at-root-features branch from a2c1b4c to dcfd7a9 Compare July 14, 2016 13:47
@rzarzynski
Copy link
Contributor Author

Rebased due to a merge conflict.

@cbodley
Copy link
Contributor

cbodley commented Jul 14, 2016

looks good to me

rest.register_default_mgr(set_logging(new RGWRESTMgr_S3(s3website_enabled)));
const bool s3website_enabled = apis_map.count("s3website") > 0;
// Swift API entrypoint could placed in the root instead of S3
const bool swift_at_root = g_conf->rgw_swift_url_prefix == "/";
Copy link
Contributor

Choose a reason for hiding this comment

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

A question - do we have to take into account if 'swift' api is enabled, while determining whether swift entry point is at root or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. The purpose of the swift_at_root is to indicate whether someone requested putting Swift API at root. It could be placed there only if Swift API is enabled and S3 (and S3 website) are turned off.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, i asked this question because what if someone changes the default url prefix to '/' and then decides to disable the swift api altogether, then we could allow the s3/ s3 website to be enabled. This may be a case which may be impossible in a real scenario :)

@pritha-srivastava
Copy link
Contributor

@rzarzynski: Looks good to me too.

@rzarzynski
Copy link
Contributor Author

Tested as a part of wip-radek-testing. Teuthology runs:

The failures seem to be unrelated to the changes.

The Pritha's commit LGTM.

@ghost
Copy link

ghost commented Oct 13, 2016

@rzarzynski Hi ! Would you be willing to try a backport to jewel ? It looks non trivial :-) If you don't have time for that don't worry about it. For the record the corresponding backport issue is http://tracker.ceph.com/issues/17208

@rzarzynski
Copy link
Contributor Author

@dachary: Hi Loic! I'm putting the backport on my TODO. I think that /info would be quite desired in Jewel also because of the recent change in Tempest that assumes its availability.

@ghost
Copy link

ghost commented Oct 13, 2016

@rzarzynski thanks, much appreciated. If it's easier than it looks, just tell me and I'll give it a try :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants