Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unified/Simplified Cloud Provider Cleanup and Resource Types #126

Merged
merged 2 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cloudwash/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ def cleanup_providers(ctx, dry, version):
click.echo(f"Version: {cloudwash_version}")
click.echo(f"Settings File: {settings.settings_file}")
if ctx.invoked_subcommand:
settings.set('dry_run', dry)
logger.info(
f"\n<<<<<<< Running the cleanup script in {'DRY' if dry else 'ACTION'} RUN mode >>>>>>>"
f"\n<<<<<<< Running the cleanup script in {'DRY' if dry else 'ACTION'} mode >>>>>>>"
)


Expand Down
37 changes: 37 additions & 0 deletions cloudwash/entities/providers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
from cloudwash.entities.resources.discs import CleanAWSDiscs
from cloudwash.entities.resources.discs import CleanAzureDiscs
from cloudwash.entities.resources.vms import CleanAWSVms
from cloudwash.entities.resources.vms import CleanAzureVMs


class providerCleanup:
jyejare marked this conversation as resolved.
Show resolved Hide resolved
def __init__(self, client):
self.client = client

@property
def vms(self):
providerclass = self.__class__.__name__
if 'Azure' in providerclass:
return CleanAzureVMs(client=self.client)
elif 'AWS' in providerclass:
return CleanAWSVms(client=self.client)
Comment on lines +11 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jyejare one more approach to solve this to remove more coupling and adding more providers and reduce the factory pattern made in parent class. let me know what you think about it

class ProviderCleanup:
    def __init__(self, client, vm_cleaner, disc_cleaner):
        self.client = client
        self.vm_cleaner = vm_cleaner
        self.disc_cleaner = disc_cleaner

    def vms(self):
        return self.vm_cleaner.clean(self.client)

    def discs(self):
        return self.disc_cleaner.clean(self.client)


class AzureCleanup(ProviderCleanup):
    def __init__(self, client):
        super().__init__(client, CleanAzureVMs(), CleanAzureDiscs())


class AWSCleanup(ProviderCleanup):
    def __init__(self, client):
        super().__init__(client, CleanAWSVms(), CleanAWSDiscs())

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm!

I see multiple issues in this pattern:

  1. Unnecessary Object Initialization of CloudProviderResourceType classes (e.g CleanAzureVMs(client), CleanAzureDiscs(client)) even though its not demanded! That also takes a huge time for all classes initialization. For now its the object creation is on demand and explicit if its demanded!
  2. The list of parameters will increase in endless number as and when number of resource types will grow. Though we can control it with kwargs, but its extra layer of verification if the object is available in params.
  3. In current design, if the non-common resource type property is needed to be added then you can do it at that cloud provider class level and dont need to send it back to base class for processing / returning.

For now, I can only think of these issues! Feel free to reply with the thoughts !

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I ack it.


@property
def discs(self):
providerclass = self.__class__.__name__
if 'Azure' in providerclass:
return CleanAzureDiscs(client=self.client)
elif 'AWS' in providerclass:
return CleanAWSDiscs(client=self.client)


class AzureCleanup(providerCleanup):
def __init__(self, client):
self.client = client
super().__init__(client)


class AWSCleanup(providerCleanup):
def __init__(self, client):
self.client = client
super().__init__(client)
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,24 @@ def _set_dry(self):
pass


class DiscsCleanup(ResourceCleanup):
@abstractmethod
def list(self):
pass

@abstractmethod
def cleanup(self):
pass

@abstractmethod
def remove(self):
pass

@abstractmethod
def _set_dry(self):
pass


class ResourceCleanupManager:
def __init__(self):
self.resources = []
Expand Down
51 changes: 51 additions & 0 deletions cloudwash/entities/resources/discs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
from cloudwash.config import settings
from cloudwash.entities.resources.base import DiscsCleanup
from cloudwash.logger import logger
from cloudwash.utils import dry_data


class CleanDiscs(DiscsCleanup):
def __init__(self, client):
self.client = client
self._delete = []
self.list()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure how this is working ? is that intentional ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is intentional. As soon as the class is initiated the listing should happen and all dry data must be updated for viewing without calling the explicit cleanup() method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also helps in if user just wanted to list / stop or any other method without actually cleaning it or calling that function !


def _set_dry(self):
# VMsContainer = namedtuple('VMsCotainer', ['delete', 'stop', 'skip'])
# return VMsContainer(self._delete, self._stop, self._skip)
jyejare marked this conversation as resolved.
Show resolved Hide resolved
dry_data['DISCS']['delete'] = self._delete
jyejare marked this conversation as resolved.
Show resolved Hide resolved

def list(self):
pass

def remove(self):
pass

def cleanup(self):
if not settings.dry_run:
self.remove()


class CleanAWSDiscs(CleanDiscs):
def list(self):
if settings.aws.criteria.disc.unassigned:
rdiscs = self.client.get_all_unattached_volumes()
rdiscs = [rdisc["VolumeId"] for rdisc in rdiscs]
self._delete.extend(rdiscs)
self._set_dry()

def remove(self):
self.client.remove_all_unused_volumes()
logger.info(f"Removed Discs: \n{self._delete}")


class CleanAzureDiscs(CleanDiscs):
def list(self):
if settings.aws.criteria.disc.unassigned:
jyejare marked this conversation as resolved.
Show resolved Hide resolved
rdiscs = self.client.list_free_discs()
self._delete.extend(rdiscs)
self._set_dry()

def remove(self):
self.client.remove_discs_by_search()
logger.info(f"Removed Discs: \n{self._delete}")
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
from cloudwash.config import settings
from cloudwash.entities.base import VMsCleanup
from cloudwash.entities.resources.base import VMsCleanup
from cloudwash.logger import logger
from cloudwash.utils import dry_data
from cloudwash.utils import total_running_time


class CleanVMs(VMsCleanup):
def __init__(self, awsclient):
self.client = awsclient
def __init__(self, client):
self.client = client
self._delete = []
self._stop = []
self._skip = []
Expand All @@ -20,12 +20,36 @@ def _set_dry(self):
dry_data['VMS']['stop'] = self._stop
dry_data['VMS']['skip'] = self._skip

def list(self):
pass

def remove(self):
for vm_name in self._delete:
self.client.get_vm(vm_name).delete()
logger.info(f"Removed VMs: \n{self._delete}")

def stop(self):
for vm_name in self._stop:
self.client.get_vm(vm_name).stop()
logger.info(f"Stopped VMs: \n{self._stop}")

def skip(self):
logger.info(f"Skipped VMs: \n{self._skip}")

def cleanup(self):
if not settings.dry_run:
self.remove()
self.stop()
self.skip()


class CleanAWSVms(CleanVMs):
def list(self):
all_vms = self.client.list_vms()

for vm in all_vms:
if vm.name in settings.aws.exceptions.vm.vm_list:
self._delete.append(vm.name)
self._skip.append(vm.name)
continue

elif total_running_time(vm).minutes >= settings.aws.criteria.vm.sla_minutes:
Expand All @@ -37,20 +61,32 @@ def list(self):
self._delete.append(vm.name)
self._set_dry()

def remove(self):
for vm_name in self._delete:
self.client.get_vm(vm_name).delete()
logger.info(f"Removed VMs: \n{self._delete}")

def stop(self):
for vm_name in self._stop:
self.client.get_vm(vm_name).stop()
logger.info(f"Stopped VMs: \n{self._stop}")

def skip(self):
logger.info(f"Skipped VMs: \n{self._skip}")
class CleanAzureVMs(CleanVMs):
def list(self):
all_vms = self.client.list_vms()

def cleanup(self):
self.remove()
self.stop()
self.skip()
for vm in all_vms:
if not vm.exists:
self._delete.append(vm.name)
continue
# Don't assess the VM that is still in creating state
if vm.state.lower() == "vmstate.creating":
continue
# Match the user defined criteria in settings to delete the VM
if vm.name in settings.azure.exceptions.vm.vm_list:
self._skip.append(vm.name)
elif (
getattr(
total_running_time(vm),
"minutes",
int(settings.azure.criteria.vm.sla_minutes) + 1,
)
>= settings.azure.criteria.vm.sla_minutes
):
if vm.name in settings.azure.exceptions.vm.stop_list:
self._stop.append(vm.name)
continue
elif vm.name.startswith(settings.azure.criteria.vm.delete_vm):
self._delete.append(vm.name)
self._set_dry()
21 changes: 5 additions & 16 deletions cloudwash/providers/aws.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""ec2 CR Cleanup Utilities"""
from cloudwash.client import compute_client
from cloudwash.config import settings
from cloudwash.entities.resources.aws import CleanVMs
from cloudwash.entities.providers import AWSCleanup
from cloudwash.logger import logger
from cloudwash.utils import dry_data
from cloudwash.utils import echo_dry
Expand All @@ -20,6 +20,8 @@ def cleanup(**kwargs):
for items in data:
dry_data[items]['delete'] = []
with compute_client("aws", aws_region=region) as aws_client:
awscleanup = AWSCleanup(client=aws_client)

# Dry Data Collection Defs
def dry_nics():
rnics = []
Expand All @@ -31,13 +33,6 @@ def dry_nics():
]
return rnics

def dry_discs():
rdiscs = []
if settings.aws.criteria.disc.unassigned:
rdiscs = aws_client.get_all_unattached_volumes()
[dry_data["DISCS"]["delete"].append(ddisc["VolumeId"]) for ddisc in rdiscs]
return rdiscs

def dry_images():
rimages = []
if settings.aws.criteria.image.unassigned:
Expand Down Expand Up @@ -89,20 +84,14 @@ def remove_stacks(stacks):
# Actual Cleaning and dry execution
logger.info(f"\nResources from the region: {region}")
if kwargs["vms"] or kwargs["_all"]:
vms_cleanup = CleanVMs(awsclient=aws_client)
if not is_dry_run:
vms_cleanup.cleanup()

awscleanup.vms.cleanup()
if kwargs["nics"] or kwargs["_all"]:
rnics = dry_nics()
if not is_dry_run and rnics:
aws_client.remove_all_unused_nics()
logger.info(f"Removed NICs: \n{rnics}")
if kwargs["discs"] or kwargs["_all"]:
rdiscs = dry_discs()
if not is_dry_run and rdiscs:
aws_client.remove_all_unused_volumes()
logger.info(f"Removed Discs: \n{rdiscs}")
awscleanup.discs.cleanup()
if kwargs["images"] or kwargs["_all"]:
rimages = dry_images()
if not is_dry_run and rimages:
Expand Down
65 changes: 4 additions & 61 deletions cloudwash/providers/azure.py
Original file line number Diff line number Diff line change
@@ -1,39 +1,10 @@
"""Azure CR Cleanup Utilities"""
from cloudwash.client import compute_client
from cloudwash.config import settings
from cloudwash.entities.providers import AzureCleanup
from cloudwash.logger import logger
from cloudwash.utils import dry_data
from cloudwash.utils import echo_dry
from cloudwash.utils import total_running_time


def _dry_vms(all_vms):
"""Filters and returns running VMs to be deleted from all VMs"""
_vms = {"stop": [], "delete": [], "skip": []}
for vm in all_vms:
# Remove the VM that's in Failed state and cant perform in assessments
if not vm.exists:
_vms["delete"].append(vm.name)
continue
# Don't assess the VM that is still in creating state
if vm.state.lower() == "vmstate.creating":
continue
# Match the user defined criteria in settings to delete the VM
if vm.name in settings.azure.exceptions.vm.vm_list:
_vms["skip"].append(vm.name)
continue
elif (
getattr(
total_running_time(vm), "minutes", int(settings.azure.criteria.vm.sla_minutes) + 1
)
>= settings.azure.criteria.vm.sla_minutes
):
if vm.name in settings.azure.exceptions.vm.stop_list:
_vms["stop"].append(vm.name)
continue
elif vm.name.startswith(settings.azure.criteria.vm.delete_vm):
_vms["delete"].append(vm.name)
return _vms


def cleanup(**kwargs):
Expand All @@ -57,17 +28,11 @@ def cleanup(**kwargs):
groups = azure_client.list_resource_groups()

for group in groups:
dry_data['VMS']['stop'] = []
dry_data['VMS']['skip'] = []
for items in data:
dry_data[items]['delete'] = []

with compute_client("azure", azure_region=region, resource_group=group) as azure_client:
# Dry Data Collection Defs
def dry_vms():
all_vms = azure_client.list_vms()
dry_data["VMS"] = _dry_vms(all_vms)
return dry_data["VMS"]
azurecleanup = AzureCleanup(client=azure_client)

def dry_nics():
rnics = []
Expand All @@ -76,13 +41,6 @@ def dry_nics():
[dry_data["NICS"]["delete"].append(dnic) for dnic in rnics]
return rnics

def dry_discs():
rdiscs = []
if settings.azure.criteria.disc.unassigned:
rdiscs = azure_client.list_free_discs()
[dry_data["DISCS"]["delete"].append(ddisc) for ddisc in rdiscs]
return rdiscs

def dry_pips():
rpips = []
if settings.azure.criteria.public_ip.unassigned:
Expand Down Expand Up @@ -119,33 +77,18 @@ def dry_images():
dry_data["IMAGES"]["delete"].extend(remove_images)
return remove_images

# Remove / Stop VMs
def remove_vms(avms):
# Remove VMs
[azure_client.get_vm(vm_name).delete() for vm_name in avms["delete"]]
# Stop VMs
[azure_client.get_vm(vm_name).stop() for vm_name in avms["stop"]]

# Actual Cleaning and dry execution
logger.info(f"\nResources from the region and resource group: {region}/{group}")

if kwargs["vms"] or kwargs["_all"]:
avms = dry_vms()
if not is_dry_run:
remove_vms(avms=avms)
logger.info(f"Stopped VMs: \n{avms['stop']}")
logger.info(f"Removed VMs: \n{avms['delete']}")
logger.info(f"Skipped VMs: \n{avms['skip']}")
azurecleanup.vms.cleanup()
if kwargs["nics"] or kwargs["_all"]:
rnics = dry_nics()
if not is_dry_run and rnics:
azure_client.remove_nics_by_search()
logger.info(f"Removed NICs: \n{rnics}")
if kwargs["discs"] or kwargs["_all"]:
rdiscs = dry_discs()
if not is_dry_run and rdiscs:
azure_client.remove_discs_by_search()
logger.info(f"Removed Discs: \n{rdiscs}")
azurecleanup.discs.cleanup()
if kwargs["pips"] or kwargs["_all"]:
rpips = dry_pips()
if not is_dry_run and rpips:
Expand Down
Loading