vcluster

vcluster copied to clipboard

What issue type does this pull request address? (keep at least one, remove the others) /kind feature

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible) resolves #771

Please provide a short message that should be published in the vcluster release notes Added a new flag --ignore-not-found that will not error out in case the vcluster is not found while deleting it

What else do we need to know?

@rahulii thanks for the PR! Looks like you need to rebase the PR on the current master, would be great if you could do that!

@FabianKramm @matskiv @mahendrabagul , if the vcluster is not found (it returns from here https://github.com/loft-sh/vcluster/blob/main/cmd/vclusterctl/cmd/delete.go#L160). Here I check if --ignore-not-found flag is set and the error is "Not Found error" to not error out. But from here, In case user says vcluster delete my-cluster --namespace my-namespace --delete-namespace --ignore-not-found the kubeClient is nil , and hence it panics. My question is: in case --ignore-not-found is set and vcluster is not found, do we construct a kubeclient from current context using https://github.com/loft-sh/vcluster/blob/main/cmd/vclusterctl/cmd/find/find.go#L42?

@rahulii I think you will need to modify the prepare function to initialize the cmd.kubeClient before it calls find.GetVCluster

@rahulii I think you will need to modify the prepare function to initialize the cmd.kubeClient before it calls find.GetVCluster

@matskiv not sure how is that possible? here the restConfig is fetched using vCluster.ClientFactory.ClientConfig().

@rahulii I think you will need to modify the prepare function to initialize the cmd.kubeClient before it calls find.GetVCluster

@matskiv here is the approach I was thinking: func (cmd *DeleteCmd) prepare(vClusterName string) error { if cmd.IgnoreNotFound { context := CurrentContext() // construct kubeclient using this context ..... } else { // current approach }

}

@rahulii I think you don't even need to check the flag in the prepare function. Just move this part to the very top of the prepare function, before GetVCluster is called:

kubeClient, err := kubernetes.NewForConfig(restConfig)
if err != nil {
  return err
}
cmd.kubeClient = kubeClient

@rahulii I think you don't even need to check the flag in the prepare function. Just move this part to the very top of the prepare function, before GetVCluster is called:

kubeClient, err := kubernetes.NewForConfig(restConfig)
if err != nil {
  return err
}
cmd.kubeClient = kubeClient

@matskiv umm to create a kubeClient it needs a restConfig which is derived as:

restConfig, err := vCluster.ClientFactory.ClientConfig()
if err != nil {
  return err
} 

here - https://github.com/loft-sh/vcluster/blob/main/cmd/vclusterctl/cmd/delete.go#L178

@rahulii yes, good point, I overlooked that. But I think we can still do it, we just need to provide the restConfig like so:

restConfig, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(clientcmd.NewDefaultClientConfigLoadingRules(), &clientcmd.ConfigOverrides{}).ClientConfig()

that^ is pretty much equivalent to the vCluster.ClientFactory.ClientConfig().

Hello @rahulii Do you plan to finish this PR or should we unassign you from the issue and close the PR for now? If we close, are you okay with someone else cherry-picking your changes into a new PR and adding the remaining code?

Hello @rahulii Do you plan to finish this PR or should we unassign you from the issue and close the PR for now? If we close, are you okay with someone else cherry-picking your changes into a new PR and adding the remaining code?

Hey @matskiv I am on it. Really sorry for delays!

Outdated