swift ring builder shows nasty stacktrace if builder file is empty or invalid

Bug #1370680 reported by keshava
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Undecided
keshava

Bug Description

swift-ring-builder command shows nasty stacktraces if the builder file is empty or invalid.
If, its empty file, throws EOF error. if a file is corrupted, throws unpickling error.
Also, during these cases throws an error code of 1, which is wrong since the chosen values for swift-ring-builder is

Exit codes: 0 = operation successful
            1 = operation completed with warnings
            2 = error

-- empty file--
[keshava@Kbook tmp]$ >object.builder
[keshava@Kbook tmp]$ swift-ring-builder object.builder
Traceback (most recent call last):
  File "/usr/local/bin/swift-ring-builder", line 6, in <module>
    exec(compile(open(__file__).read(), __file__, 'exec'))
  File "/opt/stack/swift/bin/swift-ring-builder", line 24, in <module>
    sys.exit(main())
  File "/opt/stack/swift/swift/cli/ringbuilder.py", line 834, in main
    builder = RingBuilder.load(builder_file)
  File "/opt/stack/swift/swift/common/ring/builder.py", line 991, in load
    builder = pickle.load(open(builder_file, 'rb'))
EOFError
[keshava@Kbook tmp]$ echo $?
1

-- a corrupted file --
[keshava@Kbook tmp]$ swift-ring-builder object.builder
Traceback (most recent call last):
  File "/usr/local/bin/swift-ring-builder", line 6, in <module>
    exec(compile(open(__file__).read(), __file__, 'exec'))
  File "/opt/stack/swift/bin/swift-ring-builder", line 24, in <module>
    sys.exit(main())
  File "/opt/stack/swift/swift/cli/ringbuilder.py", line 834, in main
    builder = RingBuilder.load(builder_file)
  File "/opt/stack/swift/swift/common/ring/builder.py", line 991, in load
    builder = pickle.load(open(builder_file, 'rb'))
cPickle.UnpicklingError: invalid load key, 'n'.
[keshava@Kbook tmp]$ echo $?
1

The error codes needs to be corrected, and also a meaningful user information should be provided if a file is empty or corrupted

keshava (kb-sankethi)
Changed in swift:
assignee: nobody → keshava (kb-sankethi)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (master)

Fix proposed to branch: master
Review: https://review.openstack.org/122225

Changed in swift:
status: New → In Progress
keshava (kb-sankethi)
Changed in swift:
status: In Progress → New
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/122225
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=38ba5790fb527967c2fcbaf094e76a73f4b94d38
Submitter: Jenkins
Branch: master

commit 38ba5790fb527967c2fcbaf094e76a73f4b94d38
Author: Keshava Bharadwaj <email address hidden>
Date: Thu Sep 18 00:45:35 2014 +0530

    Provides proper error handling on builder unpickle

    This patch provides the necessary error handling while unpickling
    a builder file. Earlier if a builder file is empty/invalid/corrupted,
    the stacktrace was shown to user with an exit code of 1. This fixes it
    to show a user-friendly message and also returns the exit code of 2,
    indicating there was a failure.

    Change-Id: I51eb24702c422299629f8053d4591dd10f5863f8
    Closes-Bug: #1370680

Changed in swift:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in swift:
milestone: none → 2.2.0-rc1
status: Fix Committed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/ec)

Fix proposed to branch: feature/ec
Review: https://review.openstack.org/126595

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (feature/ec)
Download full text (11.3 KiB)

Reviewed: https://review.openstack.org/126595
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=06800cbe446ce4c937a57b69517b55c3bba9b6e1
Submitter: Jenkins
Branch: feature/ec

commit 7528f2b22169e90fe8ddd19b7ef7d46ecff5d231
Author: Christian Schwede <email address hidden>
Date: Mon Oct 6 10:01:03 2014 +0000

    Fix minor typo

    Fixes minor typo in one method and adds missing parameter in other
    method. Only checked swift/container/reconciler.py for now.

    Change-Id: I5c648010f09b6e4b1fb0380bc97b266e680602f8

commit 94fd95ba30c72fbcb03367aaa8da407a408948d5
Author: OpenStack Proposal Bot <email address hidden>
Date: Sat Oct 4 06:07:47 2014 +0000

    Imported Translations from Transifex

    Change-Id: I31b5e6b0f2922150902e1bfa52144302ee0c7a8e

commit d6a827792619f3343af07fc2519f4253fbdc67f7
Author: John Dickinson <email address hidden>
Date: Fri Oct 3 10:17:00 2014 -0400

    updated AUTHORS and CHANGELOG for 2.2.0

    Change-Id: I6c0bc1570f6a48439de5a029a86f1b582f30f8a6

commit 5b2c27a5874c2b5b0a333e4955b03544f6a8119f
Author: Richard (Rick) Hawkins <email address hidden>
Date: Wed Oct 1 09:37:47 2014 -0400

    Fix metadata overall limits bug

    Currently metadata limits are checked on a per request basis. If
    multiple requests are sent within the per request limits, it is
    possible to exceed the overall limits. This patch adds an overall
    metadata check to ensure that multiple requests to add metadata to
    an account/container will check overall limits before adding
    the additional metadata.

    Change-Id: Ib9401a4ee05a9cb737939541bd9b84e8dc239c70
    Closes-Bug: 1365350

commit 301a96f664d58b4ccad8e3cbf5d5a889cc76790f
Author: Jay S. Bryant <email address hidden>
Date: Tue Sep 30 15:08:59 2014 -0500

    Ensure sys.exit called in fork_child after exception

    Currently, the fork_child() function in auditor.py does not
    handle the case where run_audit() encounters an exception
    properly.

    A simple case is where the /srv directory is set
    with permissions such that the 'swift' user cannot access it.
    Such a situation causes a os.listdir() to return an OSError
    exception. When this happens the fork_child() process does not
    run to completion and sys.exit() is not executed. The process
    that was forked off continues to run as a result. Execution goes
    back up to the audit_loop function which restarts the whole process. The
    end result is an increasing number of processes on the system
    until the parent is terminated. This can quickly exhaust the
    process descriptors on a system.

    This change wraps run_audit() in a try block and adds an
    exception handler that prints what exception was encountered.
    The sys.exit() was moved to a finally: block so that it will
    always be run, avoiding the creation of zombies.

    Change-Id: I89d7cd27112445893852e62df857c3d5262c27b3
    Closes-bug: 1375348

commit 6d49cc3092168de6d22378557b2c37ea4063beeb
Author: Samuel Merritt <email address hidden>
Date: Thu Oct 2 17:14:58 2014 -0400

    Fix ring-builder crash.

    If you adjust ...

Thierry Carrez (ttx)
Changed in swift:
milestone: 2.2.0-rc1 → 2.2.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.