From f7261c1a96e478cae2a9e9e0718cd1b6a1cc8d9a Mon Sep 17 00:00:00 2001 From: rashed-k Date: Wed, 20 Mar 2024 15:31:49 +1100 Subject: [PATCH 1/6] bug fix for deploy target config & updated test --- api/plugins/action/deploy_target_config.py | 50 ++++++++---------- .../action/test_determine_updates_required.py | 52 +++++++++++++++++-- 2 files changed, 72 insertions(+), 30 deletions(-) diff --git a/api/plugins/action/deploy_target_config.py b/api/plugins/action/deploy_target_config.py index 19207db..f7496e6 100644 --- a/api/plugins/action/deploy_target_config.py +++ b/api/plugins/action/deploy_target_config.py @@ -90,35 +90,31 @@ def run(self, tmp=None, task_vars=None): return result def determine_required_updates(existing_configs, desired_configs): - addition_required = [] - deletion_required = [ - config['id'] - for config in existing_configs - if not any( - config['branches'] == desired['branches'] - for desired in desired_configs - ) - ] + addition_required = [] + deletion_required = [] + + grouped_configs = {} + for config in existing_configs: + key = (config['branches'], config['pullrequests'], str(config['deployTarget']['id']), str(config['weight'])) + if key not in grouped_configs: + grouped_configs[key] = [] + grouped_configs[key].append(config) + for desired in desired_configs: - found = False - uptodate = True - for existing_config in existing_configs: - if existing_config['branches'] != desired['branches']: - continue - - desired['_existing_id'] = existing_config['id'] - found = True - - # Mark for update (or in this context, addition) if there are discrepancies in any key property - if (existing_config['pullrequests'] != desired['pullrequests'] or - str(existing_config['deployTarget']['id']) != str(desired['deployTarget']) or - str(existing_config['weight']) != str(desired['weight'])): - desired['_existing_id'] = existing_config['id'] - uptodate = False - break - - if not found or not uptodate: + key = (desired['branches'], desired['pullrequests'], str(desired['deployTarget']), str(desired['weight'])) + if key not in grouped_configs: addition_required.append(desired) + print(f"Marked new configuration for addition: {desired}.") + + for configs in grouped_configs.values(): + for config in configs: + if not any( + config['branches'] == desired['branches'] and + str(config['deployTarget']['id']) == str(desired['deployTarget']) and + config['pullrequests'] == desired['pullrequests'] and + str(config['weight']) == str(desired['weight']) + for desired in desired_configs): + deletion_required.append(config['id']) return addition_required, deletion_required \ No newline at end of file diff --git a/api/tests/unit/plugins/action/test_determine_updates_required.py b/api/tests/unit/plugins/action/test_determine_updates_required.py index 43b00ab..00636ca 100644 --- a/api/tests/unit/plugins/action/test_determine_updates_required.py +++ b/api/tests/unit/plugins/action/test_determine_updates_required.py @@ -15,6 +15,7 @@ def test_update_required(self): 'branches': '^(main)$', 'deployTarget': 1, 'pullrequests': 'false', + 'weight': 1 } ] @@ -72,7 +73,7 @@ def test_update_required_weight(self): assert len(addition_required) == 1, "Expected one addition required due to weight change" assert addition_required[0]['weight'] == 2, "Expected weight to be updated to 2" - assert len(deletion_required) == 0, "Expected no deletions required" + assert len(deletion_required) == 1, "Expected no deletions required" def test_update_required_cluster(self): existing_configs = [ @@ -97,7 +98,7 @@ def test_update_required_cluster(self): assert len(addition_required) == 1, "Expected one addition required due to deployTarget change" assert addition_required[0]['deployTarget'] == 2, "Expected deployTarget to be updated to 2" - assert len(deletion_required) == 0, "Expected no deletions required" + assert len(deletion_required) == 1, "Expected no deletions required" def test_orphan_existing(self): existing_configs = [ @@ -136,4 +137,49 @@ def test_orphan_existing(self): assert len(addition_required) == 2, "Expected two additions required due to changes" assert addition_required[0]['deployTarget'] == 1, "Expected the first addition to have deployTarget 1" assert addition_required[1]['deployTarget'] == 2, "Expected the second addition to have deployTarget 2" - assert len(deletion_required) == 2, "Expecting 2 deletion of orphan deploytarget config" \ No newline at end of file + assert len(deletion_required) == 2, "Expecting 2 deletion of orphan deploytarget config" + + + def test_duplicate_target_config(self): + existing_configs = [ + { + 'branches': '^(main)$', + 'deployTarget': {'id': 1, 'name': 'cluster.io'}, + 'id': 1, + 'pullrequests': 'false', + 'weight': 1 + }, + { + 'branches': '^(main)$', + 'deployTarget': {'id': 1, 'name': 'cluster.io'}, + 'id': 1, + 'pullrequests': 'false', + 'weight': 1 + }, + { + 'branches': '^(develop)$', + 'deployTarget': {'id': 2, 'name': 'cluster.io'}, + 'id': 2, + 'pullrequests': 'true', + 'weight': 1 + } + ] + desired_configs = [ + { + 'branches': '^(main)$', + 'deployTarget': 1, + 'pullrequests': 'false', + 'weight': 1 + }, + { + 'branches': '^(develop)$', + 'deployTarget': 2, + 'pullrequests': 'true', + 'weight': 1 + } + ] + + addition_required, deletion_required = determine_required_updates(existing_configs, desired_configs) + + assert len(addition_required) == 0, "Expected two additions required due to changes" + assert len(deletion_required) == 1, "Expected one deletion of duplicate deploytarget config" \ No newline at end of file From 7d9bd98901803ee07fdf9898cafd9d82ac417a74 Mon Sep 17 00:00:00 2001 From: rashed-k Date: Wed, 20 Mar 2024 15:40:20 +1100 Subject: [PATCH 2/6] minor change --- .../unit/plugins/action/test_determine_updates_required.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/tests/unit/plugins/action/test_determine_updates_required.py b/api/tests/unit/plugins/action/test_determine_updates_required.py index 00636ca..0485583 100644 --- a/api/tests/unit/plugins/action/test_determine_updates_required.py +++ b/api/tests/unit/plugins/action/test_determine_updates_required.py @@ -151,7 +151,7 @@ def test_duplicate_target_config(self): }, { 'branches': '^(main)$', - 'deployTarget': {'id': 1, 'name': 'cluster.io'}, + 'deployTarget': {'id': 3, 'name': 'cluster.io'}, 'id': 1, 'pullrequests': 'false', 'weight': 1 @@ -181,5 +181,5 @@ def test_duplicate_target_config(self): addition_required, deletion_required = determine_required_updates(existing_configs, desired_configs) - assert len(addition_required) == 0, "Expected two additions required due to changes" + assert len(addition_required) == 0, "Expected no additions changes" assert len(deletion_required) == 1, "Expected one deletion of duplicate deploytarget config" \ No newline at end of file From bb5f7f7f1864e05160de77bd6c03861f5121bdb5 Mon Sep 17 00:00:00 2001 From: rashed-k Date: Wed, 27 Mar 2024 11:26:14 +1100 Subject: [PATCH 3/6] debug-test --- api/plugins/action/deploy_target_config.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/api/plugins/action/deploy_target_config.py b/api/plugins/action/deploy_target_config.py index f7496e6..c233c24 100644 --- a/api/plugins/action/deploy_target_config.py +++ b/api/plugins/action/deploy_target_config.py @@ -95,18 +95,25 @@ def determine_required_updates(existing_configs, desired_configs): grouped_configs = {} for config in existing_configs: + # Creating a unique key based on config properties key = (config['branches'], config['pullrequests'], str(config['deployTarget']['id']), str(config['weight'])) if key not in grouped_configs: grouped_configs[key] = [] grouped_configs[key].append(config) - + # Print grouped existing configurations for debugging + print(f"Grouped existing config: {key} -> {grouped_configs[key]}") for desired in desired_configs: + # Creating a key for desired configuration to check against grouped existing configurations key = (desired['branches'], desired['pullrequests'], str(desired['deployTarget']), str(desired['weight'])) + # Print to show what we are trying to find in existing configurations + print(f"Checking if desired config exists: {key}") if key not in grouped_configs: addition_required.append(desired) print(f"Marked new configuration for addition: {desired}.") + # Printing existing configurations before checking for deletions + print(f"Existing configurations before deletion check: {grouped_configs}") for configs in grouped_configs.values(): for config in configs: if not any( @@ -116,5 +123,7 @@ def determine_required_updates(existing_configs, desired_configs): str(config['weight']) == str(desired['weight']) for desired in desired_configs): deletion_required.append(config['id']) + # Print configurations marked for deletion + print(f"Marked for deletion: {config['id']}") - return addition_required, deletion_required \ No newline at end of file + return addition_required, deletion_required From d9c19c50e0938937793bb102cb1f60f15e0cfb1f Mon Sep 17 00:00:00 2001 From: rashed-k Date: Wed, 27 Mar 2024 11:40:38 +1100 Subject: [PATCH 4/6] debug-test-2 --- api/plugins/action/deploy_target_config.py | 30 ++++++++++++++++------ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/api/plugins/action/deploy_target_config.py b/api/plugins/action/deploy_target_config.py index c233c24..03e66b4 100644 --- a/api/plugins/action/deploy_target_config.py +++ b/api/plugins/action/deploy_target_config.py @@ -95,19 +95,25 @@ def determine_required_updates(existing_configs, desired_configs): grouped_configs = {} for config in existing_configs: - # Creating a unique key based on config properties key = (config['branches'], config['pullrequests'], str(config['deployTarget']['id']), str(config['weight'])) if key not in grouped_configs: grouped_configs[key] = [] grouped_configs[key].append(config) - # Print grouped existing configurations for debugging - print(f"Grouped existing config: {key} -> {grouped_configs[key]}") + print("Grouped existing configurations by branches, pullrequests, deployTarget ID, and weight.") + + # Step 2: Identify duplicates and mark older ones for deletion + for key, configs in grouped_configs.items(): + if len(configs) > 1: + sorted_configs = sorted(configs, key=lambda x: x['id'], reverse=True) + newest_config = sorted_configs[0] + for config in sorted_configs[1:]: + deletion_required.append(config['id']) + print(f"Marked older duplicate configurations for deletion based on key {key}, keeping newest configuration with ID {newest_config['id']}.") + grouped_configs[key] = [newest_config] + # Adjusted logic for handling additions and deletions for desired in desired_configs: - # Creating a key for desired configuration to check against grouped existing configurations key = (desired['branches'], desired['pullrequests'], str(desired['deployTarget']), str(desired['weight'])) - # Print to show what we are trying to find in existing configurations - print(f"Checking if desired config exists: {key}") if key not in grouped_configs: addition_required.append(desired) print(f"Marked new configuration for addition: {desired}.") @@ -123,7 +129,15 @@ def determine_required_updates(existing_configs, desired_configs): str(config['weight']) == str(desired['weight']) for desired in desired_configs): deletion_required.append(config['id']) - # Print configurations marked for deletion - print(f"Marked for deletion: {config['id']}") + print(f"Marked configuration for deletion as it's not present in desired configs: {config}.") + + + print("Addition Required:") + for addition in addition_required: + print(addition) + + print("\nDeletion Required:") + for deletion in deletion_required: + print(deletion) return addition_required, deletion_required From 388861b09d21057ab85d13e7f17b90877cff1809 Mon Sep 17 00:00:00 2001 From: rashed-k Date: Wed, 27 Mar 2024 11:47:13 +1100 Subject: [PATCH 5/6] updated test --- .../unit/plugins/action/test_determine_updates_required.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/tests/unit/plugins/action/test_determine_updates_required.py b/api/tests/unit/plugins/action/test_determine_updates_required.py index 0485583..43c2a3a 100644 --- a/api/tests/unit/plugins/action/test_determine_updates_required.py +++ b/api/tests/unit/plugins/action/test_determine_updates_required.py @@ -151,8 +151,8 @@ def test_duplicate_target_config(self): }, { 'branches': '^(main)$', - 'deployTarget': {'id': 3, 'name': 'cluster.io'}, - 'id': 1, + 'deployTarget': {'id': 1, 'name': 'cluster.io'}, + 'id': 2, 'pullrequests': 'false', 'weight': 1 }, From 9d74c7027fff36223843e34c12173d627c7e2e73 Mon Sep 17 00:00:00 2001 From: rashed-k Date: Wed, 27 Mar 2024 11:52:09 +1100 Subject: [PATCH 6/6] final changes --- api/plugins/action/deploy_target_config.py | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/api/plugins/action/deploy_target_config.py b/api/plugins/action/deploy_target_config.py index 03e66b4..a1e1b79 100644 --- a/api/plugins/action/deploy_target_config.py +++ b/api/plugins/action/deploy_target_config.py @@ -99,27 +99,24 @@ def determine_required_updates(existing_configs, desired_configs): if key not in grouped_configs: grouped_configs[key] = [] grouped_configs[key].append(config) - print("Grouped existing configurations by branches, pullrequests, deployTarget ID, and weight.") - # Step 2: Identify duplicates and mark older ones for deletion + #Identify duplicates and mark older ones for deletion for key, configs in grouped_configs.items(): if len(configs) > 1: sorted_configs = sorted(configs, key=lambda x: x['id'], reverse=True) newest_config = sorted_configs[0] for config in sorted_configs[1:]: deletion_required.append(config['id']) - print(f"Marked older duplicate configurations for deletion based on key {key}, keeping newest configuration with ID {newest_config['id']}.") grouped_configs[key] = [newest_config] - # Adjusted logic for handling additions and deletions + # Adjusted logic for handling additions for desired in desired_configs: key = (desired['branches'], desired['pullrequests'], str(desired['deployTarget']), str(desired['weight'])) if key not in grouped_configs: addition_required.append(desired) - print(f"Marked new configuration for addition: {desired}.") + - # Printing existing configurations before checking for deletions - print(f"Existing configurations before deletion check: {grouped_configs}") + # checking existing configurations for deletions for configs in grouped_configs.values(): for config in configs: if not any( @@ -129,15 +126,6 @@ def determine_required_updates(existing_configs, desired_configs): str(config['weight']) == str(desired['weight']) for desired in desired_configs): deletion_required.append(config['id']) - print(f"Marked configuration for deletion as it's not present in desired configs: {config}.") - - - print("Addition Required:") - for addition in addition_required: - print(addition) - - print("\nDeletion Required:") - for deletion in deletion_required: - print(deletion) + return addition_required, deletion_required