fallocate_reserve cannot be specified as a percentage running under python3

Bug #1844368 reported by Jonathan Rosser
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
High
Unassigned

Bug Description

openstack-ansible sets a default value of "1%" for fallocate_reserve, and this has caused our swift tests to fail after switching to python3.

config:

[DEFAULT]
# Disable stderr logging
use_stderr = False
bind_ip = 172.29.244.100
bind_port = 6002
workers = 1
user = swift
devices = /srv
disable_fallocate = False
fallocate_reserve = 1%
<snip>

log:

Sep 17 11:43:57 aio1 swift-account-server[29425]: Error trying to load config from /etc/swift/account-server/account-server.conf: Error in file /etc/swift/account-server/account-server.conf: '%' must be followed by '%' or '(', found: '%'

Revision history for this message
Dmitriy Rabotyagov (noonedeadpunk) wrote :

I guess this comes from python config parser https://docs.python.org/3.6/library/configparser.html#configparser.ConfigParser.get

So there's a possible workaround in placing double %%, like "fallocate_reserve = 1%%", or pass "raw=true" to config parser.

I'm a bit worried, that changing behaviour might influence negatively on working environments.

Revision history for this message
Tim Burke (1-tim-z) wrote :

From IRC, looks like Thiago confirmed it. We If we don't manage to fix it in time for the initial Train release, we should definitely backport it.

Changed in swift:
status: New → Confirmed
importance: Undecided → High
tags: added: py3
Revision history for this message
Alexandre Lécuyer (alecuyer) wrote :

If the config file format does not change, it looks like this would have to be fixed in paste?
It sets up its ConfigParser without an option to set the "interpolation" parameter

Revision history for this message
Tim Burke (1-tim-z) wrote :

We already swap out some stuff [1] and register new config loaders [2] with paste. I think we can work around this entirely within Swift.

I share Dmitriy's concern about existing, running clusters, though -- we've allowed interpolation (if only out of ignorance), so the safest assumption is that somebody has deployed a system that uses it. I don't want to break them, so using raw=True doesn't seem right.

At the same time, it looks like py3's ConfigParser most-closely maps to py2's SafeConfigParser, and the more-forgiving interpolation from py2's ConfigParser is just gone.

I think what I proposed at https://review.opendev.org/#/c/685455/ should be a recent work-around?

Sorry; at least as it stands now, it does mean that any configs written out with the "1%%" workaround are going to break on upgrade, though.

[1] https://github.com/openstack/swift/blob/2.22.0/swift/common/wsgi.py#L72
[2] https://github.com/openstack/swift/blob/2.22.0/swift/common/wsgi.py#L101-L102

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

Reviewed: https://review.opendev.org/685455
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=9a33365f064c2fbde732780982e3d324b488e677
Submitter: Zuul
Branch: master

commit 9a33365f064c2fbde732780982e3d324b488e677
Author: Tim Burke <email address hidden>
Date: Fri Sep 27 11:04:43 2019 -0700

    py3: Allow percentages in configs

    Previously, configs like

        fallocate_reserve = 1%

    would cause a py3 backend server to fail to start, complaining like

        configparser.InterpolationSyntaxError: Error in file
        /etc/swift/object-server/1.conf.d: '%' must be followed
        by '%' or '(', found: '%'

    This could also come up in proxy-server configs, with things like
    percent signs in tempauth password.

    In general, we haven't really thought much about interpolation in
    configs. Python's default ConfigParser has always supported it, though,
    so we got it "for free". On py2, we didn't really have to think about
    it, since values like "1%" would pass through just fine. (It would blow
    up a SafeConfigParser, but a normal ConfigParser only does replacements
    when there's something like a "%(opt)s" in the value.)

    On py3, SafeConfigParser became ConfigParser, and the old interpolation
    mode (AFAICT) doesn't exist.

    Unfortunatley, since we "supported" interpolation, we have to assume
    there are deployments in the wild that use it, and try not to break
    them. So, do what we can to mimic the py2 behavior.

    Change-Id: I0f9cecd11f00b522a8486972551cb30af151ce32
    Closes-Bug: #1844368

Changed in swift:
status: Confirmed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift 2.23.0

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/losf)

Fix proposed to branch: feature/losf
Review: https://review.opendev.org/686864

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

Reviewed: https://review.opendev.org/686864
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=bfa8e9feb51f2b10adfec3a741661a76fcf73216
Submitter: Zuul
Branch: feature/losf

commit cb76e00e90aea834c8f3dd8a6ca5131acd43663b
Author: OpenStack Proposal Bot <email address hidden>
Date: Fri Oct 4 07:05:07 2019 +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: I40ce1d36f1c207a0d3e99a3a84a162b21b3c57cf

commit 527a57ffcdefc03a5080b07d63f0ded319e08dfe
Author: OpenStack Release Bot <email address hidden>
Date: Thu Oct 3 16:35:36 2019 +0000

    Update master for stable/train

    Add file to the reno documentation build to show release notes for
    stable/train.

    Use pbr instruction to increment the minor version number
    automatically so that master versions are higher than the versions on
    stable/train.

    Change-Id: Ia93e0b690f47c6231423a25dfd6a108a60378a21
    Sem-Ver: feature

commit 8a4becb12fbe3d4988ddee73536673d6f55682dd
Author: Tim Burke <email address hidden>
Date: Fri Sep 27 15:18:59 2019 -0700

    Authors/changelog for 2.23.0

    Also, make some CHANGELOG formatting more consistent.

    Change-Id: I380ee50e075a8676590e755f24a3fd7a7a331029

commit bf9346d88de2aeb06da3b2cde62ffa6200936367
Author: Tim Burke <email address hidden>
Date: Thu Aug 15 14:33:06 2019 -0700

    Fix some request-smuggling vectors on py3

    A Python 3 bug causes us to abort header parsing in some cases. We
    mostly worked around that in the related change, but that was *after*
    eventlet used the parsed headers to determine things like message
    framing. As a result, a client sending a malformed request (for example,
    sending both Content-Length *and* Transfer-Encoding: chunked headers)
    might have that request parsed properly and authorized by a proxy-server
    running Python 2, but the proxy-to-backend request could get misparsed
    if the backend is running Python 3. As a result, the single client
    request could be interpretted as multiple requests by an object server,
    only the first of which was properly authorized at the proxy.

    Now, after we find and parse additional headers that weren't parsed by
    Python, fix up eventlet's wsgi.input to reflect the message framing we
    expect given the complete set of headers. As an added precaution, if the
    client included Transfer-Encoding: chunked *and* a Content-Length,
    ensure that the Content-Length is not forwarded to the backend.

    Change-Id: I70c125df70b2a703de44662adc66f740cc79c7a9
    Related-Change: I0f03c211f35a9a49e047a5718a9907b515ca88d7
    Closes-Bug: 1840507

commit 0217b12b6d7d6f3727a54db65614ff1ef52d6286
Author: Matthew Oliver <email address hidden>
Date: Wed Sep 4 14:30:33 2019 +1000

    PDF Documentation Build tox target

    This patch adds a `pdf-docs` tox target that will build
    PDF versions of our docs. As per the Train community goal:

      https://governance.openstack.org/tc/goals/selected/train/pdf-doc-...

tags: added: in-feature-losf
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (master)

Related fix proposed to branch: master
Review: https://review.opendev.org/734721

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

Reviewed: https://review.opendev.org/734721
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=2854eddb4424327fc6dc9b7930fd2874b5b23df4
Submitter: Zuul
Branch: master

commit 2854eddb4424327fc6dc9b7930fd2874b5b23df4
Author: Tim Burke <email address hidden>
Date: Tue Jun 9 16:45:21 2020 -0700

    py3: (Better) fix percentages in configs

    We previously fixed a bunch of places, but not quite *all* the places;
    at the very least, some account-layer services (like the replicator and
    auditor IIRC) could still bomb out -- and it's important that
    replicators still respect fallocate_reserve!

    Now, do the NicerInterpolation thing every time we call readconf.
    Additionally, clean up the original fix to avoid globally
    monkey-patching configparser.

    Related-Bug: #1844368
    Closes-Bug: #1872553
    Change-Id: I4512e686cde37930f0482909f537220a57fef76b

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (stable/ussuri)

Related fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/737070

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (stable/train)

Related fix proposed to branch: stable/train
Review: https://review.opendev.org/737071

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (stable/ussuri)

Reviewed: https://review.opendev.org/737070
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=b40f25f9e41dcd77a4768b781ffe59f483911c4e
Submitter: Zuul
Branch: stable/ussuri

commit b40f25f9e41dcd77a4768b781ffe59f483911c4e
Author: Tim Burke <email address hidden>
Date: Tue Jun 9 16:45:21 2020 -0700

    py3: (Better) fix percentages in configs

    We previously fixed a bunch of places, but not quite *all* the places;
    at the very least, some account-layer services (like the replicator and
    auditor IIRC) could still bomb out -- and it's important that
    replicators still respect fallocate_reserve!

    Now, do the NicerInterpolation thing every time we call readconf.
    Additionally, clean up the original fix to avoid globally
    monkey-patching configparser.

    Related-Bug: #1844368
    Closes-Bug: #1872553
    Change-Id: I4512e686cde37930f0482909f537220a57fef76b
    (cherry picked from commit 2854eddb4424327fc6dc9b7930fd2874b5b23df4)

tags: added: in-stable-ussuri
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (stable/train)

Reviewed: https://review.opendev.org/737071
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=bb1263b8d4211c198dc8d5e25b9ac4cd120ae56b
Submitter: Zuul
Branch: stable/train

commit bb1263b8d4211c198dc8d5e25b9ac4cd120ae56b
Author: Tim Burke <email address hidden>
Date: Tue Jun 9 16:45:21 2020 -0700

    py3: (Better) fix percentages in configs

    We previously fixed a bunch of places, but not quite *all* the places;
    at the very least, some account-layer services (like the replicator and
    auditor IIRC) could still bomb out -- and it's important that
    replicators still respect fallocate_reserve!

    Now, do the NicerInterpolation thing every time we call readconf.
    Additionally, clean up the original fix to avoid globally
    monkey-patching configparser.

    Related-Bug: #1844368
    Closes-Bug: #1872553
    Change-Id: I4512e686cde37930f0482909f537220a57fef76b
    (cherry picked from commit 2854eddb4424327fc6dc9b7930fd2874b5b23df4)

tags: added: in-stable-train
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.