From fab9e712341f0de8fb128469ac7323fb4fdb731f Mon Sep 17 00:00:00 2001 From: Alexander Kislitsky Date: Wed, 2 Mar 2016 14:01:13 +0300 Subject: [PATCH] Config format for packages based filtering rules changed For deploying collector service on the production nodes DevOps team uses external collector configs, saved in the JSON. JSON doesn't support tuple type and dict with tuple as key also can't be serialized. We are introducing new format for filtering rules configuration. The following dicts can be used as filtering rule: - {'packages_list': ['a', 'b']} - {'packages_list': ['a', 'b']: 'from_date': None} - {'packages_list': ['a', 'b']: 'from_date': '2016-03-10T22:34:39'} - {'build_id': 'build_id_value'} - {'build_id': 'build_id_value', 'from_date': None} - {'build_id': 'build_id_value', 'from_date': '2016-03-10T22:34:39'} The old filtering rules format is backward compatible: - {'build_id_value': None} - {'build_id_value': '2016-03-10T22:34:39'} Change-Id: I1be9760bb700be5b8e20c0e27689a6b017ba75f1 Partial-Bug: #1550376 --- collector/collector/api/app_prod.py | 2 + collector/collector/api/app_test.py | 2 + collector/collector/api/config.py | 57 +++++++++---- .../api/resources/installation_structure.py | 12 ++- .../collector/test/resources/test_config.py | 84 ++++++++++++++----- collector/manage_collector.py | 2 + 6 files changed, 120 insertions(+), 39 deletions(-) diff --git a/collector/collector/api/app_prod.py b/collector/collector/api/app_prod.py index a739867..f8a773b 100644 --- a/collector/collector/api/app_prod.py +++ b/collector/collector/api/app_prod.py @@ -13,9 +13,11 @@ # under the License. from collector.api.app import app +from collector.api.config import index_filtering_rules from collector.api import log app.config.from_object('collector.api.config.Production') app.config.from_envvar('COLLECTOR_SETTINGS', silent=True) +index_filtering_rules(app) log.init_logger() diff --git a/collector/collector/api/app_test.py b/collector/collector/api/app_test.py index d5abfc6..d6a1547 100644 --- a/collector/collector/api/app_test.py +++ b/collector/collector/api/app_test.py @@ -13,9 +13,11 @@ # under the License. from collector.api.app import app +from collector.api.config import index_filtering_rules from collector.api import log app.config.from_object('collector.api.config.Testing') app.config.from_envvar('COLLECTOR_SETTINGS', silent=True) +index_filtering_rules(app) log.init_logger() diff --git a/collector/collector/api/config.py b/collector/collector/api/config.py index e45bef3..fc78ee5 100644 --- a/collector/collector/api/config.py +++ b/collector/collector/api/config.py @@ -15,6 +15,8 @@ import logging import os +import six + class Production(object): DEBUG = False @@ -32,8 +34,9 @@ class Production(object): # Structure of FILTERING_RULES for releases < 8.0: # {release: {build_id: from_dt}} # Structure of FILTERING_RULES for releases >= 8.0: - # {release: {('fuel-nailgun-8.0.0-1.mos8212.noarch', - # 'fuel-library8.0-8.0.0-1.mos7718.noarch'): from_dt}} + # {release: [{'packages_list': ['fuel-nailgun-8.0.0-1.mos8212.noarch']}, + # {'packages_list': ['fuel-8.0-8.0.0-1.mos7718.noarch'], + # 'from_date': from_dt}] # # PAY ATTENTION: you must use tuples as indexes in the FILTERING_RULES # @@ -53,8 +56,11 @@ class Production(object): # }, # '6.1.1': {}, # All builds of 6.1.1 filtered # '7.0': None, # All builds of 7.0 not filtered - # '8.0': {('fuel-nailgun-8.0.0-1.mos8212.noarch',): '2016-02-01T23:00:18', - # ('fuel-nailgun-8.0.0-2.mos9345.noarch',): '2016-02-10',} + # '8.0': [{'packages_list': ['fuel-nailgun-8.0.0-1.mos8212.noarch'], + # 'from_date': '2016-02-01T23:00:18'}, + # {'packages_list': ['fuel-nailgun-8.0.0-2.mos9345.noarch']}, + # {'build_id': 'build_id_value', 'from_date': '2016-03-01'}, + # {'build_id': 'build_id_value'}] # } # # If you don't need any filtration, please set FILTERING_RULES = None @@ -76,16 +82,35 @@ class Testing(Production): SQLALCHEMY_ECHO = True -def normalize_build_info(build_info): - """Prepare build info for searching in the filtering rules +def packages_as_index(packages): + if isinstance(packages, (list, tuple)): + return tuple(sorted(packages)) + else: + return packages - :param build_info: build_id or packages list - :return: build_id or ordered tuple of packages + +def convert_rules_to_dict(rules): + """Converts filtering rules for release to internal format + + :param rules: dict or list of filtering rules for the release + :return: dict of converted filtering rules """ - if isinstance(build_info, (list, tuple)): - return tuple(sorted(build_info)) - return build_info + # Already converted or doesn't need to be converted + if isinstance(rules, dict): + return rules + + # If rules is list of dicts + result = {} + for rule in rules: + if 'packages_list' in rule: + build_info = packages_as_index(rule['packages_list']) + else: + build_info = rule['build_id'] + + result[build_info] = rule.get('from_date') + + return result def index_filtering_rules(app): @@ -98,15 +123,11 @@ def index_filtering_rules(app): """ filtering_rules = app.config.get('FILTERING_RULES') + if not filtering_rules: return - for rules in filtering_rules.itervalues(): + for release, rules in six.iteritems(filtering_rules): if not rules: continue - - for build_info, from_dt in rules.iteritems(): - normalized_info = normalize_build_info(build_info) - if normalized_info not in rules: - rules[normalized_info] = from_dt - rules.pop(build_info) + filtering_rules[release] = convert_rules_to_dict(rules) diff --git a/collector/collector/api/resources/installation_structure.py b/collector/collector/api/resources/installation_structure.py index e9f3fe0..bbf8b1b 100644 --- a/collector/collector/api/resources/installation_structure.py +++ b/collector/collector/api/resources/installation_structure.py @@ -25,7 +25,7 @@ from collector.api.app import db from collector.api.common.util import db_transaction from collector.api.common.util import exec_time from collector.api.common.util import handle_response -from collector.api.config import normalize_build_info +from collector.api.config import packages_as_index from collector.api.db.model import InstallationStructure @@ -70,7 +70,7 @@ def _is_filtered_by_build_info(build_info, filtering_rules): if build_info is None: return False - build_info = normalize_build_info(build_info) + build_info = packages_as_index(build_info) # build info not found if build_info not in filtering_rules: @@ -98,6 +98,7 @@ def _is_filtered(structure): :return: bool """ rules = app.config.get('FILTERING_RULES') + app.logger.debug("Filtering by rules: %s", rules) # No rules specified if not rules: return False @@ -110,18 +111,25 @@ def _is_filtered(structure): # Release not in rules if release not in rules: + app.logger.debug("Release: %s not in rules. Not filtered", + release) return True filtering_rules = rules.get(release) # Filtering rules doesn't specified if filtering_rules is None: + app.logger.debug("Filtering rules are empty. Not filtered") return False filtered_by_build_id = _is_filtered_by_build_info( build_id, filtering_rules) + app.logger.debug("Filtering by build_id: %s, result: %s", + build_id, filtered_by_build_id) filtered_by_packages = _is_filtered_by_build_info( packages, filtering_rules) + app.logger.debug("Filtering by packages: %s, result: %s", + packages, filtered_by_packages) return filtered_by_build_id or filtered_by_packages diff --git a/collector/collector/test/resources/test_config.py b/collector/collector/test/resources/test_config.py index ae72fb8..971a843 100644 --- a/collector/collector/test/resources/test_config.py +++ b/collector/collector/test/resources/test_config.py @@ -19,34 +19,82 @@ from collector.test.base import BaseTest from collector.api.app import app from collector.api.config import index_filtering_rules -from collector.api.config import normalize_build_info +from collector.api.config import packages_as_index class TestConfig(BaseTest): def test_filtering_rules_indexed(self): - build_id = 'build_id_0' - filtering_rules = {(3, 2, 1): None, (2, 1): '2016-01-26', - 'build_id': build_id} release = '8.0' + packages_0 = [1, 5, 2] + packages_1 = [6, 4, 3] + from_date_1 = '2016-03-01' + packages_2 = [] + raw_rules = { + release: [ + {'packages_list': packages_0}, + {'packages_list': packages_1, 'from_date': from_date_1}, + {'packages_list': packages_2, 'from_date': None} + ] + } + + expected_rules = { + release: { + packages_as_index(packages_0): None, + packages_as_index(packages_1): from_date_1, + packages_as_index(packages_2): None + } + } + with mock.patch.dict( app.config, - {'FILTERING_RULES': {release: filtering_rules.copy()}} + {'FILTERING_RULES': copy.deepcopy(raw_rules)} ): # Checking filtering rules before sorting - actual_filtering_rules = app.config.get('FILTERING_RULES')[release] - for packages, from_dt in filtering_rules.iteritems(): - if isinstance(packages, tuple): - self.assertNotIn(tuple(sorted(packages)), - actual_filtering_rules) - self.assertIn(packages, actual_filtering_rules) + actual_rules = app.config.get('FILTERING_RULES') + actual_release_rules = actual_rules[release] + for rule in raw_rules[release]: + packages = packages_as_index(rule['packages_list']) + self.assertNotIn(packages, actual_release_rules) # Checking filtering rules after sorting index_filtering_rules(app) - actual_filtering_rules = app.config.get('FILTERING_RULES')[release] - for build_info in filtering_rules.iterkeys(): - self.assertIn(normalize_build_info(build_info), - actual_filtering_rules) + actual_rules = app.config.get('FILTERING_RULES') + self.assertEqual(expected_rules, actual_rules) + + def test_mix_packages_and_build_id(self): + release_build_id = '7.0' + build_id = 'build_id_0' + + release_mixed = '8.0' + build_id_mixed = 'build_id_1' + from_date = '2016-03-01' + packages = [1, 5, 2] + + raw_rules = { + release_mixed: [{'packages_list': packages}, + {'build_id': build_id_mixed, + 'from_date': from_date}], + release_build_id: {build_id: None} + } + + with mock.patch.dict( + app.config, + {'FILTERING_RULES': copy.deepcopy(raw_rules)} + ): + index_filtering_rules(app) + actual_filtering_rules = app.config.get('FILTERING_RULES') + + expected_rules = { + release_mixed: { + packages_as_index(packages): None, + build_id_mixed: from_date + }, + release_build_id: { + build_id: None + } + } + self.assertEqual(expected_rules, actual_filtering_rules) def test_index_filtering_rules_idempotent(self): packages = ('a', 'b', 'c') @@ -61,12 +109,10 @@ class TestConfig(BaseTest): index_filtering_rules(app) actual_rules = copy.copy( app.config.get('FILTERING_RULES')[release]) - self.assertIn(normalize_build_info(packages), actual_rules) + self.assertIn(packages_as_index(packages), actual_rules) self.assertEqual(expected_rules, actual_rules) def test_index_filtering_rules(self): - build_id = '2016-xxx.yyy' - self.assertEqual(build_id, normalize_build_info(build_id)) packages = ['z', 'x', 'a'] self.assertEqual(tuple(sorted(packages)), - normalize_build_info(packages)) + packages_as_index(packages)) diff --git a/collector/manage_collector.py b/collector/manage_collector.py index ca30faa..5e2bc21 100755 --- a/collector/manage_collector.py +++ b/collector/manage_collector.py @@ -21,6 +21,7 @@ from flask_script import Manager from collector.api import log from collector.api.app import app from collector.api import app as app_module +from collector.api.config import index_filtering_rules from collector.api.db.model import * import flask_sqlalchemy @@ -32,6 +33,7 @@ def configure_app(mode=None): } app.config.from_object(mode_map.get(mode)) app.config.from_envvar('COLLECTOR_SETTINGS', silent=True) + index_filtering_rules(app) setattr(app_module, 'db', flask_sqlalchemy.SQLAlchemy(app)) log.init_logger() return app