The collector would fail if it could not write a timestamp to the hea…

…lth_check.txt file, e.g. due to a permission error. Fixed by writing the health_check.txt file to /tmp instead of the home directory of the default user and by catching and logging any OS errors that may occur. Fixes #1057.

components/collector/Dockerfile

@@ -12,6 +12,6 @@ USER collector
 
 COPY src /home/collector
 
-HEALTHCHECK CMD python -c "from datetime import datetime as dt, timedelta; import sys; sys.exit(dt.now() - dt.fromisoformat(open('health_check.txt').read().strip()) > timedelta(seconds=600))"
+HEALTHCHECK CMD python -c "from datetime import datetime as dt, timedelta; import sys; sys.exit(dt.now() - dt.fromisoformat(open('/tmp/health_check.txt').read().strip()) > timedelta(seconds=600))"
 
 CMD ["python", "/home/collector/quality_time_collector.py"]

components/collector/src/metric_collectors/metrics_collector.py

@@ -39,10 +39,13 @@ def __init__(self) -> None:
         self.last_parameters: Dict[str, Any] = dict()
 
     @staticmethod
-    def record_health() -> None:
+    def record_health(filename: str = "/tmp/health_check.txt") -> None:
         """Record the current date and time in a file to allow for health checks."""
-        with open("health_check.txt", "w") as health_check:
-            health_check.write(datetime.now().isoformat())
+        try:
+            with open(filename, "w") as health_check:
+                health_check.write(datetime.now().isoformat())
+        except OSError as reason:
+            logging.error("Could not write health check time stamp to %s: %s", filename, reason)
 
     def start(self) -> NoReturn:
         """Start fetching measurements indefinitely."""

components/collector/tests/metric_collectors/test_metrics_collector.py

@@ -2,8 +2,9 @@
 
 import logging
 import unittest
+from datetime import datetime
 from typing import Tuple
-from unittest.mock import patch, Mock
+from unittest.mock import patch, mock_open, Mock
 
 import quality_time_collector
 from metric_collectors import MetricsCollector
@@ -33,102 +34,126 @@ def _parse_source_responses(self, responses: Responses) -> Tuple[Value, Value, E
     def tearDown(self):
         logging.disable(logging.NOTSET)
 
-    def test_fetch_without_sources(self):
+    @patch("requests.post")
+    @patch("requests.get")
+    def test_fetch_without_sources(self, mocked_get, mocked_post):
         """Test fetching measurement for a metric without sources."""
         self.metrics_response.json.return_value = dict(
             metric_uuid=dict(type="metric", addition="sum", sources=dict()))
-        with patch("requests.get", return_value=self.metrics_response):
-            with patch("requests.post") as post:
-                self.metrics_collector.fetch_measurements(60)
-        post.assert_not_called()
+        mocked_get.return_value = self.metrics_response
+        self.metrics_collector.fetch_measurements(60)
+        mocked_post.assert_not_called()
 
-    def test_fetch_with_get_error(self):
+    @patch("requests.post")
+    @patch("requests.get", Mock(side_effect=RuntimeError))
+    def test_fetch_with_get_error(self, mocked_post):
         """Test fetching measurement when getting fails."""
-        with patch("requests.get", side_effect=RuntimeError):
-            with patch("requests.post") as post:
-                self.metrics_collector.fetch_measurements(60)
-        post.assert_not_called()
+        self.metrics_collector.fetch_measurements(60)
+        mocked_post.assert_not_called()
 
-    def test_fetch_with_post_error(self):
+    @patch("requests.post", side_effect=RuntimeError)
+    @patch("requests.get")
+    def test_fetch_with_post_error(self, mocked_get, mocked_post):
         """Test fetching measurement when posting fails."""
         self.metrics_response.json.return_value = dict(
             metric_uuid=dict(
                 addition="sum", type="metric",
                 sources=dict(source_id=dict(type="source", parameters=dict(url=self.url)))))
-
-        with patch("requests.get", side_effect=[self.metrics_response, Mock()]):
-            with patch("requests.post", side_effect=RuntimeError) as post:
-                self.metrics_collector.data_model = self.data_model
-                self.metrics_collector.fetch_measurements(60)
-        post.assert_called_once_with(
+        mocked_get.side_effect = [self.metrics_response, Mock()]
+        self.metrics_collector.data_model = self.data_model
+        self.metrics_collector.fetch_measurements(60)
+        mocked_post.assert_called_once_with(
             self.measurement_api_url,
             json=dict(
                 sources=[
                     dict(api_url=self.url, landing_url=self.url, value="42", total="84", entities=[],
                          connection_error=None, parse_error=None, source_uuid="source_id")],
                 metric_uuid="metric_uuid"))
 
-    def test_collect(self):
+    @patch("builtins.open", mock_open())
+    @patch("time.sleep", Mock(side_effect=RuntimeError))
+    @patch("requests.post")
+    @patch("requests.get")
+    def test_collect(self, mocked_get, mocked_post):
         """Test the collect method."""
         self.metrics_response.json.return_value = dict(
             metric_uuid=dict(
                 addition="sum", type="metric",
                 sources=dict(source_id=dict(type="source", parameters=dict(url=self.url)))))
-        with patch("builtins.open"):  # To prevent writing the health check date/time file
-            with patch("requests.get", side_effect=[self.data_model_response, self.metrics_response, Mock()]):
-                with patch("requests.post") as post:
-                    with patch("time.sleep", side_effect=[RuntimeError]):
-                        self.assertRaises(RuntimeError, quality_time_collector.collect)
-        post.assert_called_once_with(
+        mocked_get.side_effect = [self.data_model_response, self.metrics_response, Mock()]
+        self.assertRaises(RuntimeError, quality_time_collector.collect)
+        mocked_post.assert_called_once_with(
             self.measurement_api_url,
             json=dict(
                 sources=[
                     dict(api_url=self.url, landing_url=self.url, value="42", total="84", entities=[],
                          connection_error=None, parse_error=None, source_uuid="source_id")],
                 metric_uuid="metric_uuid"))
 
-    def test_missing_collector(self):
+    @patch("requests.get")
+    def test_missing_collector(self, mocked_get):
         """Test that an exception is thrown if there's no collector for the source and metric type."""
         self.metrics_response.json.return_value = dict(
             metric_uuid=dict(
                 type="metric", addition="sum",
                 sources=dict(missing=dict(type="unknown_source", parameters=dict(url=self.url)))))
-        with patch("requests.get", return_value=self.metrics_response):
-            self.assertRaises(LookupError, self.metrics_collector.fetch_measurements, 60)
+        mocked_get.return_value = self.metrics_response
+        self.assertRaises(LookupError, self.metrics_collector.fetch_measurements, 60)
 
-    def test_fetch_twice(self):
+    @patch("requests.post")
+    @patch("requests.get")
+    def test_fetch_twice(self, mocked_get, mocked_post):
         """Test that the metric is skipped on the second fetch."""
         self.metrics_response.json.return_value = dict(
             metric_uuid=dict(
                 addition="sum", type="metric",
                 sources=dict(source_id=dict(type="source", parameters=dict(url=self.url)))))
-        with patch("requests.get", side_effect=[self.metrics_response, Mock(), self.metrics_response]):
-            with patch("requests.post") as post:
-                self.metrics_collector.data_model = self.data_model
-                self.metrics_collector.fetch_measurements(60)
-                self.metrics_collector.fetch_measurements(60)
-        post.assert_called_once_with(
+        mocked_get.side_effect = [self.metrics_response, Mock(), self.metrics_response]
+        self.metrics_collector.data_model = self.data_model
+        self.metrics_collector.fetch_measurements(60)
+        self.metrics_collector.fetch_measurements(60)
+        mocked_post.assert_called_once_with(
             "http://localhost:5001/api/v2/measurements",
             json=dict(
                 sources=[
                     dict(api_url=self.url, landing_url=self.url, value="42", total="84", entities=[],
                          connection_error=None, parse_error=None, source_uuid="source_id")],
                 metric_uuid="metric_uuid"))
 
-    def test_missing_mandatory_parameter(self):
+    @patch("requests.post")
+    @patch("requests.get")
+    def test_missing_mandatory_parameter(self, mocked_get, mocked_post):
         """Test that a metric with sources but without a mandatory parameter is skipped."""
         self.metrics_response.json.return_value = dict(
             metric_uuid=dict(
                 type="metric", addition="sum", sources=dict(missing=dict(type="source", parameters=dict(url="")))))
-        with patch("requests.get", return_value=self.metrics_response):
-            with patch("requests.post") as post:
-                self.metrics_collector.data_model = self.data_model
-                self.metrics_collector.fetch_measurements(60)
-        post.assert_not_called()
-
-    def test_fetch_data_model_after_failure(self):
-        """Test that that the data model is fetched on the second try."""
-        with patch("builtins.open"):  # To prevent writing the health check date/time file
-            with patch("requests.get", side_effect=[None, self.data_model_response]):
-                data_model = self.metrics_collector.fetch_data_model(0)
+        mocked_get.return_value = self.metrics_response
+        self.metrics_collector.data_model = self.data_model
+        self.metrics_collector.fetch_measurements(60)
+        mocked_post.assert_not_called()
+
+    @patch("builtins.open", mock_open())
+    @patch("requests.get")
+    def test_fetch_data_model_after_failure(self, mocked_get):
+        """Test that the data model is fetched on the second try."""
+        mocked_get.side_effect = [None, self.data_model_response]
+        data_model = self.metrics_collector.fetch_data_model(0)
         self.assertEqual(self.data_model, data_model)
+
+    @patch("builtins.open", new_callable=mock_open)
+    @patch("metric_collectors.metrics_collector.datetime")
+    def test_writing_health_check(self, mocked_datetime, mocked_open):
+        """Test that the current time is written to the health check file."""
+        mocked_datetime.now.return_value = now = datetime.now()
+        self.metrics_collector.record_health()
+        mocked_open.assert_called_once_with("/tmp/health_check.txt", "w")
+        mocked_open().write.assert_called_once_with(now.isoformat())
+
+    @patch("builtins.open")
+    @patch("logging.error")
+    def test_fail_writing_health_check(self, mocked_log, mocked_open):
+        """Test that a failure to open the health check file is logged, but otherwise ignored."""
+        mocked_open.side_effect = io_error = OSError("Some error")
+        self.metrics_collector.record_health()
+        mocked_log.assert_called_once_with(
+            "Could not write health check time stamp to %s: %s", "/tmp/health_check.txt", io_error)

docs/CHANGELOG.md

@@ -14,7 +14,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Fixed
 
 - When using SonarQube as source for duplication, uncovered lines, or uncovered branches, the landing url would be incorrect. Fixes [#1044](https://github.com/ICTU/quality-time/issues/1044).
-- Proxy container now waits for server and frontend containers to start. Fixes [#1046](https://github.com/ICTU/quality-time/issues/1046).
+- The docker-compose YAML file now specifies that the proxy container should wait for the server and frontend containers to start. Fixes [#1046](https://github.com/ICTU/quality-time/issues/1046).
+- The collector would fail if it could not write a timestamp to the health_check.txt file, e.g. due to a permission error. Fixed by writing the health_check.txt file to /tmp instead of the home directory of the default user and by catching and logging any OS errors that may occur. Fixes [#1057](https://github.com/ICTU/quality-time/issues/1057).
 
 ## [1.7.0] - [2020-02-22]