From 028b4869dbc7c96eaa36a5ffcfbc5041fd551303 Mon Sep 17 00:00:00 2001 From: Kevin Carter Date: Tue, 21 Jan 2020 14:35:49 -0600 Subject: [PATCH] Update the helper filter for needs_delete This change updats the helper filter so that we can better identify containers that are in need of deleting/restart. > Updates to the "needs_delete" helper will ensure we're validating data. > Tests have been added to the test function `test_needs_delete` which will ensure our data validation process is functional. Change-Id: I9add2a3277b82d63e519cbfd7da94bbedc6cef55 Signed-off-by: Kevin Carter (cherry picked from commit 166c4f184ec84ff97b77b05013a963b94b1843b0) --- .../ansible_plugins/filter/helpers.py | 51 +++++++++++-------- .../tests/plugins/filter/test_helpers.py | 34 +++++++++---- 2 files changed, 55 insertions(+), 30 deletions(-) diff --git a/tripleo_ansible/ansible_plugins/filter/helpers.py b/tripleo_ansible/ansible_plugins/filter/helpers.py index 6f5d6cb20..5597018cb 100644 --- a/tripleo_ansible/ansible_plugins/filter/helpers.py +++ b/tripleo_ansible/ansible_plugins/filter/helpers.py @@ -16,6 +16,7 @@ import ast import json +import re import six from collections import OrderedDict @@ -88,26 +89,28 @@ class FilterModule(object): for c in container_infos: c_name = c['Name'] installed_containers.append(c_name) + labels = c['Config'].get('Labels') + if not labels: + labels = dict() + managed_by = labels.get('managed_by', 'unknown').lower() - # Don't delete containers not managed by tripleo-ansible - if (c['Config']['Labels'] is None - or c['Config']['Labels'].get( - 'managed_by') != 'tripleo_ansible'): + # Check containers have a label + if not labels: + to_skip += [c_name] + + # Don't delete containers NOT managed by tripleo* or paunch* + elif not re.findall(r"(?=("+'|'.join(['tripleo', 'paunch'])+r"))", + managed_by): to_skip += [c_name] - continue # Only remove containers managed in this config_id - if (c['Config']['Labels'] is None - or c['Config']['Labels'].get('config_id') != config_id): + elif labels.get('config_id') != config_id: to_skip += [c_name] - continue # Remove containers with no config_data # e.g. broken config containers - if (c['Config']['Labels'] is not None - and 'config_data' not in c['Config']['Labels']): + elif 'config_data' not in labels: to_delete += [c_name] - continue for c_name, config_data in config.items(): # don't try to remove a container which doesn't exist @@ -125,18 +128,26 @@ class FilterModule(object): # changed. Since we already cleaned the containers not in config, # this check needs to be in that loop. # e.g. new TRIPLEO_CONFIG_HASH during a minor update - try: - c_facts = [c['Config']['Labels']['config_data'] - for c in container_infos if c_name == c['Name']] - except KeyError: - continue + c_datas = list() + for c in container_infos: + if c_name == c['Name']: + try: + c_datas.append(c['Config']['Labels']['config_data']) + except KeyError: + pass # Build c_facts so it can be compared later with config_data - c_facts = ast.literal_eval(c_facts[0]) if ( - len(c_facts)) == 1 else dict() + for c_data in c_datas: + try: + c_data = ast.literal_eval(c_data) + except (ValueError, SyntaxError): # may already be data + try: + c_data = dict(c_data) # Confirms c_data is type safe + except ValueError: # c_data is not data + c_data = dict() - if cmp(c_facts, config_data) != 0: - to_delete += [c_name] + if cmp(c_data, config_data) != 0: + to_delete += [c_name] return to_delete diff --git a/tripleo_ansible/tests/plugins/filter/test_helpers.py b/tripleo_ansible/tests/plugins/filter/test_helpers.py index 17fa05a35..31252c080 100644 --- a/tripleo_ansible/tests/plugins/filter/test_helpers.py +++ b/tripleo_ansible/tests/plugins/filter/test_helpers.py @@ -263,8 +263,7 @@ class TestHelperFilters(tests_base.TestCase): 'Name': 'mysql', 'Config': { 'Labels': { - 'config_id': 'dontdeleteme', - 'managed_by': 'triple_ansible', + 'config_id': 'tripleo_step1' } } }, @@ -275,7 +274,7 @@ class TestHelperFilters(tests_base.TestCase): 'managed_by': 'tripleo_ansible', 'config_id': 'tripleo_step1', 'container_name': 'rabbitmq', - 'name': 'rabbitmq', + 'name': 'rabbitmq' } } }, @@ -283,11 +282,11 @@ class TestHelperFilters(tests_base.TestCase): 'Name': 'swift', 'Config': { 'Labels': { - 'managed_by': 'tripleo_ansible', + 'managed_by': 'tripleo', 'config_id': 'tripleo_step1', 'container_name': 'swift', 'name': 'swift', - 'config_data': "{'foo': 'bar'}", + 'config_data': {'foo': 'bar'} } } }, @@ -295,11 +294,23 @@ class TestHelperFilters(tests_base.TestCase): 'Name': 'heat', 'Config': { 'Labels': { - 'managed_by': 'tripleo_ansible', + 'managed_by': 'tripleo-Undercloud', 'config_id': 'tripleo_step1', 'container_name': 'heat', 'name': 'heat', - 'config_data': "{'start_order': 0}", + 'config_data': "{'start_order': 0}" + } + } + }, + { + 'Name': 'test1', + 'Config': { + 'Labels': { + 'managed_by': 'tripleo-other', + 'config_id': 'tripleo_step1', + 'container_name': 'test1', + 'name': 'test1', + 'config_data': {'start_order': 0} } } }, @@ -307,8 +318,9 @@ class TestHelperFilters(tests_base.TestCase): 'Name': 'haproxy', 'Config': { 'Labels': { - 'managed_by': 'tripleo_ansible', + 'managed_by': 'paunch', 'config_id': 'tripleo_step1', + 'config_data': "" } } }, @@ -323,7 +335,7 @@ class TestHelperFilters(tests_base.TestCase): { 'Name': 'none_tripleo', 'Config': { - 'Labels': None, + 'Labels': None } }, ] @@ -343,8 +355,10 @@ class TestHelperFilters(tests_base.TestCase): 'swift': {'foo': 'bar'}, # config_data changed: restart needed 'heat': {'start_order': 1}, + # config_data changed: restart needed + 'test1': {'start_order': 2}, } - expected_list = ['rabbitmq', 'haproxy', 'heat'] + expected_list = ['rabbitmq', 'haproxy', 'heat', 'test1'] result = self.filters.needs_delete(container_infos=data, config=config, config_id='tripleo_step1')