sailesh1993 / rpms / cloud-init

Forked from rpms/cloud-init a year ago
Clone
738010
From a4b4a11f904d7f70b53c7959e489d7aab72a9fa4 Mon Sep 17 00:00:00 2001
738010
From: Eduardo Otubo <otubo@redhat.com>
738010
Date: Wed, 15 May 2019 12:15:27 +0200
738010
Subject: [PATCH 3/5] Azure: Changes to the Hyper-V KVP Reporter
738010
MIME-Version: 1.0
738010
Content-Type: text/plain; charset=UTF-8
738010
Content-Transfer-Encoding: 8bit
738010
738010
RH-Author: Eduardo Otubo <otubo@redhat.com>
738010
Message-id: <20190515121529.11191-4-otubo@redhat.com>
738010
Patchwork-id: 87885
738010
O-Subject: [rhel-7 cloud-init PATCHv2 3/5] Azure: Changes to the Hyper-V KVP Reporter
738010
Bugzilla: 1687565
738010
RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>
738010
RH-Acked-by: Mohammed Gamal <mgamal@redhat.com>
738010
738010
From: Anh Vo <anhvo@microsoft.com>
738010
738010
BZ: 1687565
738010
BRANCH: rhel7/master-18.5
738010
UPSTREAM: 86674f01
738010
BREW: 21696239
738010
738010
commit 86674f013dfcea3c075ab41373ffb475881066f6
738010
Author: Anh Vo <anhvo@microsoft.com>
738010
Date:   Mon Apr 29 20:22:16 2019 +0000
738010
738010
    Azure: Changes to the Hyper-V KVP Reporter
738010
738010
     + Truncate KVP Pool file to prevent stale entries from
738010
       being processed by the Hyper-V KVP reporter.
738010
     + Drop filtering of KVPs as it is no longer needed.
738010
     + Batch appending of existing KVP entries.
738010
738010
Signed-off-by: Eduardo Otubo <otubo@redhat.com>
738010
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
738010
---
738010
 cloudinit/reporting/handlers.py          | 117 +++++++++++++++----------------
738010
 tests/unittests/test_reporting_hyperv.py | 104 +++++++++++++--------------
738010
 2 files changed, 106 insertions(+), 115 deletions(-)
738010
 mode change 100644 => 100755 cloudinit/reporting/handlers.py
738010
 mode change 100644 => 100755 tests/unittests/test_reporting_hyperv.py
738010
738010
diff --git a/cloudinit/reporting/handlers.py b/cloudinit/reporting/handlers.py
738010
old mode 100644
738010
new mode 100755
738010
index 6d23558..10165ae
738010
--- a/cloudinit/reporting/handlers.py
738010
+++ b/cloudinit/reporting/handlers.py
738010
@@ -5,7 +5,6 @@ import fcntl
738010
 import json
738010
 import six
738010
 import os
738010
-import re
738010
 import struct
738010
 import threading
738010
 import time
738010
@@ -14,6 +13,7 @@ from cloudinit import log as logging
738010
 from cloudinit.registry import DictRegistry
738010
 from cloudinit import (url_helper, util)
738010
 from datetime import datetime
738010
+from six.moves.queue import Empty as QueueEmptyError
738010
 
738010
 if six.PY2:
738010
     from multiprocessing.queues import JoinableQueue as JQueue
738010
@@ -129,24 +129,50 @@ class HyperVKvpReportingHandler(ReportingHandler):
738010
     DESC_IDX_KEY = 'msg_i'
738010
     JSON_SEPARATORS = (',', ':')
738010
     KVP_POOL_FILE_GUEST = '/var/lib/hyperv/.kvp_pool_1'
738010
+    _already_truncated_pool_file = False
738010
 
738010
     def __init__(self,
738010
                  kvp_file_path=KVP_POOL_FILE_GUEST,
738010
                  event_types=None):
738010
         super(HyperVKvpReportingHandler, self).__init__()
738010
         self._kvp_file_path = kvp_file_path
738010
+        HyperVKvpReportingHandler._truncate_guest_pool_file(
738010
+            self._kvp_file_path)
738010
+
738010
         self._event_types = event_types
738010
         self.q = JQueue()
738010
-        self.kvp_file = None
738010
         self.incarnation_no = self._get_incarnation_no()
738010
         self.event_key_prefix = u"{0}|{1}".format(self.EVENT_PREFIX,
738010
                                                   self.incarnation_no)
738010
-        self._current_offset = 0
738010
         self.publish_thread = threading.Thread(
738010
                 target=self._publish_event_routine)
738010
         self.publish_thread.daemon = True
738010
         self.publish_thread.start()
738010
 
738010
+    @classmethod
738010
+    def _truncate_guest_pool_file(cls, kvp_file):
738010
+        """
738010
+        Truncate the pool file if it has not been truncated since boot.
738010
+        This should be done exactly once for the file indicated by
738010
+        KVP_POOL_FILE_GUEST constant above. This method takes a filename
738010
+        so that we can use an arbitrary file during unit testing.
738010
+        Since KVP is a best-effort telemetry channel we only attempt to
738010
+        truncate the file once and only if the file has not been modified
738010
+        since boot. Additional truncation can lead to loss of existing
738010
+        KVPs.
738010
+        """
738010
+        if cls._already_truncated_pool_file:
738010
+            return
738010
+        boot_time = time.time() - float(util.uptime())
738010
+        try:
738010
+            if os.path.getmtime(kvp_file) < boot_time:
738010
+                with open(kvp_file, "w"):
738010
+                    pass
738010
+        except (OSError, IOError) as e:
738010
+            LOG.warning("failed to truncate kvp pool file, %s", e)
738010
+        finally:
738010
+            cls._already_truncated_pool_file = True
738010
+
738010
     def _get_incarnation_no(self):
738010
         """
738010
         use the time passed as the incarnation number.
738010
@@ -162,20 +188,15 @@ class HyperVKvpReportingHandler(ReportingHandler):
738010
 
738010
     def _iterate_kvps(self, offset):
738010
         """iterate the kvp file from the current offset."""
738010
-        try:
738010
-            with open(self._kvp_file_path, 'rb+') as f:
738010
-                self.kvp_file = f
738010
-                fcntl.flock(f, fcntl.LOCK_EX)
738010
-                f.seek(offset)
738010
+        with open(self._kvp_file_path, 'rb') as f:
738010
+            fcntl.flock(f, fcntl.LOCK_EX)
738010
+            f.seek(offset)
738010
+            record_data = f.read(self.HV_KVP_RECORD_SIZE)
738010
+            while len(record_data) == self.HV_KVP_RECORD_SIZE:
738010
+                kvp_item = self._decode_kvp_item(record_data)
738010
+                yield kvp_item
738010
                 record_data = f.read(self.HV_KVP_RECORD_SIZE)
738010
-                while len(record_data) == self.HV_KVP_RECORD_SIZE:
738010
-                    self._current_offset += self.HV_KVP_RECORD_SIZE
738010
-                    kvp_item = self._decode_kvp_item(record_data)
738010
-                    yield kvp_item
738010
-                    record_data = f.read(self.HV_KVP_RECORD_SIZE)
738010
-                fcntl.flock(f, fcntl.LOCK_UN)
738010
-        finally:
738010
-            self.kvp_file = None
738010
+            fcntl.flock(f, fcntl.LOCK_UN)
738010
 
738010
     def _event_key(self, event):
738010
         """
738010
@@ -207,23 +228,13 @@ class HyperVKvpReportingHandler(ReportingHandler):
738010
 
738010
         return {'key': k, 'value': v}
738010
 
738010
-    def _update_kvp_item(self, record_data):
738010
-        if self.kvp_file is None:
738010
-            raise ReportException(
738010
-                "kvp file '{0}' not opened."
738010
-                .format(self._kvp_file_path))
738010
-        self.kvp_file.seek(-self.HV_KVP_RECORD_SIZE, 1)
738010
-        self.kvp_file.write(record_data)
738010
-
738010
     def _append_kvp_item(self, record_data):
738010
-        with open(self._kvp_file_path, 'rb+') as f:
738010
+        with open(self._kvp_file_path, 'ab') as f:
738010
             fcntl.flock(f, fcntl.LOCK_EX)
738010
-            # seek to end of the file
738010
-            f.seek(0, 2)
738010
-            f.write(record_data)
738010
+            for data in record_data:
738010
+                f.write(data)
738010
             f.flush()
738010
             fcntl.flock(f, fcntl.LOCK_UN)
738010
-            self._current_offset = f.tell()
738010
 
738010
     def _break_down(self, key, meta_data, description):
738010
         del meta_data[self.MSG_KEY]
738010
@@ -279,40 +290,26 @@ class HyperVKvpReportingHandler(ReportingHandler):
738010
 
738010
     def _publish_event_routine(self):
738010
         while True:
738010
+            items_from_queue = 0
738010
             try:
738010
                 event = self.q.get(block=True)
738010
-                need_append = True
738010
+                items_from_queue += 1
738010
+                encoded_data = []
738010
+                while event is not None:
738010
+                    encoded_data += self._encode_event(event)
738010
+                    try:
738010
+                        # get all the rest of the events in the queue
738010
+                        event = self.q.get(block=False)
738010
+                        items_from_queue += 1
738010
+                    except QueueEmptyError:
738010
+                        event = None
738010
                 try:
738010
-                    if not os.path.exists(self._kvp_file_path):
738010
-                        LOG.warning(
738010
-                            "skip writing events %s to %s. file not present.",
738010
-                            event.as_string(),
738010
-                            self._kvp_file_path)
738010
-                    encoded_event = self._encode_event(event)
738010
-                    # for each encoded_event
738010
-                    for encoded_data in (encoded_event):
738010
-                        for kvp in self._iterate_kvps(self._current_offset):
738010
-                            match = (
738010
-                                re.match(
738010
-                                    r"^{0}\|(\d+)\|.+"
738010
-                                    .format(self.EVENT_PREFIX),
738010
-                                    kvp['key']
738010
-                                ))
738010
-                            if match:
738010
-                                match_groups = match.groups(0)
738010
-                                if int(match_groups[0]) < self.incarnation_no:
738010
-                                    need_append = False
738010
-                                    self._update_kvp_item(encoded_data)
738010
-                                    continue
738010
-                        if need_append:
738010
-                            self._append_kvp_item(encoded_data)
738010
-                except IOError as e:
738010
-                    LOG.warning(
738010
-                        "failed posting event to kvp: %s e:%s",
738010
-                        event.as_string(), e)
738010
+                    self._append_kvp_item(encoded_data)
738010
+                except (OSError, IOError) as e:
738010
+                    LOG.warning("failed posting events to kvp, %s", e)
738010
                 finally:
738010
-                    self.q.task_done()
738010
-
738010
+                    for _ in range(items_from_queue):
738010
+                        self.q.task_done()
738010
             # when main process exits, q.get() will through EOFError
738010
             # indicating we should exit this thread.
738010
             except EOFError:
738010
@@ -322,7 +319,7 @@ class HyperVKvpReportingHandler(ReportingHandler):
738010
     # if the kvp pool already contains a chunk of data,
738010
     # so defer it to another thread.
738010
     def publish_event(self, event):
738010
-        if (not self._event_types or event.event_type in self._event_types):
738010
+        if not self._event_types or event.event_type in self._event_types:
738010
             self.q.put(event)
738010
 
738010
     def flush(self):
738010
diff --git a/tests/unittests/test_reporting_hyperv.py b/tests/unittests/test_reporting_hyperv.py
738010
old mode 100644
738010
new mode 100755
738010
index 2e64c6c..d01ed5b
738010
--- a/tests/unittests/test_reporting_hyperv.py
738010
+++ b/tests/unittests/test_reporting_hyperv.py
738010
@@ -1,10 +1,12 @@
738010
 # This file is part of cloud-init. See LICENSE file for license information.
738010
 
738010
 from cloudinit.reporting import events
738010
-from cloudinit.reporting import handlers
738010
+from cloudinit.reporting.handlers import HyperVKvpReportingHandler
738010
 
738010
 import json
738010
 import os
738010
+import struct
738010
+import time
738010
 
738010
 from cloudinit import util
738010
 from cloudinit.tests.helpers import CiTestCase
738010
@@ -13,7 +15,7 @@ from cloudinit.tests.helpers import CiTestCase
738010
 class TestKvpEncoding(CiTestCase):
738010
     def test_encode_decode(self):
738010
         kvp = {'key': 'key1', 'value': 'value1'}
738010
-        kvp_reporting = handlers.HyperVKvpReportingHandler()
738010
+        kvp_reporting = HyperVKvpReportingHandler()
738010
         data = kvp_reporting._encode_kvp_item(kvp['key'], kvp['value'])
738010
         self.assertEqual(len(data), kvp_reporting.HV_KVP_RECORD_SIZE)
738010
         decoded_kvp = kvp_reporting._decode_kvp_item(data)
738010
@@ -26,57 +28,9 @@ class TextKvpReporter(CiTestCase):
738010
         self.tmp_file_path = self.tmp_path('kvp_pool_file')
738010
         util.ensure_file(self.tmp_file_path)
738010
 
738010
-    def test_event_type_can_be_filtered(self):
738010
-        reporter = handlers.HyperVKvpReportingHandler(
738010
-            kvp_file_path=self.tmp_file_path,
738010
-            event_types=['foo', 'bar'])
738010
-
738010
-        reporter.publish_event(
738010
-            events.ReportingEvent('foo', 'name', 'description'))
738010
-        reporter.publish_event(
738010
-            events.ReportingEvent('some_other', 'name', 'description3'))
738010
-        reporter.q.join()
738010
-
738010
-        kvps = list(reporter._iterate_kvps(0))
738010
-        self.assertEqual(1, len(kvps))
738010
-
738010
-        reporter.publish_event(
738010
-            events.ReportingEvent('bar', 'name', 'description2'))
738010
-        reporter.q.join()
738010
-        kvps = list(reporter._iterate_kvps(0))
738010
-        self.assertEqual(2, len(kvps))
738010
-
738010
-        self.assertIn('foo', kvps[0]['key'])
738010
-        self.assertIn('bar', kvps[1]['key'])
738010
-        self.assertNotIn('some_other', kvps[0]['key'])
738010
-        self.assertNotIn('some_other', kvps[1]['key'])
738010
-
738010
-    def test_events_are_over_written(self):
738010
-        reporter = handlers.HyperVKvpReportingHandler(
738010
-            kvp_file_path=self.tmp_file_path)
738010
-
738010
-        self.assertEqual(0, len(list(reporter._iterate_kvps(0))))
738010
-
738010
-        reporter.publish_event(
738010
-            events.ReportingEvent('foo', 'name1', 'description'))
738010
-        reporter.publish_event(
738010
-            events.ReportingEvent('foo', 'name2', 'description'))
738010
-        reporter.q.join()
738010
-        self.assertEqual(2, len(list(reporter._iterate_kvps(0))))
738010
-
738010
-        reporter2 = handlers.HyperVKvpReportingHandler(
738010
-            kvp_file_path=self.tmp_file_path)
738010
-        reporter2.incarnation_no = reporter.incarnation_no + 1
738010
-        reporter2.publish_event(
738010
-            events.ReportingEvent('foo', 'name3', 'description'))
738010
-        reporter2.q.join()
738010
-
738010
-        self.assertEqual(2, len(list(reporter2._iterate_kvps(0))))
738010
-
738010
     def test_events_with_higher_incarnation_not_over_written(self):
738010
-        reporter = handlers.HyperVKvpReportingHandler(
738010
+        reporter = HyperVKvpReportingHandler(
738010
             kvp_file_path=self.tmp_file_path)
738010
-
738010
         self.assertEqual(0, len(list(reporter._iterate_kvps(0))))
738010
 
738010
         reporter.publish_event(
738010
@@ -86,7 +40,7 @@ class TextKvpReporter(CiTestCase):
738010
         reporter.q.join()
738010
         self.assertEqual(2, len(list(reporter._iterate_kvps(0))))
738010
 
738010
-        reporter3 = handlers.HyperVKvpReportingHandler(
738010
+        reporter3 = HyperVKvpReportingHandler(
738010
             kvp_file_path=self.tmp_file_path)
738010
         reporter3.incarnation_no = reporter.incarnation_no - 1
738010
         reporter3.publish_event(
738010
@@ -95,7 +49,7 @@ class TextKvpReporter(CiTestCase):
738010
         self.assertEqual(3, len(list(reporter3._iterate_kvps(0))))
738010
 
738010
     def test_finish_event_result_is_logged(self):
738010
-        reporter = handlers.HyperVKvpReportingHandler(
738010
+        reporter = HyperVKvpReportingHandler(
738010
             kvp_file_path=self.tmp_file_path)
738010
         reporter.publish_event(
738010
             events.FinishReportingEvent('name2', 'description1',
738010
@@ -105,7 +59,7 @@ class TextKvpReporter(CiTestCase):
738010
 
738010
     def test_file_operation_issue(self):
738010
         os.remove(self.tmp_file_path)
738010
-        reporter = handlers.HyperVKvpReportingHandler(
738010
+        reporter = HyperVKvpReportingHandler(
738010
             kvp_file_path=self.tmp_file_path)
738010
         reporter.publish_event(
738010
             events.FinishReportingEvent('name2', 'description1',
738010
@@ -113,7 +67,7 @@ class TextKvpReporter(CiTestCase):
738010
         reporter.q.join()
738010
 
738010
     def test_event_very_long(self):
738010
-        reporter = handlers.HyperVKvpReportingHandler(
738010
+        reporter = HyperVKvpReportingHandler(
738010
             kvp_file_path=self.tmp_file_path)
738010
         description = 'ab' * reporter.HV_KVP_EXCHANGE_MAX_VALUE_SIZE
738010
         long_event = events.FinishReportingEvent(
738010
@@ -132,3 +86,43 @@ class TextKvpReporter(CiTestCase):
738010
             self.assertEqual(msg_slice['msg_i'], i)
738010
             full_description += msg_slice['msg']
738010
         self.assertEqual(description, full_description)
738010
+
738010
+    def test_not_truncate_kvp_file_modified_after_boot(self):
738010
+        with open(self.tmp_file_path, "wb+") as f:
738010
+            kvp = {'key': 'key1', 'value': 'value1'}
738010
+            data = (struct.pack("%ds%ds" % (
738010
+                    HyperVKvpReportingHandler.HV_KVP_EXCHANGE_MAX_KEY_SIZE,
738010
+                    HyperVKvpReportingHandler.HV_KVP_EXCHANGE_MAX_VALUE_SIZE),
738010
+                    kvp['key'].encode('utf-8'), kvp['value'].encode('utf-8')))
738010
+            f.write(data)
738010
+        cur_time = time.time()
738010
+        os.utime(self.tmp_file_path, (cur_time, cur_time))
738010
+
738010
+        # reset this because the unit test framework
738010
+        # has already polluted the class variable
738010
+        HyperVKvpReportingHandler._already_truncated_pool_file = False
738010
+
738010
+        reporter = HyperVKvpReportingHandler(kvp_file_path=self.tmp_file_path)
738010
+        kvps = list(reporter._iterate_kvps(0))
738010
+        self.assertEqual(1, len(kvps))
738010
+
738010
+    def test_truncate_stale_kvp_file(self):
738010
+        with open(self.tmp_file_path, "wb+") as f:
738010
+            kvp = {'key': 'key1', 'value': 'value1'}
738010
+            data = (struct.pack("%ds%ds" % (
738010
+                HyperVKvpReportingHandler.HV_KVP_EXCHANGE_MAX_KEY_SIZE,
738010
+                HyperVKvpReportingHandler.HV_KVP_EXCHANGE_MAX_VALUE_SIZE),
738010
+                kvp['key'].encode('utf-8'), kvp['value'].encode('utf-8')))
738010
+            f.write(data)
738010
+
738010
+        # set the time ways back to make it look like
738010
+        # we had an old kvp file
738010
+        os.utime(self.tmp_file_path, (1000000, 1000000))
738010
+
738010
+        # reset this because the unit test framework
738010
+        # has already polluted the class variable
738010
+        HyperVKvpReportingHandler._already_truncated_pool_file = False
738010
+
738010
+        reporter = HyperVKvpReportingHandler(kvp_file_path=self.tmp_file_path)
738010
+        kvps = list(reporter._iterate_kvps(0))
738010
+        self.assertEqual(0, len(kvps))
738010
-- 
738010
1.8.3.1
738010