From 69758480f530f939b1170a4d034974f99b76103e Mon Sep 17 00:00:00 2001 From: Thirumalesh Aaraveti Date: Sun, 28 Jan 2024 22:03:29 +0530 Subject: [PATCH] Skip the deletion of unused volumes when a running cluster is present --- .../policy/aws/cleanup/unattached_volume.py | 4 +- .../policy/azure/cleanup/unattached_volume.py | 4 +- .../helpers/aws/aws_policy_operations.py | 27 +++++++++++ .../helpers/azure/azure_policy_operations.py | 28 ++++++++++++ .../aws/cleanup/test_unattached_volume.py | 26 ++++++++++- .../policy/azure/test_unattached_volume.py | 45 ++++++++++++++++--- 6 files changed, 124 insertions(+), 10 deletions(-) diff --git a/cloud_governance/policy/aws/cleanup/unattached_volume.py b/cloud_governance/policy/aws/cleanup/unattached_volume.py index f4cf71fe..6a0b13f1 100644 --- a/cloud_governance/policy/aws/cleanup/unattached_volume.py +++ b/cloud_governance/policy/aws/cleanup/unattached_volume.py @@ -19,11 +19,13 @@ def run_policy_operations(self): """ unattached_volumes = [] available_volumes = self._get_all_volumes() + active_cluster_ids = self._get_active_cluster_ids() for volume in available_volumes: tags = volume.get('Tags', []) resource_id = volume.get('VolumeId') cleanup_result = False - if Utils.equal_ignore_case(volume.get('State'), 'available'): + cluster_tag = self._get_cluster_tag(tags=volume.get('Tags')) + if Utils.equal_ignore_case(volume.get('State'), 'available') and cluster_tag not in active_cluster_ids: cleanup_days = self.get_clean_up_days_count(tags=tags) cleanup_result = self.verify_and_delete_resource(resource_id=resource_id, tags=tags, clean_up_days=cleanup_days) diff --git a/cloud_governance/policy/azure/cleanup/unattached_volume.py b/cloud_governance/policy/azure/cleanup/unattached_volume.py index 7867f577..c8f14ed3 100644 --- a/cloud_governance/policy/azure/cleanup/unattached_volume.py +++ b/cloud_governance/policy/azure/cleanup/unattached_volume.py @@ -18,10 +18,12 @@ def run_policy_operations(self): """ unattached_volumes = [] available_volumes = self._get_all_volumes() + active_cluster_ids = self._get_active_cluster_ids() for volume in available_volumes: tags = volume.get('tags') cleanup_result = False - if Utils.equal_ignore_case(volume.get('disk_state'), 'Unattached'): + cluster_tag = self._get_cluster_tag(tags=tags) + if Utils.equal_ignore_case(volume.get('disk_state'), 'Unattached') and cluster_tag not in active_cluster_ids: cleanup_days = self.get_clean_up_days_count(tags=tags) cleanup_result = self.verify_and_delete_resource( resource_id=volume.get('id'), tags=tags, diff --git a/cloud_governance/policy/helpers/aws/aws_policy_operations.py b/cloud_governance/policy/helpers/aws/aws_policy_operations.py index 8a217d55..0b121027 100644 --- a/cloud_governance/policy/helpers/aws/aws_policy_operations.py +++ b/cloud_governance/policy/helpers/aws/aws_policy_operations.py @@ -150,3 +150,30 @@ def _get_all_volumes(self, **kwargs) -> list: """ volumes = self._ec2_operations.get_volumes(**kwargs) return volumes + + def _get_active_cluster_ids(self): + """ + This method returns the active cluster id's + :return: + :rtype: + """ + active_instances = self._ec2_operations.get_ec2_instance_list() + cluster_ids = [] + for instance in active_instances: + for tag in instance.get('Tags', []): + if tag.get('Key', '').startswith('kubernetes.io/cluster'): + cluster_ids.append(tag.get('Key')) + break + return cluster_ids + + def _get_cluster_tag(self, tags: list): + """ + This method returns the cluster_tag + :return: + :rtype: + """ + if tags: + for tag in tags: + if tag.get('Key').startswith('kubernetes.io/cluster'): + return tag.get('Key') + return '' diff --git a/cloud_governance/policy/helpers/azure/azure_policy_operations.py b/cloud_governance/policy/helpers/azure/azure_policy_operations.py index 665ad282..de09f15d 100644 --- a/cloud_governance/policy/helpers/azure/azure_policy_operations.py +++ b/cloud_governance/policy/helpers/azure/azure_policy_operations.py @@ -105,3 +105,31 @@ def _get_all_volumes(self) -> list: """ volumes = self.compute_operations.get_all_disks() return volumes + + def _get_active_cluster_ids(self): + """ + This method returns the active cluster id's + :return: + :rtype: + """ + active_instances = self._get_all_instances() + cluster_ids = [] + for vm in active_instances: + tags = vm.tags if vm.tags else {} + for key, value in tags.items(): + if key.startswith('kubernetes.io/cluster'): + cluster_ids.append(key) + break + return cluster_ids + + def _get_cluster_tag(self, tags: dict): + """ + This method returns the cluster_tag + :return: + :rtype: + """ + if tags: + for key, value in tags.items(): + if key.startswith('kubernetes.io/cluster'): + return key + return '' diff --git a/tests/unittest/cloud_governance/policy/aws/cleanup/test_unattached_volume.py b/tests/unittest/cloud_governance/policy/aws/cleanup/test_unattached_volume.py index 03af071e..ee3af06c 100644 --- a/tests/unittest/cloud_governance/policy/aws/cleanup/test_unattached_volume.py +++ b/tests/unittest/cloud_governance/policy/aws/cleanup/test_unattached_volume.py @@ -1,11 +1,9 @@ - import boto3 from moto import mock_ec2 from cloud_governance.main.environment_variables import environment_variables from cloud_governance.policy.aws.cleanup.unattached_volume import UnattachedVolume - region_name = 'us-east-2' @@ -91,3 +89,27 @@ def test_unattached_volume_dry_run_no_skip(): response = response[0] assert response.get('ResourceDelete') == 'False' assert response.get('SkipPolicy') == 'NOTDELETE' + + +@mock_ec2 +def test_check_exists_cluster(): + """ + This tests verify skip the existing cluster volume + :return: + :rtype: + """ + tags = [{'Key': 'kubernetes.io/cluster/test', 'Value': 'owned'}] + environment_variables.environment_variables_dict['AWS_DEFAULT_REGION'] = region_name + environment_variables.environment_variables_dict['policy'] = 'unattached_volume' + environment_variables.environment_variables_dict['dry_run'] = 'no' + environment_variables.environment_variables_dict['DAYS_TO_TAKE_ACTION'] = 1 + ec2_client = boto3.client('ec2', region_name=region_name) + default_ami_id = 'ami-03cf127a' + ec2_client.run_instances(ImageId=default_ami_id, InstanceType='t2.micro', MaxCount=1, + MinCount=1, TagSpecifications=[{ 'ResourceType': 'instance','Tags': tags}]) + ec2_client.create_volume(AvailabilityZone=f'{region_name}a', Size=10, TagSpecifications=[{ + 'ResourceType': 'volume', + 'Tags': tags + }]) + volume_run = UnattachedVolume().run() + assert len(volume_run) == 0 diff --git a/tests/unittest/cloud_governance/policy/azure/test_unattached_volume.py b/tests/unittest/cloud_governance/policy/azure/test_unattached_volume.py index 842d9c0a..b1e75f0b 100644 --- a/tests/unittest/cloud_governance/policy/azure/test_unattached_volume.py +++ b/tests/unittest/cloud_governance/policy/azure/test_unattached_volume.py @@ -4,7 +4,7 @@ from cloud_governance.main.environment_variables import environment_variables from cloud_governance.policy.azure.cleanup.unattached_volume import UnattachedVolume -from tests.unittest.mocks.azure.mock_compute import MockDisk, MockAzure +from tests.unittest.mocks.azure.mock_compute import MockDisk, MockAzure, MockVirtualMachine def test_unattached_volume_dry_run_yes_0_unattached(): @@ -14,7 +14,9 @@ def test_unattached_volume_dry_run_yes_0_unattached(): mock_azure = MockAzure(disks=[mock_disk1]) mock_virtual_machines = Mock() mock_virtual_machines.list.side_effect = mock_azure.mock_list_disks - with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines): + mock_virtual_machines.list_all.side_effect = mock_azure.mock_list_all + with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines), \ + patch.object(ComputeManagementClient, 'virtual_machines', mock_virtual_machines): volume_run = UnattachedVolume() response = volume_run.run() assert len(response) == 0 @@ -27,7 +29,9 @@ def test_unattached_volume_dry_run_yes(): mock_azure = MockAzure(disks=[mock_disk1]) mock_virtual_machines = Mock() mock_virtual_machines.list.side_effect = mock_azure.mock_list_disks - with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines): + mock_virtual_machines.list_all.side_effect = mock_azure.mock_list_all + with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines), \ + patch.object(ComputeManagementClient, 'virtual_machines', mock_virtual_machines): volume_run = UnattachedVolume() response = volume_run.run() assert len(response) > 0 @@ -44,7 +48,9 @@ def test_unattached_volume_dry_run_no(): mock_azure = MockAzure(disks=[mock_disk1]) mock_virtual_machines = Mock() mock_virtual_machines.list.side_effect = mock_azure.mock_list_disks - with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines): + mock_virtual_machines.list_all.side_effect = mock_azure.mock_list_all + with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines), \ + patch.object(ComputeManagementClient, 'virtual_machines', mock_virtual_machines): volume_run = UnattachedVolume() response = volume_run.run() assert len(response) > 0 @@ -61,7 +67,9 @@ def test_unattached_volume_dry_run_no_7_days_action(): mock_azure = MockAzure(disks=[mock_disk1]) mock_virtual_machines = Mock() mock_virtual_machines.list.side_effect = mock_azure.mock_list_disks - with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines): + mock_virtual_machines.list_all.side_effect = mock_azure.mock_list_all + with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines), \ + patch.object(ComputeManagementClient, 'virtual_machines', mock_virtual_machines): volume_run = UnattachedVolume() response = volume_run.run() assert len(response) > 0 @@ -79,10 +87,35 @@ def test_unattached_volume_dry_run_no_skip(): mock_azure = MockAzure(disks=[mock_disk1]) mock_virtual_machines = Mock() mock_virtual_machines.list.side_effect = mock_azure.mock_list_disks - with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines): + mock_virtual_machines.list_all.side_effect = mock_azure.mock_list_all + with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines), \ + patch.object(ComputeManagementClient, 'virtual_machines', mock_virtual_machines): volume_run = UnattachedVolume() response = volume_run.run() assert len(response) > 0 response = response[0] assert response.get('ResourceDelete') == 'False' assert response.get('SkipPolicy') == 'NOTDELETE' + + +def test_check_exists_cluster(): + """ + This tests verify skip the existing cluster volume + :return: + :rtype: + """ + tags = {'kubernetes.io/cluster/test': 'owned'} + environment_variables.environment_variables_dict['policy'] = 'unattached_volume' + environment_variables.environment_variables_dict['dry_run'] = 'no' + environment_variables.environment_variables_dict['DAYS_TO_TAKE_ACTION'] = 1 + mock_disk1 = MockDisk(disk_state='Unattached', disk_size_gb=4, tags=tags) + mock_vm1 = MockVirtualMachine(tags=tags) + mock_azure = MockAzure(disks=[mock_disk1], vms=[mock_vm1]) + mock_virtual_machines = Mock() + mock_virtual_machines.list.side_effect = mock_azure.mock_list_disks + mock_virtual_machines.list_all.side_effect = mock_azure.mock_list_all + with patch.object(ComputeManagementClient, 'disks', mock_virtual_machines), \ + patch.object(ComputeManagementClient, 'virtual_machines', mock_virtual_machines): + volume_run = UnattachedVolume() + response = volume_run.run() + assert len(response) == 0