Bug 617632

Summary: Review Request: openstack-swift - OpenStack Object Storage (swift)
Product: [Fedora] Fedora Reporter: Silas Sewell <silas>
Component: Package ReviewAssignee: Ian Weller <ian>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: bloch, fedora-package-review, ian, jdarcy, jkaluza, jsmith.fedora, matt_domsch, mschmidt, notting, robyn.bergeron, silas, steve.traylen
Target Milestone: ---Flags: ian: fedora‑review+
kevin: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: openstack-swift-1.0.2-5.fc14 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-09-24 16:42:41 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On: 618909    
Bug Blocks:    

Description Silas Sewell 2010-07-23 11:29:18 EDT
Spec Url:
http://github.com/silas/rpms/raw/master/swift/swift.spec

SRPM Url:
http://github.com/downloads/silas/rpms/swift-1.0.2-1.fc13.src.rpm

Description:
OpenStack Object Storage (swift) aggregates commodity servers to work together
in clusters for reliable, redundant, and large-scale storage of static objects.

rpmlint

[silas@tidg ~]$ rpmlint /var/lib/mock/fedora-13-x86_64/result/*.rpm
8 packages and 0 specfiles checked; 0 errors, 0 warnings.

NOTE

This isn't done yet, I'm just putting it up to start getting feedback.

You'll need a higher version of eventlet than the one currently in Fedora (Fedora is 0.9.7 and swift requires 0.9.8), you can grab an updated SRPM from below.

http://github.com/downloads/silas/rpms/python-eventlet-0.9.9-1.fc13.src.rpm

TODO

 * Create swift user
 * Possibly rewrite swift-init as a collection of bash functions for better integration with Fedora init system
 * Create default etc scripts (maybe a minimal working version, not sure the best thing to do here)
 * Improve or fix any errors in man stubs
Comment 1 Silas Sewell 2010-07-28 01:14:12 EDT
SRPM:
http://github.com/downloads/silas/rpms/swift-1.0.2-2.fc14.src.rpm

Updatesd

 * Added swift user and setup basic permissions
 * Rewrote init stuff (doesn't use swift-init anymore)
 * Decided against default etc scripts as it would require bad assumptions about filesystem layout and ring setup
 * Added ticket to get eventlet updated

From my end this package is now ready for review.

rpmlint

swift.noarch: W: non-standard-uid /var/run/swift swift
...(repeats): Acceptable warning because we added swift user
8 packages and 0 specfiles checked; 0 errors, 6 warnings.
Comment 2 Silas Sewell 2010-07-28 01:34:58 EDT
SRPM:
http://github.com/downloads/silas/rpms/swift-1.0.2-3.fc14.src.rpm

Fix init return value.

rpmlint

swift.noarch: W: non-standard-uid /var/run/swift swift
...(repeats): see above
8 packages and 0 specfiles checked; 0 errors, 6 warnings.
Comment 3 Michal Schmidt 2010-07-28 09:03:31 EDT
Hello,

there is a naming collision with another software also called "swift" (http://swift.im/releases/swift-1.0beta5/ , in Fedora review in bug 618985).

We'll have to rename at least one of the "swift" packages.

Would a name OpenStack-swift or openstack-swift be acceptable for your package?
I am not familiar with OpenStack, but from a quick look at https://launchpad.net/openstack/ it seems that it encompasses more projects than just swift, as there's also OpenStack Compute (nova) - having a common prefix might be therefore useful for this reason too (e.g. a hypothetical Fedora package: OpenStack-nova).
Comment 4 Jared Smith 2010-07-28 09:36:11 EDT
I'm not a Fedora packager (yet?), but I took a look at your spec file and it looks great to me.  I didn't see anything that stood out to me from a packaging standpoint.
Comment 5 Silas Sewell 2010-07-28 10:16:19 EDT
@Michal Schmidt

I would prefer that the other package be renamed mainly because 1) the
Debian/Ubuntu package is going to be named swift
(http://bazaar.launchpad.net/~swift-core/swift/debian/files/head%3A/debian/),
2) it increases the length of the service names (swift-account =>
openstack-swift-account, swift-auth => openstack-swift-auth, etc...) and 3)
will make init/path names more inconsistent with upstream.

I'm guessing it will be less of an issue with the other package as it doesn't
look like it would be used from the cli.

@Jared Smith

I appreciate you checking it over, the more eyes on it the better.
Comment 6 Silas Sewell 2010-07-28 10:37:38 EDT
@Michal Schmidt

Nevermind, after talking to the Debian packager we've decided to prefix openstack. You're welcome to use the swift name.

Update coming soon.
Comment 7 Ian Weller 2010-07-28 11:05:05 EDT
When you've got an updated package, I'd be willing to do the review.
Comment 8 Michal Schmidt 2010-07-28 11:18:56 EDT
(In reply to comment #6)
> Nevermind, after talking to the Debian packager we've decided to prefix
> openstack.

Thanks for resolving this so quickly.


One comment about your spec file:
  Requires:         python(abi) >= 2.6
rpmbuild is supposed to detect the python(abi) Requires by itself, no need to tell it explicitly.
Comment 9 Silas Sewell 2010-07-28 12:47:35 EDT
Spec:
http://github.com/silas/rpms/raw/master/openstack-swift/openstack-swift.spec

SRPM:
http://github.com/downloads/silas/rpms/openstack-swift-1.0.2-4.fc14.src.rpm

Thanks Ian, much appreciated. I updated the package name and removed python(abi).

rpmlint

openstack-swift.noarch: W: non-standard-uid /var/run/swift swift
...(repeats): see above
8 packages and 0 specfiles checked; 0 errors, 6 warnings.
Comment 10 Jeff Darcy 2010-08-02 09:08:27 EDT
FWIW, I wrote up some instructions for installing a Swift test cluster using RHEL and KVM, available on the OpenStack wiki at http://wiki.openstack.org/RhelInstructions.  Some of it's probably superseded by the RPM, but other parts might still be useful.
Comment 11 Ian Weller 2010-08-04 15:42:37 EDT
[  OK  ] specfiles match:
  4ee0f99434806215b64d2967eb9e4407  openstack-swift.spec
  4ee0f99434806215b64d2967eb9e4407  openstack-swift.spec.1

[  OK  ] source files match upstream:
  6937c520d5db340bae8a63944e84174f  swift-1.0.2.tar.gz
  6937c520d5db340bae8a63944e84174f  swift-1.0.2.tar.gz.1

[  OK  ] package meets naming and versioning guidelines.
[FAILED] spec is properly named, cleanly written, and uses macros consistently.
  You need to be using the new python_sitelib macro:
    https://fedoraproject.org/wiki/Packaging:Python#Macros

  I feel like using the %{name} macro for the other source files would be
  prudent. Descriptions for the subpackages should probably give a little bit of
  what they do. Neither of these are required, but it would be prudent.

  On line 129, you can use dos2unix instead of sed to fix the
  end-of-line-encoding warning (and I think that would be preferred).

[  OK  ] dist tag is present.
[  OK  ] build root is correct.
[  OK  ] license field matches the actual license.
[  OK  ] license is open source-compatible.
[  OK  ] license text included in package.
[  OK  ] latest version is being packaged.
[  OK  ] BuildRequires are proper.
[  OK  ] %clean is present. 
[  OK  ] package builds in mock.
  You may have some issues with the Python 2.7 dependencies in F14/F15 when you
  eventually build this.

[  OK  ] package installs properly.
[  OK  ] rpmlint is silent.
  openstack-swift.noarch: W: non-standard-uid /var/run/swift swift
  openstack-swift-account.noarch: W: non-standard-uid /var/run/swift/account-server swift
  openstack-swift-auth.noarch: W: non-standard-uid /var/run/swift/auth-server swift
  openstack-swift-container.noarch: W: non-standard-uid /var/run/swift/container-server swift
  openstack-swift-object.noarch: W: non-standard-uid /var/run/swift/object-server swift
  openstack-swift-proxy.noarch: W: non-standard-uid /var/run/swift/proxy-server swift

  [ianweller@hovercraft REVIEW]$ rpmlint -I non-standard-uid
  non-standard-uid:
  A file in this package is owned by a non standard user. Standard users are:
  adm, amanda, apache, arpwatch, avahi, beagleindex, bin, clamav, condor, cyrus,
  daemon, dbus, desktop, distcache, dovecot, exim, fax, frontpage, ftp, games,
  gdm, gopher, haldaemon, halt, hsqldb, ident, jonas, ldap, lp, mail, mailman,
  mailnull, majordomo, mysql, named, netdump, news, nfsnobody, nobody, nocpulse,
  nscd, nslcd, ntp, nut, operator, oprofile, ovirt, pegasus, piranha, pkiuser,
  polkituser, postfix, postgres, prelude-manager, privoxy, puppet, pvm, qemu,
  quagga, radiusd, radvd, root, rpc, rpcuser, rpm, sabayon, saned, shutdown,
  smmsp, snortd, squid, sshd, sync, tcpdump, tomcat, tss, uucp, vcsa, vdsm,
  webalizer, wnn, xfs.

  This warning is ignorable.

[  OK  ] final provides and requires are sane
[FAILED] %check is present and all tests pass:
  This is on line 156:
    # Remove tests
    rm -fr %{buildroot}/%{python_sitelib}/test

  Are these tests that can be run in %check?

[  OK  ] owns the directories it creates. 
[  OK  ] doesn't own any directories it shouldn't.
[  OK  ] no duplicates in %files.
[  OK  ] file permissions are appropriate.
[  OK  ] scriptlets match those on ScriptletSnippets page.
  Line 180: I would have it say "OpenStack Swift Daemons".

  For the condrestart lines, I would assume that that's OK for this package, but
  I would also want to check explicitly with upstream to see if that makes
  sense.

[  OK  ] %docs are not necessary for the proper functioning of the package.

I OK'd the dependency stuff knowing that this bug was blocking on an update to python-eventlet, which is happening.
Comment 12 Silas Sewell 2010-08-08 21:11:45 EDT
(In reply to comment #11)

Sorry for the delay, I've been in the process of moving.

> [  OK  ] specfiles match:
>   4ee0f99434806215b64d2967eb9e4407  openstack-swift.spec
>   4ee0f99434806215b64d2967eb9e4407  openstack-swift.spec.1
> 
> [  OK  ] source files match upstream:
>   6937c520d5db340bae8a63944e84174f  swift-1.0.2.tar.gz
>   6937c520d5db340bae8a63944e84174f  swift-1.0.2.tar.gz.1
> 
> [  OK  ] package meets naming and versioning guidelines.
> [FAILED] spec is properly named, cleanly written, and uses macros consistently.
>   You need to be using the new python_sitelib macro:
>     https://fedoraproject.org/wiki/Packaging:Python#Macros

Fixed

>   I feel like using the %{name} macro for the other source files would be
>   prudent. Descriptions for the subpackages should probably give a little bit
> of
>   what they do. Neither of these are required, but it would be prudent.

Fixed %{name} issue. I'll work on better descriptions and update later.

>   On line 129, you can use dos2unix instead of sed to fix the
>   end-of-line-encoding warning (and I think that would be preferred).

Fixed

> [  OK  ] dist tag is present.
> [  OK  ] build root is correct.
> [  OK  ] license field matches the actual license.
> [  OK  ] license is open source-compatible.
> [  OK  ] license text included in package.
> [  OK  ] latest version is being packaged.
> [  OK  ] BuildRequires are proper.
> [  OK  ] %clean is present. 
> [  OK  ] package builds in mock.
>   You may have some issues with the Python 2.7 dependencies in F14/F15 when you
>   eventually build this.

Noted

> [  OK  ] package installs properly.
> [  OK  ] rpmlint is silent.
>   openstack-swift.noarch: W: non-standard-uid /var/run/swift swift
>   openstack-swift-account.noarch: W: non-standard-uid
> /var/run/swift/account-server swift
>   openstack-swift-auth.noarch: W: non-standard-uid /var/run/swift/auth-server
> swift
>   openstack-swift-container.noarch: W: non-standard-uid
> /var/run/swift/container-server swift
>   openstack-swift-object.noarch: W: non-standard-uid
> /var/run/swift/object-server swift
>   openstack-swift-proxy.noarch: W: non-standard-uid /var/run/swift/proxy-server
> swift
> 
>   [ianweller@hovercraft REVIEW]$ rpmlint -I non-standard-uid
>   non-standard-uid:
>   A file in this package is owned by a non standard user. Standard users are:
>   adm, amanda, apache, arpwatch, avahi, beagleindex, bin, clamav, condor,
> cyrus,
>   daemon, dbus, desktop, distcache, dovecot, exim, fax, frontpage, ftp, games,
>   gdm, gopher, haldaemon, halt, hsqldb, ident, jonas, ldap, lp, mail, mailman,
>   mailnull, majordomo, mysql, named, netdump, news, nfsnobody, nobody,
> nocpulse,
>   nscd, nslcd, ntp, nut, operator, oprofile, ovirt, pegasus, piranha, pkiuser,
>   polkituser, postfix, postgres, prelude-manager, privoxy, puppet, pvm, qemu,
>   quagga, radiusd, radvd, root, rpc, rpcuser, rpm, sabayon, saned, shutdown,
>   smmsp, snortd, squid, sshd, sync, tcpdump, tomcat, tss, uucp, vcsa, vdsm,
>   webalizer, wnn, xfs.
> 
>   This warning is ignorable.
> 
> [  OK  ] final provides and requires are sane
> [FAILED] %check is present and all tests pass:
>   This is on line 156:
>     # Remove tests
>     rm -fr %{buildroot}/%{python_sitelib}/test
> 
>   Are these tests that can be run in %check?

Unfortuntely no, it tries to do various socket operations which fail in mock.

> [  OK  ] owns the directories it creates. 
> [  OK  ] doesn't own any directories it shouldn't.
> [  OK  ] no duplicates in %files.
> [  OK  ] file permissions are appropriate.
> [  OK  ] scriptlets match those on ScriptletSnippets page.
>   Line 180: I would have it say "OpenStack Swift Daemons".

Fixed

>   For the condrestart lines, I would assume that that's OK for this package,
> but
>   I would also want to check explicitly with upstream to see if that makes
>   sense.

Sorry, can you elaborate on this a little more, I don't understand what I need to clarify upstream.

> [  OK  ] %docs are not necessary for the proper functioning of the package.
> 
> I OK'd the dependency stuff knowing that this bug was blocking on an update to
> python-eventlet, which is happening.    

SRPM: http://github.com/downloads/silas/rpms/openstack-swift-1.0.2-5.fc13.src.rpm

rpmlint

openstack-swift.noarch: W: non-standard-uid /var/run/swift swift
...(repeats): see above
8 packages and 0 specfiles checked; 0 errors, 6 warnings.
Comment 13 Ian Weller 2010-09-02 16:54:21 EDT
All outstanding issues have been resolved.

(In reply to comment #12)
> (In reply to comment #11)
> >   For the condrestart lines, I would assume that that's OK for this package,
> > but
> >   I would also want to check explicitly with upstream to see if that makes
> >   sense.
> 
> Sorry, can you elaborate on this a little more, I don't understand what I need
> to clarify upstream.

The guidelines state (at https://fedoraproject.org/wiki/Packaging:SysVInitScript#Initscripts_in_spec_file_scriptlets):

"if [ "$1" -ge "1" ] checks that this is an upgrade of the package. If so, restart the service if it's running. (This may not be appropriate for all services.)"

I just wanted to make sure that condrestart was appropriate for this service. It's not a blocker so you can deal with this later.

---------------------------------------------------------
 This package (openstack-swift) is APPROVED by ianweller
---------------------------------------------------------

(Sorry for the hilariously long delay, getting used to scheduling things at college has been... interesting.) :)
Comment 14 Silas Sewell 2010-09-04 17:04:49 EDT
New Package CVS Request
=======================
Package Name: openstack-swift
Short Description: OpenStack Object Storage
Owners: silas
Branches: F-13 F-14 EL-6
InitialCC:
Comment 15 Kevin Fenzi 2010-09-05 13:46:38 EDT
Git done (by process-git-requests).
Comment 16 Fedora Update System 2010-09-07 22:54:26 EDT
openstack-swift-1.0.2-5.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/openstack-swift-1.0.2-5.fc14
Comment 17 Fedora Update System 2010-09-07 22:55:03 EDT
openstack-swift-1.0.2-5.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/openstack-swift-1.0.2-5.fc13
Comment 18 Fedora Update System 2010-09-08 14:03:21 EDT
openstack-swift-1.0.2-5.fc14 has been pushed to the Fedora 14 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update openstack-swift'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/openstack-swift-1.0.2-5.fc14
Comment 19 Fedora Update System 2010-09-24 16:42:36 EDT
openstack-swift-1.0.2-5.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 20 Fedora Update System 2010-09-25 01:41:18 EDT
openstack-swift-1.0.2-5.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 21 Steve Traylen 2011-03-25 18:42:08 EDT
Hi Silas,

 Will you push the EPEL6 build you made?

Steve.
Comment 22 Matt Domsch 2011-08-29 12:44:14 EDT
Silas, are you interested in maintaining swift in EPEL6 please?  If not, mind if someone else does?