Bug 862052 - Backport constraints configuration support so glusterfs-swift can work with stock swift
Backport constraints configuration support so glusterfs-swift can work with s...
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: openstack-swift (Show other bugs)
18
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Derek Higgins
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-01 14:01 EDT by Peter Portante
Modified: 2013-08-07 19:16 EDT (History)
12 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-08-07 19:16:23 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Modified version of upstream patch a2ac5efaa64f57fbbe059066c6c4636dfd0715c2 (sans pep8 stuff) (21.89 KB, patch)
2012-10-01 14:04 EDT, Peter Portante
no flags Details | Diff

  None (edit)
Description Peter Portante 2012-10-01 14:01:03 EDT
Description of problem:

Upstream has a set of changes (made as of https://github.com/openstack/swift/commit/a2ac5efaa64f57fbbe059066c6c4636dfd0715c2) which allow the constraints to be modified by changing them in the swift.conf file. This quite powerful, as system administrators and systems leveraging Swift might have valid reasons to the change those constraints, but don't want to be burdened by maintaining code changes to do so.

This is request, if fulfilled, would be the foundational change to 1.4.8 that would allow Gluster a easier integration path with Swift 1.4.8, avoiding a large set of code changes in Swift for constraint value adjustments.


Version-Release number of selected component (if applicable): 1.4.8


How reproducible: Always.


Steps to Reproduce:
1. Install 1.4.8
2. See that config files do not contain constraint settings
3. See that constraint defaults are only in the code
  
Actual results: Constraints cannot be tailored without changing the code.


Expected results: Constraints can be tailored changing configuration files.


Additional info:

This also facilitates other fixes that require different values to the constraints.

See the attached patch for a working backport of the upstream patch (without all the pep8 changes) that passes all the unit tests.
Comment 1 Peter Portante 2012-10-01 14:04:50 EDT
Created attachment 619927 [details]
Modified version of upstream patch a2ac5efaa64f57fbbe059066c6c4636dfd0715c2 (sans pep8 stuff)
Comment 3 Alan Pevec 2012-10-02 03:31:20 EDT
> Version-Release number of selected component (if applicable): 1.4.8

Note that F18 has updated to OpenStack Folsom release Swift 1.7.4
Comment 4 Kaleb KEITHLEY 2012-10-08 08:55:57 EDT
glusterfs will need the same backport applied to 1.7.4 as well, as glusterfs will eventually rebase on 1.7.4.

Our goal is to eliminate swift from the glusterfs packages and just have a dependency on the openstack-swift packages.

Since this patch comes from upstream it shouldn't be too controversial to carry as a patch against the fedora packaging until openstack merges it into their release.

And maybe they can be encouraged to merge it into their release sooner, rather than later.
Comment 5 Alan Pevec 2012-10-08 09:36:01 EDT
Swift currently doesn't have any new milestones set
 https://launchpad.net/swift/grizzly
and Grizzly schedule is TBD at the OpenStack Design Summit next week, draft is at
 http://wiki.openstack.org/GrizzlyReleaseSchedule

I'm still not clear how do configurable constraints help eliminate Gluster patch
 https://github.com/gluster/glusterfs/blob/master/swift/1.4.8/swift.diff
could you give us a complete example config?
Comment 6 Peter Portante 2012-10-08 09:48:57 EDT
Much of the diffs that Gluster carries in its code base is to check for the presence of cluster and import Gluster constraints from a different module, since the swift constraints are hard coded.

Please see the following swift.diff for comparison:

https://github.com/portante/glusterfs/blob/657340541079b96643657371ee04fa967d2b5f23/swift/1.4.8/swift.diff
Comment 7 Mark McLoughlin 2012-10-08 11:52:11 EDT
Ok, we've had some further offline discussion. Here's my attempt to summarize it:

  - The original gluster integration which required an invasive swift
    patch[1] is being re-worked by Peter on his refactor-swift-3.3.0
    branch

  - The plan is for no swift patch at all to be required - gluster code
    will subclass swift code in order to do its thing. When Peter's work
    is complete, the new version of switch patch[2] should be empty

  - This re-working will only work if the contraints config added by
    swift commit a2ac5efaa is available

  - Once Peter's work is complete, we'll revisit this and figure out if
    the way gluster is using swift internals is something we can support.
    We'll at least need to consider details like whether gluster will need
    to depend on a specific rpm version and/or release of swift so that
    users won't unwittingly update swift without updating gluster

  - When we've figured out this plan, we can backport commit a2ac5efaa if
    it still makes sense

[1] - https://github.com/gluster/glusterfs/blob/144db7f/swift/1.4.8/swift.diff
[2] - https://github.com/portante/glusterfs/blob/refactor-swift-3.3.0/swift/1.4.8/swift.diff
Comment 8 Mark McLoughlin 2012-10-08 11:54:26 EDT
Oh, another detail I missed which wasn't clear to me - commit a2ac5efaa was not merged in time for the 1.7.4 release that is part of the OpenStack Folsom release
Comment 9 Mark McLoughlin 2012-10-08 11:55:18 EDT
Marking as needinfo for now - we're waiting on Peter's work being completed
Comment 10 Vijay Bellur 2012-10-13 13:36:57 EDT
CHANGE: http://review.gluster.org/4080 (Fix a small typ-o in the Swift README) merged in master by Anand Avati (avati@redhat.com)
Comment 11 Vijay Bellur 2012-10-16 17:14:49 EDT
CHANGE: http://review.gluster.org/4081 (Reduce the number of gratuitous differences in constraints.) merged in master by Anand Avati (avati@redhat.com)
Comment 12 Vijay Bellur 2012-10-16 17:15:18 EDT
CHANGE: http://review.gluster.org/4082 (Remove the test subdirectory diffs ahead of initial refactoring.) merged in master by Anand Avati (avati@redhat.com)
Comment 13 Vijay Bellur 2012-10-17 14:04:16 EDT
CHANGE: http://review.gluster.org/4093 (object-storage: Refactor code to reduce Swift diffs carried) merged in master by Anand Avati (avati@redhat.com)
Comment 14 Kaleb KEITHLEY 2012-11-16 15:35:17 EST
We finally have a PoC of GlusterFS packaging with an 'example' of the GlusterFS/UFO version of openstack-swift that is not extensively patched as previous versions were.

You can get all the bits from my fedorapeople repo, the src.rpm is at http://repos.fedorapeople.org/repos/kkeithle/glusterfs/XX-UFO/fedora-17/SRPMS/glusterfs-3.3.1-X.fc17.src.rpm and there's a repo file at http://repos.fedorapeople.org/repos/kkeithle/glusterfs/XX-UFO/fedora-XX-ufo.repo

N.B. our glusterfs-swift is patched with the same patches as the openstack-swift packages _plus_ the constraints-config backport only. As I'm sure you know, the constraints-config changes have now been accepted into grizzly.

Remember, our preference, and our goal, is to _not_ ship Swift at all in our rpms. To achive that, we need the constraints-config backport patch added to openstack-swift. We'd like it in both 1.4.8 and 1.7.4+, but we'll settle for it in 1.7.4+ only if that's all we can get.

Once we get the constraints-config backport patch added to openstack-swift, we'll drop the glusterfs-swift packages completely. Bear in mind that we would need to release UFO — for some definition of release, something a little more formal than "the source is in the git tree" I suppose — before we'd be ready for these next steps. In any event, the sooner we can get the constraints-config backport patch(es) into openstack-swift packages the better.
Comment 15 Mark McLoughlin 2012-11-17 10:28:52 EST
(In reply to comment #7)
> 
>   - Once Peter's work is complete, we'll revisit this and figure out if
>     the way gluster is using swift internals is something we can support.

I've taken a look at latest UFO's usage of swift internals.

Depending on swift internals like this is not supportable. We do not recommend you drop the swift code from your packages while UFO continues to depend on internal swift APIs.

Here's what you're doing that's not supportable from our POV:

  - Monkey patching of swift.common.constraints - i.e. that module is a
    private swift API and __check_object_creation is quite clearly a
    private implementation detail of that API

  - These methods:

      swift.account.server.AccountController._get_account_broker
      swift.container.server.ContainerController._get_container_broker

    and this variable:

      swift.obj.server.DiskFile

    and this class:

      swift.proxy.server.Application

    are again private implementation details swift.

Unless upstream have made a commitment that we're not aware of to maintaining compatibility for these APIs, then shipping a gluster package that depends on our swift package providing those APIs is not something we are willing to recommend or support.
Comment 16 Kaleb KEITHLEY 2012-12-14 14:07:34 EST
Gluster/UFO usage of swift internals is not relevant to the discussion. Wa have not and are not asking anyone to maintain compatibility of the APIs.

If swift internals change sufficiently from one release to the next — e.g. from 1.7.4-2 to 1.7.4-3, as unlikely as that would seem to be — so as to break compatibility, then the burden will be on Gluster to adapt as necessary.

In the short term we can specify the exact release of openstack-swift that we need. Longer term we would have to fix Gluster to work with the newer release. The ultimate fallback would be to return to bundling our own conflicting package of openstack-swift. Since the original complaint was that we're shipping a conflicting package, I guess I don't need to point out that that's not our prefered outcome.

But to the point of eliminating the conflicting package, applying the Constraints Config backport patch to the openstack-swift packaging is the final step that's necessary to allow for removal of the conflicting Swift bits from the glusterfs packaging.

(In case it hasn't occurred to anyone, if we stall long enough, Grizzly will ship, Gluster/UFO will have what it needs, courtesy of upstream, and we won't need the cooperation of the openstack-swift packagers to eliminate the conflict. It should go without saying that we think it'd be better eliminate the conflict sooner rather than later.)
Comment 17 Pete Zaitcev 2013-08-07 19:16:23 EDT
Fixed in F19.

Note You need to log in before you can comment on or make changes to this bug.