swift-ring-builder does not seem to properly parse setting weights for multiple devices at one time

Bug #1454433 reported by Troy Ablan
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Medium
Unassigned

Bug Description

This is Swift 2.3.0

swift-ring-builder 1.3

When listing multiple devices and their weights on the same command line, it seems to misparse the intent. set_info works fine, but set_weight does not.

-[/etc/swift:$]- swift-ring-builder test.builder set_weight d0 4000 d1 4000 d2 4000 d3 4000 d4 4000 d5 4000 d6 0 d7 4000
d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_"" weight set to 4000.0
Matched more than one device:
    d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_""
    d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_""
Are you sure you want to update the weight for these 2 devices? (y/N) y
d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_"" weight set to 4000.0
d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_"" weight set to 4000.0
Matched more than one device:
    d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_""
    d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_""
    d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_""
Are you sure you want to update the weight for these 3 devices? (y/N) y
d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_"" weight set to 4000.0
d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_"" weight set to 4000.0
d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_"" weight set to 4000.0
Matched more than one device:
    d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_""
    d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_""
    d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_""
    d3r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F04_""
Are you sure you want to update the weight for these 4 devices? (y/N) y
d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_"" weight set to 4000.0
d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_"" weight set to 4000.0
d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_"" weight set to 4000.0
d3r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F04_"" weight set to 4000.0
Matched more than one device:
    d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_""
    d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_""
    d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_""
    d3r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F04_""
    d4r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F01_""
Are you sure you want to update the weight for these 5 devices? (y/N) y
d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_"" weight set to 4000.0
d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_"" weight set to 4000.0
d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_"" weight set to 4000.0
d3r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F04_"" weight set to 4000.0
d4r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F01_"" weight set to 4000.0
Matched more than one device:
    d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_""
    d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_""
    d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_""
    d3r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F04_""
    d4r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F01_""
    d5r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F02_""
Are you sure you want to update the weight for these 6 devices? (y/N) y
d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_"" weight set to 4000.0
d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_"" weight set to 4000.0
d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_"" weight set to 4000.0
d3r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F04_"" weight set to 4000.0
d4r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F01_"" weight set to 4000.0
d5r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F02_"" weight set to 4000.0
Matched more than one device:
    d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_""
    d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_""
    d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_""
    d3r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F04_""
    d4r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F01_""
    d5r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F02_""
    d6r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F03_""
Are you sure you want to update the weight for these 7 devices? (y/N) y
d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_"" weight set to 0.0
d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_"" weight set to 0.0
d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_"" weight set to 0.0
d3r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F04_"" weight set to 0.0
d4r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F01_"" weight set to 0.0
d5r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F02_"" weight set to 0.0
d6r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F03_"" weight set to 0.0
Matched more than one device:
    d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_""
    d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_""
    d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_""
    d3r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F04_""
    d4r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F01_""
    d5r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F02_""
    d6r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F03_""
    d7r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F04_""
Are you sure you want to update the weight for these 8 devices? (y/N) y
d0r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F01_"" weight set to 4000.0
d1r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F02_"" weight set to 4000.0
d2r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F03_"" weight set to 4000.0
d3r1z1-[2001:db8:f1f6::806:1]:6000R[2001:db8:f1f6::806:1]:6000/F04_"" weight set to 4000.0
d4r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F01_"" weight set to 4000.0
d5r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F02_"" weight set to 4000.0
d6r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F03_"" weight set to 4000.0
d7r1z1-[2001:db8:f1f6::809:1]:6000R[2001:db8:f1f6::809:1]:6000/F04_"" weight set to 4000.0

Revision history for this message
clayg (clay-gerrard) wrote :

So if you've got some perl that's trying to update a bunch of (device, weight) pairs explicitly this bulk syntax can definitely be useful.

FWIW, the obvious `devs =` instead of `devs.extend` seemed to do the right thing [1]

The attached is just a failing unittest - it'd be nice to have a couple of tests like that to cover the behaviors we care about when the "devstr" matches multiple devices or whatever...

1. https://github.com/openstack/swift/blob/b4c1517ddd0d57f556721caf587c113e4abc0498/swift/cli/ringbuilder.py#L230

Revision history for this message
Romain LE DISEZ (rledisez) wrote :

Quick message to confirm this bug. Just ran into it.

swift-ring-builder object.builder set_weight d28 0 d29 0

2.2 => ok
d28r1z0-10.28.0.128:6000R10.28.0.128:6200/disk-00-000_"" weight set to 0.0
d29r1z0-10.28.0.128:6001R10.28.0.128:6201/disk-01-000_"" weight set to 0.0

2.7 => hu?
d28r1z0-10.28.0.128:6000R10.28.0.128:6200/disk-00-000_"" weight set to 0.0
d28r1z0-10.28.0.128:6000R10.28.0.128:6200/disk-00-000_"" weight set to 0.0
d29r1z0-10.28.0.128:6001R10.28.0.128:6201/disk-01-000_"" weight set to 0.0

2.12 => duplicate?
d28r1z0-10.28.0.128:6000R10.28.0.128:6200/disk-00-000_"" weight set to 0.0
Matched more than one device:
    d28r1z0-10.28.0.128:6000R10.28.0.128:6200/disk-00-000_""
    d29r1z0-10.28.0.128:6001R10.28.0.128:6201/disk-01-000_""
Are you sure you want to update the weight for these 2 devices? (y/N)

clayg (clay-gerrard)
tags: added: low-hanging-fruit
Changed in swift:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/509606
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=747b9d928624a3f44f1f9f0269489597cddc5997
Submitter: Jenkins
Branch: master

commit 747b9d928624a3f44f1f9f0269489597cddc5997
Author: Jan Zerebecki <email address hidden>
Date: Wed Oct 4 21:14:03 2017 +0200

    Fix swift-ring-builder set_weight with >1 device

    When iterating over the (device, weight) tuples do not carry over the
    device from the previous iteration.

    Closes-Bug: 1454433
    Change-Id: Iba82519b0b2bc80e2c1abbed308b651c4da4b06a

Changed in swift:
status: Confirmed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/deep)

Fix proposed to branch: feature/deep
Review: https://review.openstack.org/511941

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

Reviewed: https://review.openstack.org/511941
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=ded0343892787e4a33eea7c4f0eb14a999ec63d8
Submitter: Jenkins
Branch: feature/deep

commit 407f5394f0f5cb422c06b4e5b2f9fbfdb07782d1
Author: OpenStack Proposal Bot <email address hidden>
Date: Thu Oct 12 08:12:38 2017 +0000

    Imported Translations from Zanata

    For more information about this automatic import see:
    https://docs.openstack.org/i18n/latest/reviewing-translation-import.html

    Change-Id: I628cb09aa78d8e339b4762a3c9ed8aed43941261

commit 45ca39fc68cdb42b382c1638a92cc8d3cec5529a
Author: Clay Gerrard <email address hidden>
Date: Tue Oct 10 11:47:50 2017 -0700

    add mangle_client_paths to example config

    Change-Id: Ic1126fc95e8152025fccf25356c253facce3e3ec

commit 94bac4ab2fe65104d602378e8e49c37b8187a75d
Author: Tim Burke <email address hidden>
Date: Fri May 12 10:55:21 2017 -0400

    domain_remap: stop mangling client-provided paths

    The root_path option for domain_remap seems to serve two purposes:
     - provide the first component (version) for the backend request
     - be an optional leading component for the client request, which
       should be stripped off

    As a result, we have mappings like:

       c.a.example.com/v1/o -> /v1/AUTH_a/c/o

    instead of

       c.a.example.com/v1/o -> /v1/AUTH_a/c/v1/o

    which is rather bizarre. Why on earth did we *ever* start doing this?

    Now, this second behavior is managed by a config option
    (mangle_client_paths) with the default being to disable it.

    Upgrade Consideration
    =====================

    If for some reason you *do* want to drop some parts of the
    client-supplied path, add

       mangle_client_paths = True

    to the [filter:domain_remap] section of your proxy-server.conf. Do this
    before upgrading to avoid any loss of availability.

    UpgradeImpact
    Change-Id: I87944bfbf8b767e1fc36dbc7910305fa1f11eeed

commit a4a5494fd2fe8a43a5d50a21a1951266cc7c4212
Author: Alistair Coles <email address hidden>
Date: Mon Oct 9 11:33:28 2017 +0100

    test account autocreate listing format

    Related-Change: Id3ce37aa0402e2d8dd5784ce329d7cb4fbaf700d
    Change-Id: I50c22225bbebff71600bea9158bda1edd18b48b0

commit 8b7f15223cde4c19fd9cbbd97e8ad79a1b4afa8d
Author: Alistair Coles <email address hidden>
Date: Mon Oct 9 10:06:19 2017 +0100

    Add example to container-sync-realms.conf.5 man page

    Related-Change: I0760ce149e6d74f2b3f1badebac3e36da1ab7e77

    Change-Id: I129de42f91d7924c7bcb9952f17fe8a1a10ae219

commit 816331155c624c444ed123bcab412821bd7854fb
Author: HCLTech-SSW <email address hidden>
Date: Fri Oct 6 01:37:34 2017 -0700

    Added the man page for container-sync-realms.conf

    Updated the comments of reviewers.

    Change-Id: I0760ce149e6d74f2b3f1badebac3e36da1ab7e77
    Closes-Bug: #1607026

commit 747b9d928624a3f44f1f9f0269489597cddc5997
Author: Jan Zerebecki <email address hidden>
Date: Wed Oct 4 21:14:03 2017 +0200

    Fix swift-ring-builder set_weight with >1 device

  ...

Read more...

tags: added: in-feature-deep
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/s3api)

Fix proposed to branch: feature/s3api
Review: https://review.openstack.org/512277

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: feature/s3api
Review: https://review.openstack.org/512283

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on swift (feature/s3api)

Change abandoned by Alistair Coles (<email address hidden>) on branch: feature/s3api
Review: https://review.openstack.org/512283
Reason: I was just trying to get sensible topic

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

Reviewed: https://review.openstack.org/512277
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=f94d6567a7e2e8b3ca1168b4a41c42c1ee371af5
Submitter: Zuul
Branch: feature/s3api

commit 24188beb81d39790034fa0902246163a7bf54c91
Author: Samuel Merritt <email address hidden>
Date: Thu Oct 12 16:13:25 2017 -0700

    Remove some leftover threadpool cruft.

    Change-Id: I43a1a428bd96a2e18aac334c03743a9f94f7d3e1

commit 1d67485c0b935719e0c8999eb353dfd84713add6
Author: Samuel Merritt <email address hidden>
Date: Fri Apr 15 12:43:44 2016 -0700

    Move all monkey patching to one function

    Change-Id: I2db2e53c50bcfa17f08a136581cfd7ac4958ada2

commit 407f5394f0f5cb422c06b4e5b2f9fbfdb07782d1
Author: OpenStack Proposal Bot <email address hidden>
Date: Thu Oct 12 08:12:38 2017 +0000

    Imported Translations from Zanata

    For more information about this automatic import see:
    https://docs.openstack.org/i18n/latest/reviewing-translation-import.html

    Change-Id: I628cb09aa78d8e339b4762a3c9ed8aed43941261

commit 45ca39fc68cdb42b382c1638a92cc8d3cec5529a
Author: Clay Gerrard <email address hidden>
Date: Tue Oct 10 11:47:50 2017 -0700

    add mangle_client_paths to example config

    Change-Id: Ic1126fc95e8152025fccf25356c253facce3e3ec

commit 94bac4ab2fe65104d602378e8e49c37b8187a75d
Author: Tim Burke <email address hidden>
Date: Fri May 12 10:55:21 2017 -0400

    domain_remap: stop mangling client-provided paths

    The root_path option for domain_remap seems to serve two purposes:
     - provide the first component (version) for the backend request
     - be an optional leading component for the client request, which
       should be stripped off

    As a result, we have mappings like:

       c.a.example.com/v1/o -> /v1/AUTH_a/c/o

    instead of

       c.a.example.com/v1/o -> /v1/AUTH_a/c/v1/o

    which is rather bizarre. Why on earth did we *ever* start doing this?

    Now, this second behavior is managed by a config option
    (mangle_client_paths) with the default being to disable it.

    Upgrade Consideration
    =====================

    If for some reason you *do* want to drop some parts of the
    client-supplied path, add

       mangle_client_paths = True

    to the [filter:domain_remap] section of your proxy-server.conf. Do this
    before upgrading to avoid any loss of availability.

    UpgradeImpact
    Change-Id: I87944bfbf8b767e1fc36dbc7910305fa1f11eeed

commit a4a5494fd2fe8a43a5d50a21a1951266cc7c4212
Author: Alistair Coles <email address hidden>
Date: Mon Oct 9 11:33:28 2017 +0100

    test account autocreate listing format

    Related-Change: Id3ce37aa0402e2d8dd5784ce329d7cb4fbaf700d
    Change-Id: I50c22225bbebff71600bea9158bda1edd18b48b0

commit 8b7f15223cde4c19fd9cbbd97e8ad79a1b4afa8d
Author: Alistair Coles <email address hidden>
Date: Mon Oct 9 10:06:19 2017 +0100

    Add example to container-sync-realms.conf.5 man page

    Related-Change: I0760ce149e6d74f2b3f1badebac3e36da1ab7e77

    Change-Id: I129de42f91d7924c7bcb9952f17fe8a1a10ae219

commit 816331155c624c444ed123bcab412...

tags: added: in-feature-s3api
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift 2.16.0

This issue was fixed in the openstack/swift 2.16.0 release.

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.