distrib_codename alpha comparison will soon yield unintended behavior

[Ryan Beisner]

Presuming that after 'z' will come 'a' - with regard to distro codename, several charms are about to have unintended behavior due to the use of alpha comparison logic. The numeric version comparison should be used instead.

I've not done a full audit of all OpenStack Charms but that needs to be done as soon as possible, and associated with this bug.

Here are a some examples which need to be fixed:

https://github.com/openstack/charm-nova-compute/search?utf8=%E2%9C%93&q=distro_codename

if distro_codename < "wily":

if distro_codename >= 'yakkety':

.

Here is an example of the safe way to do it:

https://github.com/openstack/charm-nova-cloud-controller/search?utf8=%E2%9C%93&q=DISTRIB_RELEASE&type=Code

if lsb_release()['DISTRIB_RELEASE'] >= '15.10':

[Ryan Beisner (1chb1n)]

Some of these alpha comparisons are in charm repos (ie. hook code), and some of them are in charmhelpers.

Here are some examples of trouble-to-come from charmhelpers:

contrib/openstack/context.py: if lsb_release()['DISTRIB_CODENAME'].lower() > 'trusty':

contrib/hardening/ssh/checks/config.py: if lsb_release()['DISTRIB_CODENAME'].lower() >= 'trusty':

[Alex Kavanagh (ajkavanagh)]

I'm going to be the one to say it, but this was a bad idea from the start. charms.openstack mostly implements indexed listed comparisons - e.g. releases.index(release) > releases.index('trusty').

I'm guessing an audit of the charms, charm-helpers and then fix?

[James Page (james-page)]

"I'm going to be the one to say it, but this was a bad idea from the start."

Agreed and I like the way you solved this issue for charms.openstack, so lets follow that route!

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/447457

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/453261

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/453276

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/453276
Committed: https://git.openstack.org/cgit/openstack/charm-nova-compute/commit/?id=2358f94b30349a8543e75179750d1d61dba03a9f
Submitter: Jenkins
Branch: master

commit 2358f94b30349a8543e75179750d1d61dba03a9f
Author: Alex Kavanagh <email address hidden>
Date: Tue Apr 4 18:53:20 2017 +0100

    Fix alphanumeric comparisons for openstack and ubuntu releases

    - sync charmhelpers with fix-alpha helpers
    - fix up code where the alpha comparisons are done
    - fix tests which assumed mocks would just work on os_release()

    Change-Id: Iffd4ed0aecbb966fe493fe0f24bf2422aab55bb6
    Related-Bug: #1659575

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/447457
Committed: https://git.openstack.org/cgit/openstack/charm-keystone/commit/?id=b8b5acd44d5ecf7490b31802cf2459c215a95822
Submitter: Jenkins
Branch: master

commit b8b5acd44d5ecf7490b31802cf2459c215a95822
Author: Alex Kavanagh <email address hidden>
Date: Mon Mar 20 11:37:16 2017 +0000

    Fix alphanumeric comparisons for openstack and ubuntu releases

    - sync charmhelpers with fix-alpha helpers
    - fix up code where the alpha comparisons are done
    - fix tests which assumed mocks would just work on os_release()

    Change-Id: I9f4a3b15e53c757c2ae5ffb2eb45b6cdaecf4c8e
    Related-Bug: #1659575

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/453261
Committed: https://git.openstack.org/cgit/openstack/charm-nova-cloud-controller/commit/?id=f9b97ac66c8dd09349e325536fdeee0569abde0c
Submitter: Jenkins
Branch: master

commit f9b97ac66c8dd09349e325536fdeee0569abde0c
Author: Alex Kavanagh <email address hidden>
Date: Tue Apr 4 17:53:31 2017 +0100

    Fix alphanumeric comparisons for openstack and ubuntu releases

    - sync charmhelpers with fix-alpha helpers
    - fix up code where the alpha comparisons are done
    - fix tests which assumed mocks would just work on os_release()

    Change-Id: I48b4853ed343bd3188c16f2bc432b4ca0badc473
    Related-Bug: #1659575