[Merge] ~lamoura/update-notifier:update-esm-handling into update-notifier:master

Lucas Albuquerque Medeiros de Moura lucas.moura at canonical.com
Thu Apr 15 20:32:46 UTC 2021


Thanks for the review Grant,

I think the message suggestion is worth thinking about. But I agree that we don't have enough time to address it.

Regarding the integration, we are already relying on some parts of the shared messaging work. Specially for the ESM Apps header and the expired messages. But for other messages, I think that if in the future we do have a message that is not easy to compute in update-notifier and need to presented in the middle of the text produced by it, I think the right away is to rely on the uaclient shared messages infrastructure

Diff comments:

> diff --git a/tests/test_motd.py b/tests/test_motd.py
> index ec1a09f..873155c 100755
> --- a/tests/test_motd.py
> +++ b/tests/test_motd.py
> @@ -16,132 +18,398 @@ def get_message(*args, **kwds):
>  class TestMotd(unittest.TestCase):
>      """ Validate /etc/motd text """
>  
> -    def test_esm_disabled_upto_date_esm_avail(self):
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_disabled_upto_date_esm_avail(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
>          self.assertEqual(
>              get_message(upgrades=0, security_updates=0,
> -                        esm_updates=0, have_esm=False,
> -                        disabled_esm_updates=23),
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=False, have_esm_apps=False,
> +                        disabled_esm_infra_updates=23,
> +                        disabled_esm_apps_updates=0),
>              textwrap.dedent(
> -                """
> +                """\
>                  UA Infra: Extended Security Maintenance (ESM) is not enabled.
>  
>                  0 updates can be installed immediately.
>                  0 of these updates are security updates.
>  
> -                Enable UA Infra: ESM to receive 23 additional security updates.
> +                Enable UA Apps: ESM to receive additional future security updates.
>                  See https://ubuntu.com/security/esm or run: sudo ua status
> -                """).lstrip())
>  
> -    def test_esm_disabled_security_esm_avail(self):
> +                23 additional security updates can be applied with UA Infra: ESM
> +                Learn more about enabling UA Infra: ESM service at https://ubuntu.com/esm
> +                """))
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_disabled_security_esm_avail(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
>          self.assertEqual(
>              get_message(upgrades=15, security_updates=7,
> -                        esm_updates=0, have_esm=False,
> -                        disabled_esm_updates=23),
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=False, have_esm_apps=False,
> +                        disabled_esm_infra_updates=23,
> +                        disabled_esm_apps_updates=0),
>              textwrap.dedent(
> -                """
> +                """\
>                  UA Infra: Extended Security Maintenance (ESM) is not enabled.
>  
>                  15 updates can be installed immediately.
>                  7 of these updates are security updates.
>                  To see these additional updates run: apt list --upgradable
>  
> -                Enable UA Infra: ESM to receive 23 additional security updates.
> +                Enable UA Apps: ESM to receive additional future security updates.
>                  See https://ubuntu.com/security/esm or run: sudo ua status
> -                """).lstrip())
>  
> -    def test_esm_disabled_security_no_esm_avail(self):
> +                23 additional security updates can be applied with UA Infra: ESM
> +                Learn more about enabling UA Infra: ESM service at https://ubuntu.com/esm
> +                """))
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_disabled_security_no_esm_avail(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
>          self.assertEqual(
>              get_message(upgrades=15, security_updates=7,
> -                        esm_updates=0, have_esm=False,
> -                        disabled_esm_updates=0),
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=False, have_esm_apps=False,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=0),
>              textwrap.dedent(
> -                """
> +                """\
>                  UA Infra: Extended Security Maintenance (ESM) is not enabled.

>From what I understand, Infra is the place to go for ESM distros. Without it, you can't get any more updates on those distros. That's why I think we are prioritizing those type of message on ESM distros

>  
>                  15 updates can be installed immediately.
>                  7 of these updates are security updates.
>                  To see these additional updates run: apt list --upgradable
>  
> +                Enable UA Apps: ESM to receive additional future security updates.
> +                See https://ubuntu.com/security/esm or run: sudo ua status
> +
>                  Enable UA Infra: ESM to receive additional future security updates.
>                  See https://ubuntu.com/security/esm or run: sudo ua status
> -                """).lstrip())
> +                """))
>  
> -    def test_esm_disabled_nosecurity(self):
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_disabled_nosecurity(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
>          self.assertEqual(
>              get_message(upgrades=15, security_updates=0,
> -                        esm_updates=0, have_esm=False,
> -                        disabled_esm_updates=0),
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=False, have_esm_apps=False,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=0),
>              textwrap.dedent(
> -                """
> +                """\
>                  UA Infra: Extended Security Maintenance (ESM) is not enabled.
>  
>                  15 updates can be installed immediately.
>                  0 of these updates are security updates.
>                  To see these additional updates run: apt list --upgradable
>  
> +                Enable UA Apps: ESM to receive additional future security updates.
> +                See https://ubuntu.com/security/esm or run: sudo ua status
> +
>                  Enable UA Infra: ESM to receive additional future security updates.
>                  See https://ubuntu.com/security/esm or run: sudo ua status
> -                """).lstrip())
> +                """))
>  
> -    def test_esm_disabled_noupdates(self):
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_disabled_noupdates(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
>          self.assertEqual(
>              get_message(upgrades=0, security_updates=0,
> -                        esm_updates=0, have_esm=False,
> -                        disabled_esm_updates=0),
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=False, have_esm_apps=False,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=0),
>              textwrap.dedent(
> -                """
> +                """\
>                  UA Infra: Extended Security Maintenance (ESM) is not enabled.
>  
>                  0 updates can be installed immediately.
>                  0 of these updates are security updates.
>  
> +                Enable UA Apps: ESM to receive additional future security updates.
> +                See https://ubuntu.com/security/esm or run: sudo ua status
> +
>                  Enable UA Infra: ESM to receive additional future security updates.
>                  See https://ubuntu.com/security/esm or run: sudo ua status
> -                """).lstrip())
> +                """))
>  
> -    def test_esm_enabled_nosecurity(self):
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_enabled_nosecurity(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
>          self.assertEqual(
>              get_message(upgrades=35, security_updates=0,
> -                        esm_updates=13, have_esm=True,
> -                        disabled_esm_updates=0),
> +                        esm_infra_updates=13, esm_apps_updates=0,
> +                        have_esm_infra=True, have_esm_apps=False,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=0),
>              textwrap.dedent(
> -                """
> +                """\
>                  UA Infra: Extended Security Maintenance (ESM) is enabled.
>  
>                  35 updates can be installed immediately.
> -                13 of these updates are fixed through UA Infra: ESM.
> +                13 of these updates are UA Infra: ESM security updates.
>                  0 of these updates are security updates.
>                  To see these additional updates run: apt list --upgradable
> -                """).lstrip())
>  
> -    def test_esm_enabled_somesecurity(self):
> +                Enable UA Apps: ESM to receive additional future security updates.
> +                See https://ubuntu.com/security/esm or run: sudo ua status
> +                """))
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_enabled_somesecurity(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
>          self.assertEqual(
>              get_message(upgrades=47, security_updates=7,
> -                        esm_updates=13, have_esm=True,
> -                        disabled_esm_updates=0),
> +                        esm_infra_updates=13, esm_apps_updates=0,
> +                        have_esm_infra=True, have_esm_apps=False,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=0),
>              textwrap.dedent(
> -                """
> +                """\
>                  UA Infra: Extended Security Maintenance (ESM) is enabled.
>  
>                  47 updates can be installed immediately.
> -                13 of these updates are fixed through UA Infra: ESM.
> +                13 of these updates are UA Infra: ESM security updates.
>                  7 of these updates are security updates.
>                  To see these additional updates run: apt list --upgradable
> -                """).lstrip())
>  
> -    def test_esm_enabled_noupdates(self):
> +                Enable UA Apps: ESM to receive additional future security updates.
> +                See https://ubuntu.com/security/esm or run: sudo ua status
> +                """))
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_enabled_noupdates(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
>          self.assertEqual(
>              get_message(upgrades=0, security_updates=0,
> -                        esm_updates=0, have_esm=True,
> -                        disabled_esm_updates=0),
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=True, have_esm_apps=False,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=10),
>              textwrap.dedent(
> -                """
> +                """\
>                  UA Infra: Extended Security Maintenance (ESM) is enabled.
>  
>                  0 updates can be installed immediately.
>                  0 of these updates are security updates.
> +
> +                10 additional security updates can be applied with UA Apps: ESM
> +                Learn more about enabling UA Apps: ESM service at https://ubuntu.com/esm
> +                """))
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_and_esm_apps_enabled(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
> +        self.assertEqual(
> +            get_message(upgrades=30, security_updates=18,
> +                        esm_infra_updates=15, esm_apps_updates=15,
> +                        have_esm_infra=True, have_esm_apps=True,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=0),
> +            textwrap.dedent(
> +                """
> +                UA Infra: Extended Security Maintenance (ESM) is enabled.
> +
> +                30 updates can be installed immediately.
> +                15 of these updates are UA Infra: ESM security updates.
> +                15 of these updates are UA Apps: ESM security updates.
> +                18 of these updates are security updates.

Yeah, I can see the logic behind your idea. Right now I am trying to follow the spec as close as I can. But I do agree that we may update those message later

> +                To see these additional updates run: apt list --upgradable
> +                """).lstrip())
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_disable_and_esm_apps_enabled(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
> +        self.assertEqual(
> +            get_message(upgrades=30, security_updates=18,
> +                        esm_infra_updates=0, esm_apps_updates=15,
> +                        have_esm_infra=False, have_esm_apps=True,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=0),
> +            textwrap.dedent(
> +                """
> +                UA Infra: Extended Security Maintenance (ESM) is not enabled.
> +
> +                30 updates can be installed immediately.
> +                15 of these updates are UA Apps: ESM security updates.
> +                18 of these updates are security updates.
> +                To see these additional updates run: apt list --upgradable
> +
> +                Enable UA Infra: ESM to receive additional future security updates.
> +                See https://ubuntu.com/security/esm or run: sudo ua status
> +                """).lstrip())
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_disabled_wih_pkgs_and_esm_apps_enabled(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
> +        self.assertEqual(
> +            get_message(upgrades=30, security_updates=18,
> +                        esm_infra_updates=0, esm_apps_updates=15,
> +                        have_esm_infra=False, have_esm_apps=True,
> +                        disabled_esm_infra_updates=40,
> +                        disabled_esm_apps_updates=0),
> +            textwrap.dedent(
> +                """
> +                UA Infra: Extended Security Maintenance (ESM) is not enabled.
> +
> +                30 updates can be installed immediately.
> +                15 of these updates are UA Apps: ESM security updates.
> +                18 of these updates are security updates.
> +                To see these additional updates run: apt list --upgradable
> +
> +                40 additional security updates can be applied with UA Infra: ESM
> +                Learn more about enabling UA Infra: ESM service at https://ubuntu.com/esm
>                  """).lstrip())
>  
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_esm_infra_disabled_and_esm_apps_disabled_with_pkgs(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
> +        self.assertEqual(
> +            get_message(upgrades=30, security_updates=18,
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=False, have_esm_apps=False,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=40),
> +            textwrap.dedent(
> +                """\
> +                UA Infra: Extended Security Maintenance (ESM) is not enabled.
> +
> +                30 updates can be installed immediately.
> +                18 of these updates are security updates.
> +                To see these additional updates run: apt list --upgradable
> +
> +                40 additional security updates can be applied with UA Apps: ESM
> +                Learn more about enabling UA Apps: ESM service at https://ubuntu.com/esm
> +
> +                Enable UA Infra: ESM to receive additional future security updates.
> +                See https://ubuntu.com/security/esm or run: sudo ua status
> +                """))
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=True)
> +    def test_no_esm_infra_and_apps_index_in_esm_distro(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
> +        self.assertEqual(
> +            get_message(upgrades=30, security_updates=18,
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=None, have_esm_apps=None,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=0),
> +            textwrap.dedent(
> +                """\
> +                30 updates can be installed immediately.
> +                18 of these updates are security updates.
> +                To see these additional updates run: apt list --upgradable
> +                """))
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=False)
> +    @mock.patch("apt_check.is_esm_distro", return_value=False)
> +    def test_no_esm_infra_and_apps_index_in_non_lts_distro(
> +        self, _m_esm_distro, _m_is_lts
> +    ):
> +        self.assertEqual(
> +            get_message(upgrades=30, security_updates=18,
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=None, have_esm_apps=None,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=0),
> +            textwrap.dedent(
> +                """\
> +                30 updates can be installed immediately.
> +                18 of these updates are security updates.
> +                To see these additional updates run: apt list --upgradable
> +                """))
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=False)
> +    @mock.patch("apt_check.is_esm_distro", return_value=False)
> +    def test_esm_infra_disabled_and_esm_apps_disabled_with_no_esm_distro(
> +        self, _m_esm_distro, _m_is_lts,
> +    ):
> +        self.assertEqual(
> +            get_message(upgrades=30, security_updates=18,
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=False, have_esm_apps=False,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=40),
> +            textwrap.dedent(
> +                """\
> +                30 updates can be installed immediately.
> +                18 of these updates are security updates.
> +                To see these additional updates run: apt list --upgradable
> +                """))
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=False)
> +    @mock.patch("subprocess.Popen")
> +    def test_message_for_distro_that_will_not_go_into_esm_mode(
> +        self, m_popen, _m_is_lts
> +    ):
> +        comm_mock = mock.MagicMock()
> +        comm_mock.communicate.return_value = (
> +            "(unknown)".encode("utf-8"), "")
> +        m_popen.return_value = comm_mock
> +
> +        self.assertEqual(
> +            get_message(upgrades=30, security_updates=18,
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=False, have_esm_apps=False,
> +                        disabled_esm_infra_updates=0,
> +                        disabled_esm_apps_updates=40),
> +            textwrap.dedent(
> +                """\
> +                30 updates can be installed immediately.
> +                18 of these updates are security updates.
> +                To see these additional updates run: apt list --upgradable
> +                """))
> +
> +    @mock.patch("apt_check.is_lts_distro", return_value=True)
> +    @mock.patch("apt_check.is_esm_distro", return_value=False)
> +    def test_message_for_lts_distro_not_in_esm_mode_yet(
> +        self, _m_is_esm, _m_is_lts
> +    ):
> +        self.assertEqual(
> +            get_message(upgrades=30, security_updates=18,
> +                        esm_infra_updates=0, esm_apps_updates=0,
> +                        have_esm_infra=False, have_esm_apps=False,
> +                        disabled_esm_infra_updates=10,
> +                        disabled_esm_apps_updates=40),
> +            textwrap.dedent(
> +                """\
> +                30 updates can be installed immediately.
> +                18 of these updates are security updates.
> +                To see these additional updates run: apt list --upgradable
> +
> +                40 additional security updates can be applied with UA Apps: ESM
> +                Learn more about enabling UA Apps: ESM service at https://ubuntu.com/esm
> +                """))
> +
>  
>  if __name__ == "__main__":
>      import logging


-- 
https://code.launchpad.net/~lamoura/update-notifier/+git/update-notifier/+merge/401002
Your team Ubuntu Core Development Team is subscribed to branch update-notifier:master.



More information about the Ubuntu-reviews mailing list