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: allow beast frontend to listen on specific IP address #20000

Merged
merged 1 commit into from Jan 30, 2018

Conversation

zhouyuan
Copy link
Contributor

This patch allows the beast frontend to listen on a specific
IP address rather than *. e.g., with below configuration:

rgw frontends = beast port=1.2.3.4:8080

rgw server will listen on 1.2.3.4 only.

Signed-off-by: Yuan Zhou yuan.zhou@intel.com

Copy link
Contributor

@cbodley cbodley left a comment

Choose a reason for hiding this comment

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

very cool, thank you. i think we'd be better of adding a separate config key like "ip" or "addr" to the frontend config though, instead of extending "port" this way

it would also be good to consider whether the civetweb frontend could take advantage of this feature too

and whatever we do, we'll need to document it clearly. i don't see anything at all about rgw_frontends in https://github.com/ceph/ceph/blob/master/doc/radosgw/config-ref.rst, but that's where it would go - i can work on that separately

@zhouyuan
Copy link
Contributor Author

@cbodley yes, the civetweb frontend support this configuration already, actually I find the missing gap when doing some tests on a existing RGW cluster with civetweb frontend.

@cbodley
Copy link
Contributor

cbodley commented Jan 19, 2018

posting for my reference: http://tracker.ceph.com/issues/13670 is the doc bug for rgw_frontends

@zhouyuan
Copy link
Contributor Author

@cbodley I'm trying to add separate config key "addr" and "port "here, do you think we should change this for civetweb/fastcgi frontend also? To avoid breaking some existing cluster I guess we may should also accept the "port" configuration?

@cbodley
Copy link
Contributor

cbodley commented Jan 22, 2018

@zhouyuan thanks, i didn't realize that civetweb uses "port" for the address as well (https://github.com/ceph/civetweb/blob/wip-listen4/src/civetweb.c#L10293-L10299). let's leave that how it as, and use the new "addr" config for beast only - we'll just need to document that clearly so we don't confuse people on upgrade

@cbodley
Copy link
Contributor

cbodley commented Jan 22, 2018

thinking about this more, i actually prefer civetweb's approach here - that syntax allows you to specify multiple ports, like 80+443s, each of which could specify a bind address. if we separate the addr configuration from the port, you'd have to run multiple frontends (ie rgw_frontends = beast addr=127.0.0.1 port=8000, beast port=80) to accomplish that

port = std::stol(confs[1], nullptr, 0);
}

tcp::endpoint ep = {boost::asio::ip::address::from_string(ip_addr), port};
Copy link
Contributor

@cbodley cbodley Jan 22, 2018

Choose a reason for hiding this comment

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

according to the docs for from_string(), it's deprecated in favor of make_address() - can you use the overload that takes string_view and avoid the use of vector/string? something like:

  tcp::address addr; // default to 'any'
  boost::string_view input = port_str;
  auto colon = input.find(':');
  if (colon != input.npos) {
    boost::system::error_code ec;
    addr = boost::asio::ip::address::make_address(input.substr(0, colon), ec);
    if (ec) {
      lderr(ctx()) << "failed to parse address '" << port_str << "': " << ec.message() << dendl;
      return -ec.value();
    }
  }
  tcp::endpoint ep = {addr, port};

then eventually we can extend the frontend to listen on multiple sockets

@cbodley
Copy link
Contributor

cbodley commented Jan 22, 2018

see #20058 for first draft of frontend configuration docs

@cbodley
Copy link
Contributor

cbodley commented Jan 23, 2018

thanks, this looks good! i created a ticket to track this for backport, could you please add Fixes: http://tracker.ceph.com/issues/22778 to your commit message?

This patch allows the beast frontend to listen on a specific
IP address rather than *. e.g., with below configuration:

rgw frontends = beast port=1.2.3.4:8080

rgw server will listen on 1.2.3.4 only.

Fixes: http://tracker.ceph.com/issues/22778

Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@cbodley cbodley removed the needs-qa label Jan 30, 2018
@cbodley cbodley merged commit 9cdd2fe into ceph:master Jan 30, 2018
@cbodley
Copy link
Contributor

cbodley commented Jan 30, 2018

thanks @zhouyuan! i've built on this work to add support for multiple endpoints in #20188

@zhouyuan zhouyuan deleted the rgw_beast_listen branch January 31, 2018 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants