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

Add support for deleting resources from multiple regions in AWS #19

Merged
merged 1 commit into from
Jul 21, 2022

Conversation

shubhamsg199
Copy link
Collaborator

@shubhamsg199 shubhamsg199 commented Jun 15, 2022

Adding support for deleting resources from multiple regions in AWS.

  • The region for ec2 in settings.yaml will take the list of regions instead of a single region

Output:

$ swach -d ec2 --all
<<<<<<< Running the cleanup script in DRY RUN mode >>>>>>> 
The EC2 providers settings are initialized and validated !

Cleaning resources from the region: ap-south-1

=========== DRY SUMMARY ============

No resources are eligible for cleanup!

====================================


Cleaning resources from the region: eu-west-1

=========== DRY SUMMARY ============

No resources are eligible for cleanup!

====================================


Cleaning resources from the region: us-west-2

=========== DRY SUMMARY ============

No resources are eligible for cleanup!

====================================

Issue: #12

@@ -22,5 +22,5 @@ PROVIDERS:
EC2:
USERNAME:
PASSWORD:
REGION:
REGION: []

Choose a reason for hiding this comment

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

I suggest to keep the same convention for all cloud providers. Currently, the REGION is just a string. Now, it will be a string for Azure and GCP and an array of string for AWS.

What do you think about using REGION for a single string and REGIONS for an array? REGIONS could throw an error something like 'Not yet supported for Azure/GCP' for cloud providers other than AWS. In the future, others can add such a support for Azure and GCP.

Copy link
Collaborator Author

@shubhamsg199 shubhamsg199 Jun 27, 2022

Choose a reason for hiding this comment

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

Thanks for your input @apodhrad
Since deleting resources from multiple regions is the objective I think an array for REGIONS will be a good idea.
We can also add the same support for AZURE/GCP and handle the errors accordingly in the future.
OR
I can also create a new module which can have the Regions data and settings.yaml would then contain REGIONS as Strings itself.
So users would also have to update that new module file with the regions they want the resources to be deleted which will be a new task in addition to updating settings.yaml
what do you think? @apodhrad , any views on this @jyejare

@jyejare
Copy link
Collaborator

jyejare commented Jun 24, 2022

@shubhamsg199 Please rebase your PR with new merged PR #24 !

@shubhamsg199 shubhamsg199 force-pushed the cleanup_multiple_regions branch 3 times, most recently from 6ef5bbc to 4bcc8a9 Compare June 30, 2022 07:58
Copy link
Collaborator

@jyejare jyejare left a comment

Choose a reason for hiding this comment

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

The looping needs to take care of deleting the resources along with not showing those deleted resources from one region in another region for dry data. Same is impacting in VM deletion.

Also, I we can rename region to plural regions for all CRs and we can pass single, multiple regions or all. In case of all we will auto fetch list of regions from EC2Client.list_regions()

cloudwash/client.py Outdated Show resolved Hide resolved
cloudwash/providers/ec2.py Outdated Show resolved Hide resolved
cloudwash/providers/ec2.py Outdated Show resolved Hide resolved
cloudwash/providers/ec2.py Outdated Show resolved Hide resolved
@shubhamsg199 shubhamsg199 marked this pull request as draft July 5, 2022 11:48
@jyejare
Copy link
Collaborator

jyejare commented Jul 18, 2022

Ping @shubhamsg199 !!!

@shubhamsg199 shubhamsg199 force-pushed the cleanup_multiple_regions branch 2 times, most recently from 5538692 to 15c354c Compare July 19, 2022 15:12
@shubhamsg199 shubhamsg199 marked this pull request as ready for review July 19, 2022 15:19
@shubhamsg199 shubhamsg199 force-pushed the cleanup_multiple_regions branch 2 times, most recently from 4eb3750 to 0baf46c Compare July 20, 2022 07:07
cloudwash/providers/ec2.py Show resolved Hide resolved
settings.yaml.template Outdated Show resolved Hide resolved
@jyejare jyejare merged commit b48e118 into RedHatQE:master Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants