swift-ring-builder output display bad column alignment with ipv6

Bug #1567105 reported by clayg
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Wishlist
Nandini

Bug Description

things just don't line up - we're probably expecting some sort of fixed width thing - I think some of the dispersion output commands have some weird auto formatting - we should either wrap up all of our tabular output to a helper module...

    vagrant@saio:/vagrant/.scratch$ swift-ring-builder container.builder
    container.builder, build version 103
    1024 partitions, 3.000000 replicas, 1 regions, 1 zones, 3 devices, 0.00 balance, 0.00 dispersion
    The minimum number of hours before a partition can be reassigned is 1 (0:00:00 remaining)
    The overload factor is 10.00% (0.100000)
    Ring file container.ring.gz not found, probably it hasn't been written yet
    Devices: id region zone ip address port replication ip replication port name weight partitions balance flags meta
                42 1 1 fd22:d006:1028:1070::77 6001 fd22:d006:1028:1080::77 6004 d42 12.73 1024 -0.00
                43 1 1 fd22:d006:1028:1070::77 6001 fd22:d006:1028:1080::77 6004 d43 12.73 1024 -0.00
                44 1 1 fd22:d006:1028:1070::77 6001 fd22:d006:1028:1080::77 6004 d44 12.73 1024 -0.00

Revision history for this message
Pete Zaitcev (zaitcev) wrote :

Since we're at it, we ought to do something about the insane waste in the "replication port" column. Rename it to "rep.port" or something. The "Devices:" prefix is also worthless.

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

i don't like the cryptic syntax of "rep.port" - maybe if we made the output syntax "cluster ip:port", "replication ip:port" it would be cooler?

mind the "[ip:v6]:port" syntax

Changed in swift:
assignee: nobody → Sivasathurappan Radhakrishnan (siva-radhakrishnan)
Changed in swift:
assignee: Sivasathurappan Radhakrishnan (siva-radhakrishnan) → nobody
Nandini (nandini-tata)
Changed in swift:
assignee: nobody → Nandini (nandini-tata)
Revision history for this message
Pete Zaitcev (zaitcev) wrote :

I'm onboard with clayg's suggestion. I didn't want to change columns in case anyone parses the output programmatically. It seemed less risky only reformat the headers. Are we going to add something like -h key (for human-readable output), or just take our chances with compatibility?

Revision history for this message
Nandini (nandini-tata) wrote :

I was thinking of keeping the columns as is (separate columns for ip and ports) and just aligning the output irrespective of ipv4 or ipv6 address. That way, we don't have to worry about square braces for [ipv6] as we would not be combining ip address with the port (ip address:port)

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/319387

Changed in swift:
status: New → In Progress
Revision history for this message
clayg (clay-gerrard) wrote :

I think programatic parsing of this text output should be unsupported. We've never documented the format as stable - buyer beware. I'd be ok with a a `swift-ring-builder <ring>.builder --json` - seems unrelated.

I think combining the ip address with port has potential to save critical column space ("replication port" is 400% longer than the typical four digit port number) and also *help* with readability.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/319387
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=99a6f915ffc3aba2086e22dfba1f33b4fee46e81
Submitter: Jenkins
Branch: master

commit 99a6f915ffc3aba2086e22dfba1f33b4fee46e81
Author: Nandini Tata <email address hidden>
Date: Fri May 20 17:41:29 2016 +0000

    swift-ring-builder output corrected for ipv6

    Adjusted width of ip and port columns in swift-ring-builder command
    output to dynamically span to the longest ip or the longest port in
    the devices list. Also combined the port and ip address columns for
    better visual clarity. Took care of ipv6 format [ipv6]:port

    Modified the corresponding test case with expected output.

    Change-Id: I65837f8fed70be60b53d5a817a4ce529ad0f070e
    Closes-Bug: #1567105

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

Fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/363111

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

Reviewed: https://review.openstack.org/363111
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=1ab2a296f58ae76aeffef9f3f0fb90e15358be27
Submitter: Jenkins
Branch: feature/hummingbird

commit 3b5850836c59c46f2507a7f62aceccf4c37e5d41
Author: gecong1973 <email address hidden>
Date: Tue Aug 30 15:08:49 2016 +0800

    Remove white space between print and ()

    There is a white space between print and ()
    in /tempauth.py, This patch fix it

    Change-Id: Id3493bdef12223aa3a2bffc200db8710f5949101

commit f88e7fc0da2ed6a63e0ea3c3459d80772b3068cd
Author: zheng yin <email address hidden>
Date: Mon Aug 29 20:21:44 2016 +0800

    Clarify test case in common/ring/test_builder

    They use a bare assertRaises(ValueError, ring.RingBuilder, *,*,*), but
    it's not clear which one raises which ValueError(), so I extend them to
    validate the error strings as well.

    Change-Id: I63280a9fc47ff678fe143e635046a0b402fd4506

commit d68b1bd6ddf44c5088e9d02dcb2f1b802c71411b
Author: zhufl <email address hidden>
Date: Mon Aug 29 14:31:27 2016 +0800

    Remove unnecessary tearDown

    This is to remove unnecessary tearDown to keep code clean.

    Change-Id: Ie70e40d6b55f379b0cc9bc372a35705462cade8b

commit d2fc2614575b04fd9cab5ae589880b92eee9b186
Author: Matthew Oliver <email address hidden>
Date: Fri Aug 19 16:17:31 2016 +1000

    Authorise versioned write PUTs before copy

    Currently a versioned write PUT uses a pre-authed request to move
    it into the versioned container before checking whether the
    user is authorised. This can lead to some interesting behaviour
    whereby a user can select a versioned object path that it does not
    have access to, request a put on that versioned object, and this
    request will execute the copy part of the request before it fails
    due to lack of permissions.

    This patch changes the behaviour to be the same as versioned DELETE
    where the request is authorised before anything is moved.

    Change-Id: Ia8b92251718d10b1eb44a456f28d3d2569a30003
    Closes-Bug: #1562175

commit c1ef6539b6ba9d2e4354c9cd2eec8a0195cdb19f
Author: Clay Gerrard <email address hidden>
Date: Thu Aug 25 11:00:49 2016 -0700

    add test for expirer processes == process

    This is a follow up from a change that improved the error message.

    Related-Change: I3d12b79470d122b2114f9ee486b15d381f290f95

    Change-Id: I093801f3516a60b298c13e2aa026c11c68a63792

commit 01477c78c1163822de41484e914a0736e622085b
Author: zheng yin <email address hidden>
Date: Thu Aug 25 15:37:42 2016 +0800

    Fix ValueError information in obj/expirer

    I fix error information in raise ValueError(...)
    For example:
        if a>=b:
            # It should be under below and not 'a must be less than or equal to b'
            raise ValueError('a must be less than b')

    Change-Id: I3d12b79470d122b2114f9ee486b15d381f290f95

commit b81f53b964fdb8f3b50dd369ce2e194ee4dbb0b7
Author: zheng yin <email address hidden>
Date: Tue Aug 23 14:26:47 2016 +0800

    Improve readability in the obj server's unit tests

    This change improves the reada...

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

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

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

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