From e2c94870efdd78fc8698aca421970f9ea8fec4b5 Mon Sep 17 00:00:00 2001 From: Xavier Vello Date: Fri, 13 Apr 2018 13:58:59 +0200 Subject: [PATCH] fix + improve tests --- kubelet/datadog_checks/kubelet/__init__.py | 7 ++- kubelet/datadog_checks/kubelet/cadvisor.py | 4 +- kubelet/datadog_checks/kubelet/kubelet.py | 3 ++ kubelet/tests/test_cadvisor.py | 52 ++++++++++++++++++- .../tests/{test_filter.py => test_common.py} | 31 ++++++++--- kubelet/tests/test_kubelet.py | 38 -------------- 6 files changed, 85 insertions(+), 50 deletions(-) rename kubelet/tests/{test_filter.py => test_common.py} (65%) diff --git a/kubelet/datadog_checks/kubelet/__init__.py b/kubelet/datadog_checks/kubelet/__init__.py index 7a742b30a4918..856803ab1ca24 100644 --- a/kubelet/datadog_checks/kubelet/__init__.py +++ b/kubelet/datadog_checks/kubelet/__init__.py @@ -1,8 +1,11 @@ from .kubelet import KubeletCheck from .__about__ import __version__ -from .common import ContainerFilter +from .common import ContainerFilter, get_pod_by_uid, is_static_pending_pod + __all__ = [ 'KubeletCheck', '__version__', - 'ContainerFilter' + 'ContainerFilter', + 'get_pod_by_uid', + 'is_static_pending_pod' ] diff --git a/kubelet/datadog_checks/kubelet/cadvisor.py b/kubelet/datadog_checks/kubelet/cadvisor.py index 8d2e2b447c427..bc32c0ac6d418 100644 --- a/kubelet/datadog_checks/kubelet/cadvisor.py +++ b/kubelet/datadog_checks/kubelet/cadvisor.py @@ -169,6 +169,4 @@ def _update_container_metrics(self, instance, subcontainer): if is_pod and subcontainer.get("spec", {}).get("has_network"): net = stats['network'] - self.rate(NAMESPACE + '.network_errors', - sum(float(net[x]) for x in NET_ERRORS), - tags=tags) + self.rate(NAMESPACE + '.network_errors', sum(float(net[x]) for x in NET_ERRORS), tags=tags) diff --git a/kubelet/datadog_checks/kubelet/kubelet.py b/kubelet/datadog_checks/kubelet/kubelet.py index 8851f3f450edb..18635276e4e1e 100644 --- a/kubelet/datadog_checks/kubelet/kubelet.py +++ b/kubelet/datadog_checks/kubelet/kubelet.py @@ -129,6 +129,9 @@ def check(self, instance): else: # Prometheus self.process(self.metrics_url, send_histograms_buckets=send_buckets, instance=instance) + # Free up memory + self.pod_list = None + def perform_kubelet_query(self, url, verbose=True, timeout=10): """ Perform and return a GET request against kubelet. Support auth and TLS validation. diff --git a/kubelet/tests/test_cadvisor.py b/kubelet/tests/test_cadvisor.py index f73b6baab9d34..f0ba18e36e9b2 100644 --- a/kubelet/tests/test_cadvisor.py +++ b/kubelet/tests/test_cadvisor.py @@ -2,16 +2,31 @@ # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) import sys - +import json +import mock import pytest import requests_mock from requests.exceptions import HTTPError from datadog_checks.kubelet import KubeletCheck +from .test_kubelet import mock_from_file, EXPECTED_METRICS_COMMON, NODE_SPEC + # Skip the whole tests module on Windows pytestmark = pytest.mark.skipif(sys.platform == 'win32', reason='tests for linux only') +EXPECTED_METRICS_CADVISOR = [ + 'kubernetes.network_errors', + 'kubernetes.diskio.io_service_bytes.stats.total', +] + + +@pytest.fixture +def aggregator(): + from datadog_checks.stubs import aggregator + aggregator.reset() + return aggregator + @requests_mock.mock() def test_detect_cadvisor_nominal(m): @@ -32,3 +47,38 @@ def test_detect_cadvisor_port_zero(): with pytest.raises(ValueError): url = KubeletCheck.detect_cadvisor("http://kubelet:10250", 0) assert url == "" + + +def test_kubelet_check_cadvisor(monkeypatch, aggregator): + cadvisor_url = "http://valid:port/url" + check = KubeletCheck('kubelet', None, {}, [{}]) + monkeypatch.setattr(check, 'retrieve_pod_list', + mock.Mock(return_value=json.loads(mock_from_file('pods_list_1.2.json')))) + monkeypatch.setattr(check, 'retrieve_node_spec', mock.Mock(return_value=NODE_SPEC)) + monkeypatch.setattr(check, '_perform_kubelet_check', mock.Mock(return_value=None)) + monkeypatch.setattr(check, 'retrieve_cadvisor_metrics', + mock.Mock(return_value=json.loads(mock_from_file('cadvisor_1.2.json')))) + monkeypatch.setattr(check, 'process', mock.Mock(return_value=None)) + monkeypatch.setattr(check, 'detect_cadvisor', mock.Mock(return_value=cadvisor_url)) + + # We filter out slices unknown by the tagger, mock a non-empty taglist + monkeypatch.setattr('datadog_checks.kubelet.cadvisor.tags_for_docker', + mock.Mock(return_value=["foo:bar"])) + monkeypatch.setattr('datadog_checks.kubelet.cadvisor.tags_for_pod', + mock.Mock(return_value=["foo:bar"])) + + check.check({}) + assert check.cadvisor_legacy_url == cadvisor_url + check.retrieve_pod_list.assert_called_once() + check.retrieve_node_spec.assert_called_once() + check.retrieve_cadvisor_metrics.assert_called_once() + check._perform_kubelet_check.assert_called_once() + check.process.assert_not_called() + + # called twice so pct metrics are guaranteed to be there + check.check({}) + for metric in EXPECTED_METRICS_COMMON: + aggregator.assert_metric(metric) + for metric in EXPECTED_METRICS_CADVISOR: + aggregator.assert_metric(metric) + assert aggregator.metrics_asserted_pct == 100.0 diff --git a/kubelet/tests/test_filter.py b/kubelet/tests/test_common.py similarity index 65% rename from kubelet/tests/test_filter.py rename to kubelet/tests/test_common.py index bfd8876f6807f..212cb9763bb46 100644 --- a/kubelet/tests/test_filter.py +++ b/kubelet/tests/test_common.py @@ -8,7 +8,9 @@ import pytest import json -from datadog_checks.kubelet import ContainerFilter +from datadog_checks.kubelet import ContainerFilter, get_pod_by_uid, is_static_pending_pod + +from .test_kubelet import mock_from_file # Skip the whole tests module on Windows pytestmark = pytest.mark.skipif(sys.platform == 'win32', reason='tests for linux only') @@ -17,11 +19,6 @@ HERE = os.path.abspath(os.path.dirname(__file__)) -def mock_from_file(fname): - with open(os.path.join(HERE, 'fixtures', fname)) as f: - return f.read() - - def test_container_filter(monkeypatch): is_excluded = mock.Mock(return_value=False) monkeypatch.setattr('datadog_checks.kubelet.common.is_excluded', is_excluded) @@ -57,3 +54,25 @@ def test_container_filter(monkeypatch): assert filter.is_excluded(short_cid) is True is_excluded.assert_called_once() is_excluded.assert_called_with(ctr_name, ctr_image) + + +def test_pod_by_uid(): + podlist = json.loads(mock_from_file('pods.txt')) + + pod = get_pod_by_uid("260c2b1d43b094af6d6b4ccba082c2db", podlist) + assert pod is not None + assert pod["metadata"]["name"] == "kube-proxy-gke-haissam-default-pool-be5066f1-wnvn" + + +def test_is_static_pod(): + podlist = json.loads(mock_from_file('pods.txt')) + + # kube-proxy-gke-haissam-default-pool-be5066f1-wnvn is static + pod = get_pod_by_uid("260c2b1d43b094af6d6b4ccba082c2db", podlist) + assert pod is not None + assert is_static_pending_pod(pod) is True + + # fluentd-gcp-v2.0.10-9q9t4 is not static + pod = get_pod_by_uid("2edfd4d9-10ce-11e8-bd5a-42010af00137", podlist) + assert pod is not None + assert is_static_pending_pod(pod) is False diff --git a/kubelet/tests/test_kubelet.py b/kubelet/tests/test_kubelet.py index e77e732f8ed17..bf323fdb44c75 100644 --- a/kubelet/tests/test_kubelet.py +++ b/kubelet/tests/test_kubelet.py @@ -63,11 +63,6 @@ 'kubernetes.io.read_bytes' ] -EXPECTED_METRICS_CADVISOR = [ - 'kubernetes.network_errors', - 'kubernetes.diskio.io_service_bytes.stats.total', -] - Label = namedtuple('Label', 'name value') @@ -141,39 +136,6 @@ def test_kubelet_check_prometheus(monkeypatch, aggregator): assert aggregator.metrics_asserted_pct == 100.0 -def test_kubelet_check_cadvisor(monkeypatch, aggregator): - cadvisor_url = "http://valid:port/url" - check = KubeletCheck('kubelet', None, {}, [{}]) - monkeypatch.setattr(check, 'retrieve_pod_list', - mock.Mock(return_value=json.loads(mock_from_file('pods_list_1.2.json')))) - monkeypatch.setattr(check, 'retrieve_node_spec', mock.Mock(return_value=NODE_SPEC)) - monkeypatch.setattr(check, '_perform_kubelet_check', mock.Mock(return_value=None)) - monkeypatch.setattr(check, 'retrieve_cadvisor_metrics', - mock.Mock(return_value=json.loads(mock_from_file('cadvisor_1.2.json')))) - monkeypatch.setattr(check, 'process', mock.Mock(return_value=None)) - monkeypatch.setattr(check, 'detect_cadvisor', mock.Mock(return_value=cadvisor_url)) - - monkeypatch.setattr('datadog_checks.kubelet.cadvisor.tags_for_docker', - mock.Mock(return_value=["foo:bar"])) - - check.check({}) - - assert check.cadvisor_legacy_url == cadvisor_url - check.retrieve_pod_list.assert_called_once() - check.retrieve_node_spec.assert_called_once() - check.retrieve_cadvisor_metrics.assert_called_once() - check._perform_kubelet_check.assert_called_once() - check.process.assert_not_called() - - # called twice so pct metrics are guaranteed to be there - check.check({}) - for metric in EXPECTED_METRICS_COMMON: - aggregator.assert_metric(metric) - for metric in EXPECTED_METRICS_CADVISOR: - aggregator.assert_metric(metric) - assert aggregator.metrics_asserted_pct == 100.0 - - def test_is_container_metric(): check = KubeletCheck('kubelet', None, {}, [{}])