frontend: Add delete cluster button for non dynamic clusters #3423

vyncent-t · 2025-06-02T15:19:28Z

Desciption

This PR introduces changes to enable deleting non dynamic clusters while at the Headlamp home view. Users now have the option to either remove the cluster from their default kubeconfig (similar to how it is now for other clusters), or to remove the cluster from the actual source kubeconfig file on their machine.

How to test non default kubeconfig delete

Start headlamp in app mode
Create a non dynamic cluster
Click the menu options for said dynamic cluster
Notice the Delete button is now available for dynamic clusters
Locate your kubeconfig for your nondynamic clusters on your machine
Click delete and view pop up dialog for the delete cluster button
Click yes and notice the cluster being removed from the kubeconfig file

How to test default kubeconfig delete

Start headlamp in app mode
Create a non dynamic cluster
Click the menu options for said dynamic cluster
Notice the Delete button is now available for dynamic clusters
Locate your kubeconfig for your nondynamic clusters on your machine
Click delete and view pop up dialog for the delete cluster button
Click yes and notice the cluster being removed from the the home view and not the kubeconfig file

Reworked from - #2567

vyncent-t · 2025-06-04T15:49:58Z

reviewed deleting non dynamic clusters from different config locations with @skoeva

works as intended 💯

vyncent-t · 2025-06-09T15:28:58Z

was asked to keep this in one PR and not split it by backend/frontend, mind running through the tests one more time like we did before?

@skoeva

skoeva

Works for me, LGTM

joaquimrocha

I know I asked questions or gave suggestions about previously existing code, but they are good to think about in the context of this PR. Please check them out.
About the i18n, please do not translate new strings unless you know the language (because it'll be difficult to spot by speakers later).

backend/pkg/config/config.go

backend/cmd/headlamp.go

joaquimrocha · 2025-06-12T13:19:52Z

backend/cmd/headlamp.go

+	if source == kubeConfigSource {
+		err = kubeconfig.RemoveContextFromFile(originalName, configPath)
+	} else {
+		err = kubeconfig.RemoveContextFromFile(name, configPath)
+	}


We could instead assign a variable with the actual name we are using for deleting and then have only one function call with it (more readable).

frontend/src/stateless/index.ts

frontend/src/components/App/Home/ClusterContextMenu.tsx

joaquimrocha

Much better now but still room to improve.

backend/cmd/headlamp.go

backend/pkg/config/config.go

backend/cmd/headlamp.go

frontend/src/lib/k8s/api/v1/clusterApi.ts

joaquimrocha · 2025-06-12T21:28:19Z

frontend/src/lib/k8s/api/v1/clusterApi.ts

+  if (source === 'kubeconfig' && originalName && removeKubeConfig) {
+    // for non dynamic clusters, we need to use the original name as a query parameter to find the actual context in the kubeconfig
+    // and remove it from the kubeconfig file.
+    deleteURL = `/cluster/${cluster}?removeKubeConfig=${removeFromKubeConfig}&source=${source}&configPath=${kubeconfigOrigin}&originalName=${originalName}`;


If we are sending the configPath, doesn't the source="kubeconfig" get redundant?
i.e. can we decide in the backend if we should do the same logic by just checking if configPath is sent in the request?
(honest question)

I see what you're saying here, but having something as destructive as this delete function I went for a more double requirement approach.

I have been experimenting with this logic and finding ways to extend the functionality for permanent deleting clusters for plugins also (I will open a PR for this later after this one is merged, if you think its a worth while feature to have. I have the implementation ready, I just want to land it for non dynamic first)

We are technically getting the config path (under kubeconfigOrigin) for every delete request when setting the check box to true, so having just a check for a location to delete from I feel isn't narrow enough.

On the other hand, this does open up possibilities of deleting beyond the non dynamic range here (as I mentioned for removing clusters from default headlamp dir) so I think having another check to make sure the kubeconfig source is nondynamic would be needed for this scope, as it is easily turned to delete in other places.

frontend/src/components/App/Home/ClusterContextMenu.tsx

vyncent-t · 2025-06-20T14:19:03Z

added a disable option for the confirm dialog to work with checkboxes

also added confirm step to deleting clusters

joaquimrocha · 2025-06-20T21:30:13Z

backend/cmd/headlamp.go

+	originalName := r.URL.Query().Get("originalName")
+	source := r.URL.Query().Get("source")
+
+	const kubeConfigSource = "kubeconfig"


If this is the commit introducing the lint issue, you should add the fix into this one and not on a different commit.

joaquimrocha · 2025-06-20T22:51:54Z

frontend/src/components/App/Home/ClusterContextMenu.tsx

+    const description =
+      cluster.meta_data?.source === 'kubeconfig'
+        ? t(
+            'translation|This action will delete cluster {{ clusterName }} from source {{ source }}',


I think there's still some confusion in what this message is about:
If users can only either delete or not delete the cluster (including removing it from the kubeconfig), then we don't need a check box to confirm they want to remove the cluster from the kubeconfig.

Adding the checkbox would be only useful in a scenario where we can give the option for the cluster to be removed from Headlamp's listing but not from the kubeconfig. Since this is not possible as the clusters are read from the kubeconfig, we can just tell the user that the action will remove the cluster/context from the kubeconfig file (no need for a checkbox).

And the user choices/buttons should be "Cancel" "Delete".

Hope this makes sense.

removed the checkbox logic

joaquimrocha

Thanks for the update. I think now it's very close.

joaquimrocha · 2025-06-24T11:27:47Z

frontend/src/components/App/Home/ClusterContextMenu.tsx

-    deleteCluster(cluster.name || '')
+    const clusterID = cluster.meta_data?.clusterID;
+    const clusterSource = cluster.meta_data?.source;
+    const originalName = clusterSource === 'kubeconfig' ? cluster.meta_data?.originalName : '';


Could it be?

const originalName = cluster.meta_data?.originalName ?? '';

added this, works the same

joaquimrocha · 2025-06-24T11:27:49Z

frontend/src/components/App/Home/ClusterContextMenu.tsx

+  useEffect(() => {
+    // If the cluster is from kubeconfig, we want to set the delete to kubeconfig.
+    if (cluster?.meta_data?.source === 'kubeconfig') {
+      setDeleteFromKubeconfig(true);
+    } else {
+      setDeleteFromKubeconfig(false);
+    }
+  }, [deleteFromKubeconfig]);


Why do you need this state?
You could just have const deleteFromKubeconfig = cluster?.meta_data?.source === 'kubeconfig'.

Had react render issues when trying to use something close to this but inside removeClusterDescription

Moved it under kubeconfigOrigin and seems to work the same now

…beConfigPersistenceFile into MakeHeadlampKubeConfigsDir and DefaultHeadlampKubeConfigFile

…rom local kubeconfig

…clusterID

…terID for nondynamic clusters

vyncent-t · 2025-06-24T13:41:47Z

The splitting rename issue for the dynamic clusters seems to come from the current main and not these changes.
#3502

these changes work as intended w/ @sniok

… dynamic clusters

…g non-dynamic clusters

joaquimrocha · 2025-06-24T17:36:22Z

/lgtm
/approve

k8s-ci-robot · 2025-06-24T17:36:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: joaquimrocha, skoeva, vyncent-t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [joaquimrocha]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 2, 2025

k8s-ci-robot requested review from ashu8912 and yolossn June 2, 2025 15:19

vyncent-t self-assigned this Jun 2, 2025

vyncent-t force-pushed the cluster-delete-button-app-mode branch from 42e0eb8 to 3b173f4 Compare June 2, 2025 20:32

vyncent-t changed the title ~~backend: config: Add DefaultKubeConfigPersistenceDir to config.go~~ frontend: Add delete cluster button for non dynamic clusters Jun 2, 2025

vyncent-t marked this pull request as ready for review June 2, 2025 21:03

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 2, 2025

k8s-ci-robot requested a review from illume June 2, 2025 21:03

vyncent-t force-pushed the cluster-delete-button-app-mode branch from 3b173f4 to c6f403b Compare June 4, 2025 18:23

skoeva mentioned this pull request Jun 4, 2025

frontend: ClusterTable: Add delete cluster functionality #3339

Open

skoeva approved these changes Jun 10, 2025

View reviewed changes

joaquimrocha requested changes Jun 12, 2025

View reviewed changes

vyncent-t force-pushed the cluster-delete-button-app-mode branch from c6f403b to fe14134 Compare June 12, 2025 21:01

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 12, 2025

joaquimrocha requested changes Jun 12, 2025

View reviewed changes

vyncent-t force-pushed the cluster-delete-button-app-mode branch 5 times, most recently from 057042e to c00dd32 Compare June 18, 2025 20:33

joaquimrocha requested changes Jun 20, 2025

View reviewed changes

joaquimrocha added this to Headlamp Release Plan / Roadmap Jun 23, 2025

github-project-automation bot moved this to Queued in Headlamp Release Plan / Roadmap Jun 23, 2025

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2025

vyncent-t force-pushed the cluster-delete-button-app-mode branch from c00dd32 to 2bd066c Compare June 23, 2025 17:12

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2025

vyncent-t force-pushed the cluster-delete-button-app-mode branch 4 times, most recently from 1fe018f to 55df4aa Compare June 23, 2025 19:16

joaquimrocha requested changes Jun 24, 2025

View reviewed changes

vyncent-t added 4 commits June 24, 2025 08:57

backend: config: Rework DefaultKubeConfigPresistenceDir and DefaultKu…

906430a

…beConfigPersistenceFile into MakeHeadlampKubeConfigsDir and DefaultHeadlampKubeConfigFile

backend: headlamp: Rework deleteCluster to handle removing clusters f…

c8d81a0

…rom local kubeconfig

frontend: stateless: Rework deleteClusterKubeconfig to now work with …

6f39aa2

…clusterID

frontend: clusterApi: rework deleteCluster function to work with clus…

f5270c1

…terID for nondynamic clusters

vyncent-t force-pushed the cluster-delete-button-app-mode branch from 55df4aa to 16b7e4b Compare June 24, 2025 12:57

vyncent-t requested a review from joaquimrocha June 24, 2025 15:07

frontend: ClusterContextMenu: Add delete cluster logic and UI for non…

7e33c41

… dynamic clusters

vyncent-t force-pushed the cluster-delete-button-app-mode branch from 16b7e4b to 7e33c41 Compare June 24, 2025 16:31

frontend: ClusterTable: Fix 'something went wrong' screen for deletin…

0fcb51b

…g non-dynamic clusters

k8s-ci-robot assigned joaquimrocha Jun 24, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 24, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 24, 2025

k8s-ci-robot merged commit 76a0b48 into kubernetes-sigs:main Jun 24, 2025
13 checks passed

github-project-automation bot moved this from Queued to Done in Headlamp Release Plan / Roadmap Jun 24, 2025

frontend: Add delete cluster button for non dynamic clusters #3423

frontend: Add delete cluster button for non dynamic clusters #3423

Conversation

vyncent-t commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Desciption

How to test non default kubeconfig delete

How to test default kubeconfig delete

Uh oh!

vyncent-t commented Jun 4, 2025

Uh oh!

vyncent-t commented Jun 9, 2025

Uh oh!

skoeva left a comment

Choose a reason for hiding this comment

Uh oh!

joaquimrocha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joaquimrocha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vyncent-t commented Jun 20, 2025

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!

joaquimrocha 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!

vyncent-t commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joaquimrocha commented Jun 24, 2025

Uh oh!

k8s-ci-robot commented Jun 24, 2025

Uh oh!

Uh oh!

Uh oh!

vyncent-t commented Jun 2, 2025 •

edited

Loading

vyncent-t commented Jun 24, 2025 •

edited

Loading