diff --git a/components/collector/Dockerfile b/components/collector/Dockerfile index 07c0806dab..7e37f5605e 100644 --- a/components/collector/Dockerfile +++ b/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"] diff --git a/components/collector/src/metric_collectors/metrics_collector.py b/components/collector/src/metric_collectors/metrics_collector.py index a8179c7661..a0424fceba 100644 --- a/components/collector/src/metric_collectors/metrics_collector.py +++ b/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.""" diff --git a/components/collector/tests/metric_collectors/test_metrics_collector.py b/components/collector/tests/metric_collectors/test_metrics_collector.py index dbf745e670..edc70912f3 100644 --- a/components/collector/tests/metric_collectors/test_metrics_collector.py +++ b/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,34 +34,35 @@ 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=[ @@ -68,18 +70,19 @@ def test_fetch_with_post_error(self): 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=[ @@ -87,27 +90,29 @@ def test_collect(self): 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=[ @@ -115,20 +120,40 @@ def test_fetch_twice(self): 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) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 45c5e641e4..80c0644882 100644 --- a/docs/CHANGELOG.md +++ b/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]