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: Turn off fcgi as a frontend #15070

Merged
merged 1 commit into from May 24, 2017

Conversation

Projects
None yet
7 participants
@tserlin
Contributor

tserlin commented May 12, 2017

Addresses:
Red Hat BZ 1343189
and
http://tracker.ceph.com/issues/16784

Signed-off-by: Thomas Serlin tserlin@redhat.com

RGW: Turn off fcgi as a frontend
Signed-off-by: Thomas Serlin <tserlin@redhat.com>

@tserlin tserlin requested review from mattbenjamin and yehudasa May 12, 2017

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 12, 2017

with this change, does any package depend on fcgi?

rpm -qp --requires file.rpm | grep fcgi
@tserlin

This comment has been minimized.

Contributor

tserlin commented May 12, 2017

Ken,

Here's the build in shaman for CentOS 7: https://4.chacra.ceph.com/r/ceph/wip-turn-off-fcgi-tserlin/2643fe88b27d0162a1b49f13ab5a0939b0a916b2/centos/7/flavors/default/x86_64/

I went through all (except the debuginfo package) and ran "rpm -qp --requires" on them, and none returned anything about fcgi.

@ktdreyer

thanks, that's great

@tchaikov

tchaikov requested changes May 15, 2017 edited

@tserlin my 2¢:

maybe, instead, we can switch the default backend to something other than "fcgi", for example, "beast"?

@cbodley what do you think?

@cbodley

This comment has been minimized.

Contributor

cbodley commented May 15, 2017

@tchaikov thanks for pointing out the rgw defaults, we should definitely change those to civetweb - and i think this pr would be a good place to address that

i would prefer to document the fastcgi frontend as deprecated for at least one release before removing it entirely. in the meantime, we should keep building and testing it in teuthology (despite the test failures it causes) to prevent new regressions

as long as the conditional compilation works correctly, upstream builds can keep it ON by default, and downstream builds can turn it OFF manually. does that make everyone happy?

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 15, 2017

does that make everyone happy?

I would really rather this be disabled upstream. Every point where we diverge downstream is an additional support burden.

There are unanswered security questions about fcgi. See https://bugs.launchpad.net/ubuntu/+source/libfcgi/+bug/933417/comments/5 for example. It's time to move on.

@cbodley

This comment has been minimized.

Contributor

cbodley commented May 15, 2017

It's time to move on.

this is intended to land before luminous? i'm all for removing it, as long as there's consensus on upstream mailing lists. i didn't see that from @yehudasa's recent RGW: removal of support for fastcgi thread. if we're going to cite security concerns for removing instead of deprecating, that decision should be announced asap

@cbodley

This comment has been minimized.

Contributor

cbodley commented May 15, 2017

@ktdreyer thanks for following up on the list. no more objections from me

p.s. i made the changes to rgw's frontend defaults suggested by @tchaikov at #15098

@tserlin tserlin changed the title from DNM - rgw: Turn off fcgi as a frontend to rgw: Turn off fcgi as a frontend May 18, 2017

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 19, 2017

@tchaikov Does #15098 address your review comments?

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 19, 2017

SUSE is another downstream eager to remove the FastCGI dependency from the packaging.

I guess this change will cause lots of teuthology failures in the rgw suite? I am willing to work on modifying the rgw tests so they don't do FastCGI-specific things like start apache etc.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 19, 2017

@smithfarm partially. the "code rot" concern still stands.

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 19, 2017

how can we make sure that fcgi is still functional for people who want to stick with it

From my point of view this is a non-goal, and we should not contribute to an illusion that fcgi is maintained.

@liewegas

This comment has been minimized.

Member

liewegas commented May 19, 2017

fwiw i'm all for ripping out fcgi support entirely!

@cbodley

This comment has been minimized.

Contributor

cbodley commented May 19, 2017

I guess this change will cause lots of teuthology failures in the rgw suite? I am willing to work on modifying the rgw tests so they don't do FastCGI-specific things like start apache etc.

@smithfarm I have some rgw suite cleanup work already in progress, I can take care of this (edit: #15184)

@cbodley

This comment has been minimized.

Contributor

cbodley commented May 24, 2017

@tchaikov @mattbenjamin @yehudasa any objections to merging this?

@tchaikov

if we follow this with a PR dropping fcgi support completely, that would be great!

@mattbenjamin mattbenjamin merged commit d06d134 into master May 24, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@ktdreyer ktdreyer deleted the wip-turn-off-fcgi-tserlin branch May 24, 2017

smithfarm added a commit to smithfarm/ceph that referenced this pull request May 25, 2017

build/ops: drop libfcgi build dependency
Follow-up on ceph#15070 and ceph#15098

Signed-off-by: Nathan Cutler <ncutler@suse.com>

smithfarm added a commit to SUSE/ceph that referenced this pull request May 25, 2017

build/ops: drop libfcgi build dependency
Follow-up on ceph#15070 and ceph#15098

Signed-off-by: Nathan Cutler <ncutler@suse.com>
(cherry picked from commit 69e8bee)

smithfarm added a commit to SUSE/ceph that referenced this pull request May 27, 2017

build/ops: drop libfcgi build dependency
Follow-up on ceph#15070 and ceph#15098

Signed-off-by: Nathan Cutler <ncutler@suse.com>
(cherry picked from commit 69e8bee)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment