Merge ~simpoir/ubuntu/+source/landscape-client:beyond_19.04 into ubuntu/+source/landscape-client:ubuntu/devel

[Simon Poirier]

[Andreas Hasenack (ahasenack)]

Thanks, comments inline

review: Needs Fixing

[Andreas Hasenack (ahasenack)]

You are still patching a maintainer script (postinst)

review: Needs Fixing

[Simon Poirier (simpoir)]

I've "unpatchified" that change.

[Andreas Hasenack (ahasenack)]

LGTM, +1

Do you want immediate sponsoring of this?

review: Approve

[Andreas Hasenack (ahasenack)]

Sponsoring ba2b91dfd960345a2bcdae49cd034f6e579a2287

[Andreas Hasenack (ahasenack)]

Tagged and uploaded:

$ git push pkg upload/18.01-0ubuntu9
Enumerating objects: 32, done.
Counting objects: 100% (32/32), done.
Delta compression using up to 2 threads
Compressing objects: 100% (25/25), done.
Writing objects: 100% (25/25), 6.95 KiB | 711.00 KiB/s, done.
Total 25 (delta 17), reused 0 (delta 0)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/landscape-client
 * [new tag] upload/18.01-0ubuntu9 -> upload/18.01-0ubuntu9

$ dput ubuntu ../landscape-client_18.01-0ubuntu9_source.changes
Checking signature on .changes
gpg: ../landscape-client_18.01-0ubuntu9_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../landscape-client_18.01-0ubuntu9.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading landscape-client_18.01-0ubuntu9.dsc: done.
  Uploading landscape-client_18.01-0ubuntu9.debian.tar.xz: done.
  Uploading landscape-client_18.01-0ubuntu9_source.buildinfo: done.
  Uploading landscape-client_18.01-0ubuntu9_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 181c79d..c8629f4 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+landscape-client (18.01-0ubuntu9) eoan; urgency=medium
+
+  * d/p/product-name-vminfo-1828217.patch: Add product_name to things scanned
+    for vm_info (LP: #1828217)
+  * d/landscape-client.postinst: Set default value if data_path is
+    missing. (LP: #1728681)
+  * d/p/stagger-launch-1788518.patch: Add option to stagger launch of broker
+    plugins. (LP: #1788518)
+  * d/landscape-client.init: Fix init script stop action (LP: #1833137)
+
+ -- Simon Poirier <[e-mail]>  Fri, 14 Jun 2019 17:29:07 -0400
+
 landscape-client (18.01-0ubuntu8) eoan; urgency=medium
 
   * Set LANG in sysinfo wrapper. (LP: #1780071)
diff --git a/debian/landscape-client.init b/debian/landscape-client.init
index fcaeafd..ed51cc2 100644
--- a/debian/landscape-client.init
+++ b/debian/landscape-client.init
@@ -63,7 +63,8 @@ case "$1" in
 
 	stop)
 		log_daemon_msg "Stopping $NAME daemon"
-		start-stop-daemon --retry 30 --stop --quiet --pidfile $PIDFILE
+		# not a typo, linux process names are truncated to 15chars
+		start-stop-daemon --retry 30 --stop --quiet --pidfile $PIDFILE -n landscape-clien
 		log_end_msg $?
 		;;
 
diff --git a/debian/landscape-client.postinst b/debian/landscape-client.postinst
index d8d91ba..6e441ea 100644
--- a/debian/landscape-client.postinst
+++ b/debian/landscape-client.postinst
@@ -102,6 +102,9 @@ END
         # user information.  The flag file will be removed by the client when
         # the update completes.
         DATA_PATH="`grep ^data_path /etc/landscape/client.conf | cut -d= -f2 | tr -d '[[:space:]]'`"
+        if [ -z "$DATA_PATH" ]; then
+            DATA_PATH=/var/lib/landscape/client
+        fi
         install --owner=landscape --directory $DATA_PATH
         USER_UPDATE_FLAG_FILE="$DATA_PATH/user-update-flag"
         install --owner=landscape /dev/null $USER_UPDATE_FLAG_FILE
diff --git a/debian/patches/product-name-vminfo-1828217.patch b/debian/patches/product-name-vminfo-1828217.patch
 new file mode 100644
index 0000000..4420e0c
--- /dev/null
+++ b/debian/patches/product-name-vminfo-1828217.patch
@@ -0,0 +1,49 @@
+Description: Add product_name to things scanned for vm_info
+Author: Simon Poirier <[e-mail]>
+Origin: upstream, https://github.com/CanonicalLtd/landscape-client/commit/ad28feeb7d3031268eeb206c4d8bdcd015ad79ae
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1828217
+Last-Update: 2019-06-14
+
+--- a/landscape/lib/tests/test_vm_info.py
++++ b/landscape/lib/tests/test_vm_info.py
+@@ -179,6 +179,11 @@
+         self.make_dmi_info("sys_vendor", "Some other vendor")
+         self.assertEqual(b"", get_vm_info(root_path=self.root_path))
+ 
++    def test_get_vm_info_with_kvm_product(self):
++        """get_vm_info returns 'kvm', if product_name is 'KVM'."""
++        self.make_dmi_info("product_name", "KVM")
++        self.assertEqual(b"kvm", get_vm_info(root_path=self.root_path))
++
+ 
+ class GetContainerInfoTest(BaseTestCase):
+ 
+--- a/landscape/lib/vm_info.py
++++ b/landscape/lib/vm_info.py
+@@ -6,6 +6,9 @@
+ from landscape.lib.fs import read_text_file
+ 
+ 
++DMI_FILES = ("sys_vendor", "chassis_vendor", "bios_vendor", "product_name")
++
++
+ def get_vm_info(root_path="/"):
+     """
+     Return a bytestring with the virtualization type if it's known, an empty
+@@ -22,7 +25,7 @@
+     # Iterate through all dmi *_vendors, as clouds can (and will) customize
+     # sysinfo values. (https://libvirt.org/formatdomain.html#elementsSysinfo)
+     dmi_info_path = os.path.join(root_path, "sys/class/dmi/id")
+-    for dmi_info_file in ("sys_vendor", "chassis_vendor", "bios_vendor"):
++    for dmi_info_file in DMI_FILES:
+         dmi_vendor_path = os.path.join(dmi_info_path, dmi_info_file)
+         if not os.path.exists(dmi_vendor_path):
+             continue
+@@ -76,6 +79,7 @@
+         ("nutanix", b"kvm"),
+         ("openstack", b"kvm"),
+         ("qemu", b"kvm"),
++        ("kvm", b"kvm"),
+         ("vmware", b"vmware"))
+     for name, vm_type in content_vendors_map:
+         if name in vendor:
diff --git a/debian/patches/series b/debian/patches/series
index 79d72dd..ebfbf13 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -7,3 +7,5 @@ post-upgrade-reboot.patch
 unicode-tags-script.patch
 1616116-resync-loop.patch
 mem-usage-1685885.patch
+product-name-vminfo-1828217.patch
+stagger-launch-1788518.patch
diff --git a/debian/patches/stagger-launch-1788518.patch b/debian/patches/stagger-launch-1788518.patch
 new file mode 100644
index 0000000..d0f2908
--- /dev/null
+++ b/debian/patches/stagger-launch-1788518.patch
@@ -0,0 +1,258 @@
+Description: Add option to stagger launch of broker plugins.
+Author: Simon Poirier <[e-mail]>
+Origin: upstream, https://github.com/CanonicalLtd/landscape-client/commit/597a584083f5245b8960e6c20d67e0fe25bdcdc2
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1788518
+Last-Update: 2019-06-14
+
+diff --git a/example.conf b/example.conf
+index 47dac275..3965d29d 100644
+--- a/example.conf
++++ b/example.conf
+@@ -50,6 +50,16 @@ ping_url = http://landscape.canonical.com/ping
+ #
+ #ssl_public_key
+ 
++# The ratio over which to spread landscape intervals. This is to mitigate
++# the impact of restarting the host of multiple landscape-client instances.
++# Values range from 0.0 to 1.0, from unstaggered to spreading over
++# the full interval.
++#
++# Default value is 0.1, meaning jobs will start with up to 10% delay
++#
++# Example:
++#   stagger_launch = 0.5
++
+ # If set to True interrupt (SIGINT) signals will be ignored by the
+ # landscape-client daemon.
+ ignore_sigint = False
+diff --git a/landscape/client/broker/client.py b/landscape/client/broker/client.py
+index 62287329..042c18a4 100644
+--- a/landscape/client/broker/client.py
++++ b/landscape/client/broker/client.py
+@@ -1,5 +1,6 @@
+-from logging import info, exception, error
++from logging import info, exception, error, debug
+ import sys
++import random
+ 
+ from twisted.internet.defer import maybeDeferred, succeed
+ 
+@@ -105,9 +106,18 @@ class BrokerClientPlugin(object):
+             if self.run_immediately:
+                 self._run_with_error_log()
+             if self.run_interval is not None:
+-                self._loop = self.client.reactor.call_every(
+-                    self.run_interval,
+-                    self._run_with_error_log)
++                delay = (random.random() * self.run_interval *
++                         self.client.config.stagger_launch)
++                debug("delaying start of %s for %d seconds",
++                      format_object(self), delay)
++                self._loop = self.client.reactor.call_later(
++                    delay, self._start_loop)
++
++    def _start_loop(self):
++        """Launch the client loop."""
++        self._loop = self.client.reactor.call_every(
++            self.run_interval,
++            self._run_with_error_log)
+ 
+     def _run_with_error_log(self):
+         """Wrap self.run in a Deferred with a logging error handler."""
+@@ -140,10 +150,11 @@ class BrokerClient(object):
+     """
+     name = "client"
+ 
+-    def __init__(self, reactor):
++    def __init__(self, reactor, config):
+         super(BrokerClient, self).__init__()
+         self.reactor = reactor
+         self.broker = None
++        self.config = config
+         self._registered_messages = {}
+         self._plugins = []
+         self._plugin_names = {}
+diff --git a/landscape/client/broker/tests/helpers.py b/landscape/client/broker/tests/helpers.py
+index 97c5e4f4..fc010209 100644
+--- a/landscape/client/broker/tests/helpers.py
++++ b/landscape/client/broker/tests/helpers.py
+@@ -206,7 +206,9 @@ class BrokerClientHelper(RemoteBrokerHelper):
+         # The client needs its own reactor to avoid infinite loops
+         # when the broker broadcasts and event
+         test_case.client_reactor = FakeReactor()
+-        test_case.client = BrokerClient(test_case.client_reactor)
++        config = BrokerConfiguration()
++        config.stagger_launch = 0  # let's keep tests deterministic
++        test_case.client = BrokerClient(test_case.client_reactor, config)
+         test_case.client.broker = test_case.remote
+ 
+ 
+diff --git a/landscape/client/broker/tests/test_client.py b/landscape/client/broker/tests/test_client.py
+index 967f77db..3295b02b 100644
+--- a/landscape/client/broker/tests/test_client.py
++++ b/landscape/client/broker/tests/test_client.py
+@@ -178,6 +178,26 @@ class BrokerClientTest(LandscapeTest):
+         deferred.callback(123)
+         self.assertEquals(runs, [True, True])
+ 
++    @mock.patch("random.random")
++    def test_run_interval_staggered(self, mock_random):
++        """
++        If a plugin has a run method and staggered_launch is set,
++        the launch gets delayed by a random factor.
++        """
++        mock_random.return_value = 1.0
++        plugin = BrokerClientPlugin()
++        plugin.run_interval = 60
++        plugin.run = mock.Mock()
++        self.client.config.stagger_launch = 0.5
++        self.client.add(plugin)
++        self.client_reactor.advance(30)
++        self.assertEqual(0, plugin.run.call_count)
++        self.client_reactor.advance(60)
++        self.assertEqual(1, plugin.run.call_count)
++        self.client_reactor.advance(60)
++        self.assertEqual(2, plugin.run.call_count)
++        self.assertEqual(1, mock_random.call_count)
++
+     def test_run_immediately(self):
+         """
+         If a plugin has a C{run} method and C{run_immediately} is C{True},
+diff --git a/landscape/client/deployment.py b/landscape/client/deployment.py
+index 8c2f905e..7c5464cb 100644
+--- a/landscape/client/deployment.py
++++ b/landscape/client/deployment.py
+@@ -82,6 +82,7 @@ class Configuration(BaseConfiguration):
+               - C{ping_url} (C{"http://landscape.canonical.com/ping"})
+               - C{ssl_public_key}
+               - C{ignore_sigint} (C{False})
++              - C{stagger_launch} (C{0.1})
+         """
+         parser = super(Configuration, self).make_parser()
+         logging.add_cli_options(parser, logdir="/var/log/landscape")
+@@ -111,6 +112,10 @@ class Configuration(BaseConfiguration):
+                           metavar="INTERVAL",
+                           help="The number of seconds between flushes to disk "
+                                "for persistent data.")
++        parser.add_option("--stagger-launch", metavar="STAGGER_RATIO",
++                          dest="stagger_launch", default=0.1, type=float,
++                          help="Ratio, between 0 and 1, by which to scatter "
++                               "various tasks of landscape.")
+ 
+         # Hidden options, used for load-testing to run in-process clones
+         parser.add_option("--clones", default=0, type=int, help=SUPPRESS_HELP)
+diff --git a/landscape/client/manager/manager.py b/landscape/client/manager/manager.py
+index a662e13e..18d13148 100644
+--- a/landscape/client/manager/manager.py
++++ b/landscape/client/manager/manager.py
+@@ -12,7 +12,7 @@ class Manager(BrokerClient):
+     name = "manager"
+ 
+     def __init__(self, reactor, config):
+-        super(Manager, self).__init__(reactor)
++        super(Manager, self).__init__(reactor, config)
+         self.reactor = reactor
+         self.config = config
+         self.store = ManagerStore(self.config.store_filename)
+diff --git a/landscape/client/monitor/monitor.py b/landscape/client/monitor/monitor.py
+index 7ab82560..ebd3823d 100644
+--- a/landscape/client/monitor/monitor.py
++++ b/landscape/client/monitor/monitor.py
+@@ -12,7 +12,7 @@ class Monitor(BrokerClient):
+ 
+     def __init__(self, reactor, config, persist, persist_filename=None,
+                  step_size=5 * 60):
+-        super(Monitor, self).__init__(reactor)
++        super(Monitor, self).__init__(reactor, config)
+         self.reactor = reactor
+         self.config = config
+         self.persist = persist
+diff --git a/landscape/client/tests/helpers.py b/landscape/client/tests/helpers.py
+index 73775f99..a15353fc 100644
+--- a/landscape/client/tests/helpers.py
++++ b/landscape/client/tests/helpers.py
+@@ -175,6 +175,7 @@ class MonitorHelper(FakeBrokerServiceHelper):
+         persist_filename = test_case.makePersistFile()
+         test_case.config = MonitorConfiguration()
+         test_case.config.load(["-c", test_case.config_filename])
++        test_case.config.stagger_launch = 0  # let's keep tests deterministic
+         test_case.reactor = FakeReactor()
+         test_case.monitor = Monitor(
+             test_case.reactor, test_case.config,
+diff --git a/landscape/client/tests/test_configuration.py b/landscape/client/tests/test_configuration.py
+index 58381321..36d6d3ec 100644
+--- a/landscape/client/tests/test_configuration.py
++++ b/landscape/client/tests/test_configuration.py
+@@ -753,6 +753,13 @@ class LandscapeSetupScriptTest(LandscapeTest):
+         self.assertEqual(self.config.access_group, u"webservers")
+         mock_input.assert_not_called()
+ 
++    @mock.patch("landscape.client.configuration.input")
++    def test_stagger_defined_on_command_line(self, mock_input):
++        self.assertEqual(self.config.stagger_launch, 0.1)
++        self.config.load_command_line(["--stagger-launch", "0.5"])
++        self.assertEqual(self.config.stagger_launch, 0.5)
++        mock_input.assert_not_called()
++
+     @mock.patch("landscape.client.configuration.show_help")
+     def test_show_header(self, mock_show_help):
+         help_snippet = "This script will"
+diff --git a/man/landscape-client.1 b/man/landscape-client.1
+index 3a16c4a9..adb4bdc1 100644
+--- a/man/landscape-client.1
++++ b/man/landscape-client.1
+@@ -1,5 +1,5 @@
+ .\" Text automatically generated by txt2man
+-.TH landscape-client 1 "05 January 2017" "" ""
++.TH landscape-client 1 "14 February 2019" "" ""
+ .SH NAME
+ \fBlandscape-client \fP- Landscape system client
+ \fB
+diff --git a/man/landscape-config.1 b/man/landscape-config.1
+index 6f362f2f..7d398fff 100644
+--- a/man/landscape-config.1
++++ b/man/landscape-config.1
+@@ -1,5 +1,5 @@
+ .\" Text automatically generated by txt2man
+-.TH landscape-config 1 "05 January 2017" "" ""
++.TH landscape-config 1 "14 February 2019" "" ""
+ .SH NAME
+ \fBlandscape-config \fP- configure the Landscape management client
+ \fB
+@@ -139,6 +139,11 @@ Comma separated list of tag names to be sent to the
+ server.
+ .TP
+ .B
++\fB--stagger-launch\fP=STAGGER_RATIO
++Ratio, between 0 and 1, by which to scatter various
++tasks of landscape.
++.TP
++.B
+ \fB--import\fP=FILENAME_OR_URL
+ Filename or URL to import configuration from.
+ Imported \fIoptions\fP behave as if they were passed in
+diff --git a/man/landscape-config.txt b/man/landscape-config.txt
+index 223398b7..5a100bf0 100644
+--- a/man/landscape-config.txt
++++ b/man/landscape-config.txt
+@@ -58,6 +58,8 @@ OPTIONS
+                            EC2 or UEC. Read below for details.
+   --tags=TAGS              Comma separated list of tag names to be sent to the
+                            server.
++  --stagger-launch=STAGGER_RATIO  Ratio, between 0 and 1, by which to scatter various
++                           tasks of landscape.
+   --import=FILENAME_OR_URL   Filename or URL to import configuration from.
+                            Imported options behave as if they were passed in
+                            the command line, with precedence being given to
+diff --git a/man/landscape-sysinfo.1 b/man/landscape-sysinfo.1
+index e8dd2ff4..90852975 100644
+--- a/man/landscape-sysinfo.1
++++ b/man/landscape-sysinfo.1
+@@ -1,5 +1,5 @@
+ .\" Text automatically generated by txt2man
+-.TH landscape-sysinfo 1 "07 March 2018" "" ""
++.TH landscape-sysinfo 1 "14 February 2019" "" ""
+ .SH NAME
+ \fBlandscape-sysinfo \fP- Display a summary of the current system status
+ \fB
+