"Remove chrony in a container" patch doesn't work

[Alex Kavanagh]

The "remove chrony in a container" patch doesn't actually work as it has a bug. The bug is that "chrony" is passed as a string to filter_missing_packages() which is expecting an list of packages. As strings are iterable, this results in filter_missing_packages looking a ['c', 'h', ...].

Fix: pass an an array.

Note, this got merged as there is no related test to verify it. This may be because there are no tests associated with the feature currently.

[Alex Kavanagh (ajkavanagh)]

Note the actual bug in in charms.ceph, but it affects the ceph-mon charm.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/charms.ceph/+/792720

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/charm-ceph-mon/+/792721

[Liam Young (gnuoy)]

I think it still doesn't work even after https://review.opendev.org/c/openstack/charms.ceph/+/792720. The logic seems wrong. Specifically:

         install_list = filter_missing_packages([CHRONY_PACKAGE])
         if not install_list:
             rm_packages.append(CHRONY_PACKAGE)

Since filter_missing_packages returns a list of packages which are installed this is only adding 'chrony' to the list if it is not present.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/charms.ceph/+/792720
Committed: https://opendev.org/openstack/charms.ceph/commit/626a5ce3ca868286d7341fd7e53e9e161f969ecc
Submitter: "Zuul (22348)"
Branch: master

commit 626a5ce3ca868286d7341fd7e53e9e161f969ecc
Author: Alex Kavanagh <email address hidden>
Date: Sat May 22 15:30:36 2021 +0100

    Fix "Remove chrony in a container" patch with test

    Unfortunately, the original patch didn't work [1]. Then a fix was
    applied also without a test [2]. Unfortunately, this wasn't correct
    either.

    This patch simplifies the function, which makes it a little easier to
    reason about. Also a test is added to stop regressions in the future.

    [1] Ie3c9c5899c1d46edd21c32868938d3290db321e7
    [2] Ifef76eacd8bd837d2181ec75e406aa35f88b8b5b
    Closes-Bug: #1929054

    Change-Id: I93f53274bc8c7e62ebb8cb7bf81ff816f4bfd98b

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/21.04
Review: https://review.opendev.org/c/openstack/charms.ceph/+/792881

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/charms.ceph/+/792881
Committed: https://opendev.org/openstack/charms.ceph/commit/d1515daf737ceaca8e544b15bdcee5c10f67888e
Submitter: "Zuul (22348)"
Branch: stable/21.04

commit d1515daf737ceaca8e544b15bdcee5c10f67888e
Author: Trent Lloyd <email address hidden>
Date: Thu May 20 11:47:43 2021 +0800

    filter_missing_packages expects a list, not string

    filter_missing_packages expects a list and not a single package name as
    a string. Pass the chrony package as a single item list to avoid it
    checking installation candiates for 'c', 'h', 'r', 'o', 'n' and 'y'

    This patch simplifies the function, which makes it a little easier to
    reason about. Also a test is added to stop regressions in the future.

    [1] Ie3c9c5899c1d46edd21c32868938d3290db321e7
    [2] Ifef76eacd8bd837d2181ec75e406aa35f88b8b5b
    Closes-Bug: #1929054

    Change-Id: I93f53274bc8c7e62ebb8cb7bf81ff816f4bfd98b

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Alex Kavanagh <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/charm-ceph-mon/+/792721
Reason: This is now fixed due to a charms.ceph sync in the last cycle.