Merge ~paul-meyer/cloud-init:lp1722959 into cloud-init:master

[Paul Meyer]

Add Hyper-V KVP reporter

Add a reporter that reports events to a Hyper-V host using Key-Value-Pair
exchange protocol and can be used to obtain high level diagnostic
information from the host.

To use this facility, the KVP user-space daemon (hv_kvp_daemon) has to be
running. It reads the kvpFile when the host requests the guest to
enumerate the KVP's.

This reporter collates all events for a module (origin|name) in a single
json string in the dictionary.

For more information, see
https://technet.microsoft.com/en-us/library/dn798287.aspx#Linux%20guests

Add a reporter that reports events to a Hyper-V host using Key-Value-Pair
exchange protocol and can be used to obtain high level diagnostic
information from the host.

To use this facility, the KVP user-space daemon (hv_kvp_daemon) has to be
running. It reads the kvpFile when the host requests the guest to
enumerate the KVP's.

This reporter collates all events for a module (origin|name) in a single
json string in the dictionary.

For more information, see
https://technet.microsoft.com/en-us/library/dn798287.aspx#Linux%20guests

[Scott Moser (smoser)]

So would i expect to find a subsequent merge proposal that would build in configuration of this reporter into Azure datasource?

e7cca41... by Paul Meyer on 2017-12-08

Add test intent docstrings, import mock from helpers

[Paul Meyer (paul-meyer)]

> So would i expect to find a subsequent merge proposal that would build in
> configuration of this reporter into Azure datasource?

Actually, I've been thinking that we may want to move most of that built-in config to files on disk in the images themselves, such that they can be properly overridden by custom images. Also, for this reporter it makes sense to have it on even while cloud-init searches for a data source, that way, we can detect if DSAzure is broken if the VM doesn't come online.

[Scott Moser (smoser)]

> > So would i expect to find a subsequent merge proposal that would build in
> > configuration of this reporter into Azure datasource?
>
> Actually, I've been thinking that we may want to move most of that built-in
> config to files on disk in the images themselves, such that they can be
> properly overridden by custom images. Also, for this reporter it makes sense
> to have it on even while cloud-init searches for a data source, that way, we
> can detect if DSAzure is broken if the VM doesn't come online.

Generally speaking we're after the goal that installing cloud-init in an image "does the right thing". That is as opposed to requiring Canonical or anyone building images to "know" that they should put some configuration files down in /etc/cloud/cloud.cfg.d.

I do appreciate that you'd like information to be posted when cloud-init is searching for easier debugging, but such changes wont work or cause issue on other platforms. We like to allow "one image" to work on different platforms.

[Paul Meyer (paul-meyer)]

> > > So would i expect to find a subsequent merge proposal that would build in
> > > configuration of this reporter into Azure datasource?
> >
> > Actually, I've been thinking that we may want to move most of that built-in
> > config to files on disk in the images themselves, such that they can be
> > properly overridden by custom images. Also, for this reporter it makes sense
> > to have it on even while cloud-init searches for a data source, that way, we
> > can detect if DSAzure is broken if the VM doesn't come online.
>
> Generally speaking we're after the goal that installing cloud-init in an image
> "does the right thing". That is as opposed to requiring Canonical or anyone
> building images to "know" that they should put some configuration files down
> in /etc/cloud/cloud.cfg.d.
>
> I do appreciate that you'd like information to be posted when cloud-init is
> searching for easier debugging, but such changes wont work or cause issue on
> other platforms. We like to allow "one image" to work on different platforms.

I understand the concern and I agree. We'll do both then. Add this as default configuration from the Azure DS and also make the defaults overridable from /etc/cloud for custom images (lp 1735008).

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:e7cca41b254f76af884c14c5aa6d0b3387f6546d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/616/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/616/rebuild

review: Approve (continuous-integration)

Unmerged commits

e7cca41... by Paul Meyer on 2017-12-08

Add test intent docstrings, import mock from helpers

1e4cc7d... by Paul Meyer on 2017-05-17

Add Hyper-V KVP reporter

diff --git a/cloudinit/reporting/handlers.py b/cloudinit/reporting/handlers.py
index 4066076..6c89616 100644
--- a/cloudinit/reporting/handlers.py
+++ b/cloudinit/reporting/handlers.py
@@ -1,13 +1,16 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
 import abc
+import contextlib
+import fcntl
 import json
 import six
+import struct
 
 from cloudinit import log as logging
 from cloudinit.registry import DictRegistry
 from cloudinit import (url_helper, util)
-
+from datetime import datetime
 
 LOG = logging.getLogger(__name__)
 
@@ -85,9 +88,100 @@ class WebHookHandler(ReportingHandler):
             LOG.warning("failed posting event: %s", event.as_string())
 
 
+HV_KVP_EXCHANGE_MAX_VALUE_SIZE = 2048
+HV_KVP_EXCHANGE_MAX_KEY_SIZE = 512
+HV_KVP_RECORD_SIZE = (HV_KVP_EXCHANGE_MAX_KEY_SIZE +
+                      HV_KVP_EXCHANGE_MAX_VALUE_SIZE)
+
+
+class HyperVKvpReportingHandler(ReportingHandler):
+    """
+    Reports events to a Hyper-V host using Key-Value-Pair exchange protocol
+    and can be used to obtain high level diagnostic information from the host.
+
+    To use this facility, the KVP user-space daemon (hv_kvp_daemon) has to be
+    running. It reads the kvpFile when the host requests the guest to
+    enumerate the KVP's.
+
+    This reporter collates all events for a module (origin|name) in a single
+    json string in the dictionary.
+
+    For more information, see
+    https://technet.microsoft.com/en-us/library/dn798287.aspx#Linux%20guests
+    """
+
+    def __init__(self,
+                 kvpFile='/var/lib/hyperv/.kvp_pool_1',
+                 event_types=None):
+        super(HyperVKvpReportingHandler, self).__init__()
+        self._kvpFile = kvpFile
+        self._event_types = event_types
+
+    @staticmethod
+    def _decodeKVP(data):
+        kvp = dict()
+
+        num = int(len(data) / (HV_KVP_RECORD_SIZE))
+        for i in range(num):
+            k = (data[HV_KVP_RECORD_SIZE * i:]
+                 [0:HV_KVP_EXCHANGE_MAX_KEY_SIZE].decode('utf-8')
+                 .strip('\x00'))
+            v = (data[HV_KVP_RECORD_SIZE * i + HV_KVP_EXCHANGE_MAX_KEY_SIZE:]
+                 [0:HV_KVP_EXCHANGE_MAX_VALUE_SIZE].decode('utf-8')
+                 .strip('\x00'))
+            kvp[k] = v
+
+        return kvp
+
+    @staticmethod
+    def _encodeKVP(kvp):
+        data = bytes()
+        for k, v in kvp.items():
+            data += (struct.pack("%ds%ds" %
+                     (HV_KVP_EXCHANGE_MAX_KEY_SIZE,
+                      HV_KVP_EXCHANGE_MAX_VALUE_SIZE),
+                     k.encode('utf-8'), v.encode('utf-8')))
+        return data
+
+    @contextlib.contextmanager
+    def _kvps(self):
+        """Add a key/value pair to the kvp file."""
+        with open(self._kvpFile, 'rb+') as f:
+            fcntl.flock(f, fcntl.LOCK_EX)  # lock will be release when f closes
+            kvp = self._decodeKVP(f.read())
+            yield kvp
+            data = self._encodeKVP(kvp)
+            f.seek(0)
+            f.truncate(0)
+            f.write(data)
+
+    def publish_event(self, event):
+        if (not self._event_types or event.event_type in self._event_types):
+            with self._kvps() as kvp:
+                k = "{0}|{1}".format(event.origin, event.name)
+                v = kvp.get(k)
+                if v is not None:
+                    v = json.loads(v)
+                else:
+                    v = []
+
+                data = {
+                    "type": event.event_type,
+                    "ts": (datetime.utcfromtimestamp(event.timestamp)
+                                   .isoformat() + 'Z'),
+                    "msg": event.description}
+
+                if hasattr(event, 'result'):
+                    data['result'] = event.result
+
+                v.append(data)
+                kvp[k] = json.dumps(v, separators=(',', ':'))
+
+
 available_handlers = DictRegistry()
 available_handlers.register_item('log', LogHandler)
 available_handlers.register_item('print', PrintHandler)
 available_handlers.register_item('webhook', WebHookHandler)
+available_handlers.register_item('hyperv', HyperVKvpReportingHandler)
 
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_reporting_hyperv.py b/tests/unittests/test_reporting_hyperv.py
new file mode 100644
index 0000000..38d5ebe
--- /dev/null
+++ b/tests/unittests/test_reporting_hyperv.py
@@ -0,0 +1,97 @@
+# Copyright 2015 Canonical Ltd.
+#
+# This file is part of cloud-init. See LICENSE file for license information.
+
+from cloudinit.reporting import events
+from cloudinit.reporting import handlers
+
+from cloudinit.tests.helpers import TestCase, mock
+
+
+class TextKvpReporter(TestCase):
+
+    def test_encode_decode(self):
+        """Binary encoding of KVP yields expected size and is reversible.
+
+        See https://technet.microsoft.com/en-us/library/dn798287
+        """
+        kvp = {'key1': 'value1', 'key2': 'value2'}
+        data = handlers.HyperVKvpReportingHandler._encodeKVP(kvp)
+        self.assertEqual(len(data), handlers.HV_KVP_RECORD_SIZE * len(kvp))
+        decodedkvp = handlers.HyperVKvpReportingHandler._decodeKVP(data)
+        self.assertEqual(kvp, decodedkvp)
+
+    def test_event_type_can_be_filtered(self):
+        """HyperVKvpReportingHandler only reports events specified in constructor.
+
+        This can be used to limit what event_types are reported."""
+        reporter = handlers.HyperVKvpReportingHandler(
+            event_types=['foo', 'bar'])
+
+        kvps = dict()
+        mock_kvps = mock.Mock()
+        mock_kvps.__enter__ = mock.Mock(return_value=kvps)
+        mock_kvps.__exit__ = mock.Mock(return_value=False)
+
+        with mock.patch.object(reporter, '_kvps', return_value=mock_kvps):
+            reporter.publish_event(
+                events.ReportingEvent('foo', 'name', 'description'))
+            reporter.publish_event(
+                events.ReportingEvent('bar', 'name', 'description2'))
+            reporter.publish_event(
+                events.ReportingEvent('some_other', 'name', 'description3'))
+
+            print("kvps: " + str(kvps))
+
+            self.assertEqual(1, len(kvps))
+            v = list(kvps.values())[0]
+            self.assertIn('foo', v)
+            self.assertIn('bar', v)
+            self.assertNotIn('some_other', v)
+
+    def test_events_are_grouped_by_source_and_name(self):
+        """HyperVKvpReportingHandler collates events by origin|name.
+
+        This keeps the number of KVPs relatively small as such works
+        around a bug in the KVP daemon."""
+        reporter = handlers.HyperVKvpReportingHandler()
+
+        kvps = dict()
+        mock_kvps = mock.Mock()
+        mock_kvps.__enter__ = mock.Mock(return_value=kvps)
+        mock_kvps.__exit__ = mock.Mock(return_value=False)
+
+        with mock.patch.object(reporter, '_kvps', return_value=mock_kvps):
+            self.assertEqual(0, len(kvps))
+
+            reporter.publish_event(
+                events.ReportingEvent('foo', 'name1', 'description'))
+            reporter.publish_event(
+                events.ReportingEvent('bar', 'name1', 'description'))
+            self.assertEqual(1, len(kvps))
+
+            reporter.publish_event(
+                events.ReportingEvent('foo', 'name2', 'description'))
+            self.assertEqual(2, len(kvps))
+
+            reporter.publish_event(
+                events.ReportingEvent('foo', 'name1', 'description',
+                                      origin="origin2"))
+            self.assertEqual(3, len(kvps))
+
+    def test_finish_event_result_is_logged(self):
+        """The result field is logged when available on the event."""
+        reporter = handlers.HyperVKvpReportingHandler()
+
+        kvps = dict()
+        mock_kvps = mock.Mock()
+        mock_kvps.__enter__ = mock.Mock(return_value=kvps)
+        mock_kvps.__exit__ = mock.Mock(return_value=False)
+
+        with mock.patch.object(reporter, '_kvps', return_value=mock_kvps):
+            reporter.publish_event(
+                events.FinishReportingEvent('name2', 'description1',
+                                            result=events.status.FAIL))
+            self.assertIn('FAIL', list(kvps.values())[0])
+
+# vi: ts=4 expandtab