Delete OCP cluster leftovers using openshift-installer #166

oharan2 · 2025-04-24T12:22:03Z

Main changes:

Make filtering more strict by deselecting a group of cluster-related resources if one of them has recently modified
Update Dockerfile to install the openshift-installer, for local env check if exists
Update cli with a --yes, -y flag to bypass prompt questions before cluster destroying
- Otherwise handle user input with [Y/N]
Added LeftoverAWSOcp class that holds OCP cluster with all it's collected metadata and filtered resources
- Modified other oop hirarchy for OCP cleanup
Write down a temp metadata file as an input for the installer:
- Get the most accurate metadata by extracting OCP's resources tags
Update the dry run log with printing the names of the OCP clusters with associated leftovers found:

jyejare · 2025-05-18T11:52:06Z

@oharan2 The checks are failing in black. Please fix

jyejare

Finishing the 1st round of review. Suggested some changes.

jyejare · 2025-05-18T11:55:14Z

cloudwash/cli.py

    is_flag=True,
    help="Remove only unused OCP Cluster occupied resources from the provider",
 )
+@click.option("-y", "--yes", is_flag=True, help="Answer yes to all prompts")


If we don't have choice to say no, or if we should not provide the choice, let's remove the option here.

The alternative is -d, say no to all cluster deletion makes the deletion mode redundant.

In deletion mode, if not passing -y (yes to all prompts), you can go cluster by cluster and decide which one to clean up.
Therefore you're in deletion mode, but you can safely exclude some of it.

I think the framework that Cloudwash comes up with for excluding is to have the resource in exceptions list. That means the user has to pre-choose the clusters that user dont want to delete resources from.

Can we move to exceptions mode, How that would impact your use case ?

cloudwash/constants.py

jyejare · 2025-05-18T11:59:02Z

cloudwash/utils.py

    "DISCS": {"delete": []},
    "PIPS": {"delete": []},
-    "OCPS": {"delete": []},
+    "OCPS": {"delete": [], "clusters": []},


As per the cloudwash design 'clusters' must be part of delete dict

What the part that you concern will be breaking?
The clusters part is only for producing a dry run report with printing the actual ocp names that the resources are associated with.
The deletable part that is considered is individual resources and not entire clusters.
See use of generated report in the pic inside the updated PR description.

Yup, Got that But again the clusters are nothing but resources, we just changing its place from independent key-value to within the standard dictionary structure.

Saying this because we will be streamlining the dry data printing in future by having its own Class and hence good to keep the standard going fwd.

cloudwash/utils.py

jyejare · 2025-05-18T12:06:37Z

cloudwash/utils.py

+
+
+def destroy_ocp_cluster(metadata_path: str, cluster_name: str):
+    if metadata_path == "" or not os.path.exists(metadata_path):


You can choose to use Pathlib to play with file system.

Optional!

jyejare · 2025-05-18T12:08:11Z

cloudwash/utils.py

+        # Return without raising exception, will try to fetch next OCP cluster info
+        logger.error(f"Failed to load cluster info from metadata path: {metadata_path}.")
+    else:
+        err_msg = f"Failed to cleanup OCP cluster {cluster_name}. Failure info:"


Define this where it's needed, it's bit confusing 😔

jyejare · 2025-05-18T12:10:42Z

cloudwash/utils.py

+            logger.error(f"{err_msg}\n{ex}")
+
+
+def validate_deletion_with_user_input(cluster_name) -> bool:


The user input in automated resources cleanup is almost impossible.

Would you still want to go ahead ?

Correct. This is why we have to provide the cli option -y for automated cleanups, the alternative is -d (see comment above)

jyejare

Few more structural and consistency comments.

Also, Did you find the chance to run the changes on multi-clusters for multi type resources.

jyejare · 2025-06-02T13:27:09Z

cloudwash/cli.py

    is_flag=True,
    help="Remove only unused OCP Cluster occupied resources from the provider",
 )
+@click.option("-y", "--yes", is_flag=True, help="Answer yes to all prompts")


I think the framework that Cloudwash comes up with for excluding is to have the resource in exceptions list. That means the user has to pre-choose the clusters that user dont want to delete resources from.

Can we move to exceptions mode, How that would impact your use case ?

jyejare · 2025-06-02T13:30:44Z

cloudwash/utils.py

    "DISCS": {"delete": []},
    "PIPS": {"delete": []},
-    "OCPS": {"delete": []},
+    "OCPS": {"delete": [], "clusters": []},


Yup, Got that But again the clusters are nothing but resources, we just changing its place from independent key-value to within the standard dictionary structure.

Saying this because we will be streamlining the dry data printing in future by having its own Class and hence good to keep the standard going fwd.

jyejare · 2025-06-02T13:40:35Z

cloudwash/utils.py

+            print(f'Error: Input {user_input} unrecognised. Please try again.')
+
+
+def destroy_ocp_cluster_wrapper(metadata_path: str, cluster_name: str, user_validation=False):


Docstring please, maybe for all the functions here, may be autogenerate :)

jyejare · 2025-06-02T13:45:35Z

cloudwash/entities/resources/ocps.py

-        self.client = client
-        self._delete = []
+    def __init__(self):
+        self._deletable = {"ocp_clusters": [], "filtered_leftovers": []}


I think its good to be consistant with var names, That is self_delete = {}

jyejare · 2025-06-02T13:49:28Z

cloudwash/entities/resources/ocps.py

+    def __init__(self, client):
+        self.client = client
+        self.cleaning_region = self.client.cleaning_region
+        super().__init__()


Doesnt seems to be required.

oharan2 added 6 commits April 24, 2025 15:18

Delete clusters trough openshift-installer

56f3344

Use strict filtering for all associated resources

0101795

Filter cluster resources as group

322f026

Print openshift-installer cli output only during failures

cd55d5f

Update note

2c59933

Add user confirmation and -y --yes flags to the cli

b2bd7e6

oharan2 force-pushed the delete_installer branch 2 times, most recently from a232acb to b2bd7e6 Compare April 29, 2025 11:34

jyejare marked this pull request as draft May 5, 2025 12:25

oharan2 added 2 commits May 6, 2025 14:52

Make filtering more strict

e802971

Set clusters as LeftoverAWSOcp object. Update metadata

a69e100

oharan2 changed the title ~~WIP: Delete OCP cluster leftovers using openshift-installer~~ Delete OCP cluster leftovers using openshift-installer May 8, 2025

oharan2 added 2 commits May 9, 2025 14:51

Move AWS properties to CleanAWSOcps constructor

24dccae

Move openshift-install under /usr/local/bin

2cffbe6

oharan2 force-pushed the delete_installer branch from 0fa041f to 2cffbe6 Compare May 9, 2025 14:58

oharan2 marked this pull request as ready for review May 9, 2025 15:01

Remove debug commands from Dockerfile

61607a6

jyejare reviewed May 18, 2025

View reviewed changes

oharan2 requested a review from jyejare May 20, 2025 12:20

jyejare reviewed Jun 2, 2025

View reviewed changes



		def destroy_ocp_cluster(metadata_path: str, cluster_name: str):
		if metadata_path == "" or not os.path.exists(metadata_path):

		logger.error(f"{err_msg}\n{ex}")


		def validate_deletion_with_user_input(cluster_name) -> bool:

		print(f'Error: Input {user_input} unrecognised. Please try again.')


		def destroy_ocp_cluster_wrapper(metadata_path: str, cluster_name: str, user_validation=False):

Delete OCP cluster leftovers using openshift-installer #166

Are you sure you want to change the base?

Delete OCP cluster leftovers using openshift-installer #166

Uh oh!

Conversation

oharan2 commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jyejare commented May 18, 2025

Uh oh!

jyejare left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jyejare left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

oharan2 commented Apr 24, 2025 •

edited

Loading