From 9acaf0d319c725d25fade5a25e368185b7a0abb3 Mon Sep 17 00:00:00 2001 From: Tristan Michelet Date: Thu, 24 Mar 2016 19:34:39 +0000 Subject: [PATCH] [Config] handle `.yaml.default` files review fix from #2273 --- config.py | 24 +++++----- tests/core/fixtures/checks/invalid_check_2.py | 2 +- tests/core/fixtures/checks/valid_conf_2.yaml | 5 +++ tests/core/test_config.py | 45 ++++++++++++++----- 4 files changed, 55 insertions(+), 21 deletions(-) create mode 100644 tests/core/fixtures/checks/valid_conf_2.yaml diff --git a/config.py b/config.py index 47c2c41c69..b1444acd1f 100644 --- a/config.py +++ b/config.py @@ -30,7 +30,6 @@ SubprocessOutputEmptyError, ) - # CONSTANTS AGENT_VERSION = "5.8.0" DATADOG_CONF = "datadog.conf" @@ -771,22 +770,31 @@ def _all_configs_paths(osname, agentConfig): """ try: confd_path = get_confd_path(osname) - all_configs = [glob.glob(os.path.join(confd_path, '*.yaml'))] + all_configs = glob.glob(os.path.join(confd_path, '*.yaml')) + all_default_configs = glob.glob(os.path.join(confd_path, '*.yaml.default')) except PathNotFound, e: log.error("No conf.d folder found at '%s' or in the directory where the Agent is currently deployed.\n" % e.args[0]) sys.exit(3) + if all_default_configs: + current_configs = set([_conf_path_to_check_name(conf) for conf in all_configs]) + for default_config in all_default_configs: + if not _conf_path_to_check_name(default_config) in current_configs: + all_configs.append(default_config) + # Compatibility code for the Nagios checks if it's still configured # in datadog.conf # FIXME: 6.x, should be removed if not any('nagios' in config for config in itertools.chain(*all_configs)): # check if it's configured in datadog.conf the old way if any([nagios_key in agentConfig for nagios_key in NAGIOS_OLD_CONF_KEYS]): - all_configs.append(['deprecated/nagios']) - + all_configs.append('deprecated/nagios') return all_configs +def _conf_path_to_check_name(conf_path): + return conf_path.rsplit('/', 1)[-1].split('.yaml')[0] + def _checks_places(agentConfig, osname): """ Return methods to generated paths to inspect for a check provided it's name """ @@ -822,10 +830,6 @@ def _validate_config(config_path, check_name, agentConfig): log.exception("Unable to parse yaml config in %s" % config_path) traceback_message = traceback.format_exc() return False, None, {check_name: {'error': str(e), 'traceback': traceback_message}} - # Look for the per-check config, which *must* exist - if not check_config.get('instances'): - log.error("Config %s is missing 'instances'" % config_path) - return False, None, {} return True, check_config, {} def _validate_check(check_name, check_path): @@ -903,9 +907,9 @@ def load_check_directory(agentConfig, hostname): checks_places = _checks_places(agentConfig, osname) - for config_path in itertools.chain(*all_configs_paths): + for config_path in all_configs_paths: # '/etc/dd-agent/checks.d/my_check.py' -> 'my_check' - check_name = config_path.rsplit('.', 1)[0].rsplit('/', 1)[-1] + check_name = _conf_path_to_check_name(config_path) conf_is_valid, check_config, invalid_check = _validate_config(config_path, check_name, agentConfig) init_failed_checks.update(invalid_check) diff --git a/tests/core/fixtures/checks/invalid_check_2.py b/tests/core/fixtures/checks/invalid_check_2.py index bf0c952941..6c7d3207ed 100644 --- a/tests/core/fixtures/checks/invalid_check_2.py +++ b/tests/core/fixtures/checks/invalid_check_2.py @@ -1,4 +1,4 @@ -import nothing +import nothing # noqa class InvalidCheck(object): pass diff --git a/tests/core/fixtures/checks/valid_conf_2.yaml b/tests/core/fixtures/checks/valid_conf_2.yaml new file mode 100644 index 0000000000..7aa1d97035 --- /dev/null +++ b/tests/core/fixtures/checks/valid_conf_2.yaml @@ -0,0 +1,5 @@ +init_config: + +instances: + - host: localhost + - host: localh0st diff --git a/tests/core/test_config.py b/tests/core/test_config.py index 30b5668163..ee29628f86 100644 --- a/tests/core/test_config.py +++ b/tests/core/test_config.py @@ -16,6 +16,7 @@ # No more hardcoded default checks DEFAULT_CHECKS = [] + class TestConfig(unittest.TestCase): def testWhiteSpaceConfig(self): """Leading whitespace confuse ConfigParser @@ -69,10 +70,10 @@ def testHostname(self): u'i-123445', u'5dfsdfsdrrfsv', u'432498234234A' - u'234234235235235235', # Couldn't find anything in the RFC saying it's not valid + u'234234235235235235', # Couldn't find anything in the RFC saying it's not valid u'A45fsdff045-dsflk4dfsdc.ret43tjssfd', u'4354sfsdkfj4TEfdlv56gdgdfRET.dsf-dg', - u'r'*255, + u'r' * 255, ] not_valid_hostnames = [ @@ -93,11 +94,11 @@ def testWindowsSplit(self): # Make the function run as if it was on windows func = Platform.is_win32 try: - Platform.is_win32 = staticmethod(lambda : True) + Platform.is_win32 = staticmethod(lambda: True) test_cases = [ - ("C:\\Documents\\Users\\script.py:C:\\Documents\\otherscript.py", ["C:\\Documents\\Users\\script.py","C:\\Documents\\otherscript.py"]), - ("C:\\Documents\\Users\\script.py:parser.py", ["C:\\Documents\\Users\\script.py","parser.py"]) + ("C:\\Documents\\Users\\script.py:C:\\Documents\\otherscript.py", ["C:\\Documents\\Users\\script.py", "C:\\Documents\\otherscript.py"]), + ("C:\\Documents\\Users\\script.py:parser.py", ["C:\\Documents\\Users\\script.py", "parser.py"]) ] for test_case, expected_result in test_cases: @@ -114,11 +115,14 @@ def testDefaultChecks(self): self.assertTrue(c in init_checks_names) -TEMP_3RD_PARTY_CHECKS_DIR = '/tmp/dd-agent-tests/3rd-party' -TEMP_ETC_CHECKS_DIR = '/tmp/dd-agent-tests/etc/checks.d' -TEMP_ETC_CONF_DIR = '/tmp/dd-agent-tests/etc/conf.d' -TEMP_AGENT_CHECK_DIR = '/tmp/dd-agent-tests' -FIXTURE_PATH = 'tests/core/fixtures/checks' +TMP_DIR = tempfile.gettempdir() +DD_AGENT_TEST_DIR = 'dd-agent-tests' +TEMP_3RD_PARTY_CHECKS_DIR = os.path.join(TMP_DIR, DD_AGENT_TEST_DIR, '3rd-party') +TEMP_ETC_CHECKS_DIR = os.path.join(TMP_DIR, DD_AGENT_TEST_DIR, 'etc', 'checks.d') +TEMP_ETC_CONF_DIR = os.path.join(TMP_DIR, DD_AGENT_TEST_DIR, 'etc', 'conf.d') +TEMP_AGENT_CHECK_DIR = os.path.join(TMP_DIR, DD_AGENT_TEST_DIR) +FIXTURE_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'fixtures', 'checks') + @mock.patch('config.get_checksd_path', return_value=TEMP_AGENT_CHECK_DIR) @mock.patch('config.get_confd_path', return_value=TEMP_ETC_CONF_DIR) @@ -232,6 +236,27 @@ def testConfigDeprecatedNagiosConfig(self, *args): self.assertEquals(1, len(checks['initialized_checks'])) self.assertEquals('valid_check_1', checks['initialized_checks'][0].check(None)) + def testConfigDefault(self, *args): + copyfile('%s/valid_conf.yaml' % FIXTURE_PATH, + '%s/test_check.yaml.default' % TEMP_ETC_CONF_DIR) + copyfile('%s/valid_check_1.py' % FIXTURE_PATH, + '%s/test_check.py' % TEMP_ETC_CHECKS_DIR) + checks = load_check_directory({"additional_checksd": TEMP_ETC_CHECKS_DIR}, "foo") + self.assertEquals(1, len(checks['initialized_checks'])) + + def testConfigCustomOverDefault(self, *args): + copyfile('%s/valid_conf.yaml' % FIXTURE_PATH, + '%s/test_check.yaml.default' % TEMP_ETC_CONF_DIR) + # a 2nd valid conf file, slightly different so that we can test which one has been picked up + # (with 2 instances for instance) + copyfile('%s/valid_conf_2.yaml' % FIXTURE_PATH, + '%s/test_check.yaml' % TEMP_ETC_CONF_DIR) + copyfile('%s/valid_check_1.py' % FIXTURE_PATH, + '%s/test_check.py' % TEMP_ETC_CHECKS_DIR) + checks = load_check_directory({"additional_checksd": TEMP_ETC_CHECKS_DIR}, "foo") + self.assertEquals(1, len(checks['initialized_checks'])) + self.assertEquals(2, checks['initialized_checks'][0].instance_count()) # check that we picked the right conf + def tearDown(self): for _dir in self.TEMP_DIRS: rmtree(_dir)