clusterctl delete should be idempotent

[jessehu]

What steps did you take and what happened:

$ clusterctl delete --all --include-crd --include-namespace -v 4
Deleting Provider="bootstrap-kubeadm" Version="v1.2.3" Namespace="capi-kubeadm-bootstrap-system"
Deleting Provider="control-plane-kubeadm" Version="v1.2.3" Namespace="capi-kubeadm-control-plane-system"
Deleting Provider="cluster-api" Version="v1.2.3" Namespace="capi-system"
Deleting Provider="infrastructure-ipam" Version="v1.2.0" Namespace="capm3-system"  ## This is a provider installed by us
Error: failed to list objects for the "cluster.x-k8s.io/v1beta1, Kind=Machine" GroupVersionKind: action failed after 9 attempts: the server could not find the requested resource (get machines.cluster.x-k8s.io)

$ clusterctl delete --all --include-crd --include-namespace -v 4
Error: failed to check Cluster API version: customresourcedefinitions.apiextensions.k8s.io "clusters.cluster.x-k8s.io" not found

$ k get crd |grep x-k8s
providers.clusterctl.cluster.x-k8s.io     2022-10-14T10:40:16Z

What did you expect to happen:
After the 1st run of clusterctl failed, we're able to run the clusterctl again and it deletes all remaining providers.

Anything else you would like to add:
The 2nd run of clusterctl failed because clusterctl requires the "clusters.cluster.x-k8s.io" CRD must exist (see source code) but this CRD was already deleted in the 1st run of clusterctl.

If I manually create the "clusters.cluster.x-k8s.io" CRD (similiar to clusterClient.ProviderInventory().EnsureCustomResourceDefinitions() at L74 in delete.go), the 2nd run of clusterctl is able to delete all remaining providers. If the 2nd run failed too due to failed to list objects for the ..., we can run clusterctl multi times until it succeeds.

So my question is:

• is calling CheckCAPIContract() necessary at the beginning of clusterctl delete? CheckCAPIContract() ensures CAPI is installed by checking "clusters.cluster.x-k8s.io" CRD existence.
• or can we add the option CheckCAPIContractOption.AllowCAPINotInstalled in DeleteOptions and pass it to CheckCAPIContract(options ...CheckCAPIContractOption)?

Environment:

• clusterctl version: &version.Info{Major:"1", Minor:"2", GitVersion:"v1.2.3", GitCommit:"bb377163f141d69b7a61479756ee96891f6670bd", GitTreeState:"clean", BuildDate:"2022-10-10T12:30:14Z", GoVersion:"go1.18.7", Compiler:"gc", Platform:"darwin/arm64"}
• kind version: kind v0.12.0 go1.17.8 darwin/arm64,
• Kubernetes version: (use kubectl version): v1.23.4
• OS (e.g. from /etc/os-release):

/kind bug
/area clusterctl

[fabriziopandini]

/triage accepted
I think it makes sense to investigate if we can make delete more resilient/re-entrant; another option that we can explore is to delete providers is an ordered fashion (so we delete CAPI only at the end)

[jessehu]

Thanks. But this won't work if delete CAPI provider (at the end) fails in the middle.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[cnmcavoy]

Hit this today myself when deleting some clusters, clusterctl delete failed in one and then repeated deletes would not remove the CRDs or other resources. I actually had to re-install CAPI, then issue the delete again.

clusterctl and CAPI v1.5.1

[jessehu]

We call cmd/clusterctl/client.Delete() like this to bypass this issue.

retry:
	if err := s.ensureClusterAPICRDExist(); err != nil {
		return err
	}
	if err := cli.Delete(options); err != nil {
		log.Printf("Uninstalling Cluster API components failed: %s", err)
		if s.opts.Retry > 0 {
			log.Printf("Will retry in 3 seconds")
			s.opts.Retry--
			time.Sleep(time.Second * 3)
			goto retry
		}
		return err
	}
	if err := s.deleteClusterAPICRD(); err != nil {
		return err
	}

	log.Printf("Uninstalling Cluster API components succeeded")
	return nil


//go:embed manifests/capi-clusters-crd.yaml
var capiClustersCRDManifest []byte

// ensureClusterAPICRDExist installs the Cluster API Clusters CRD, to make clusterctlclient.Client.Delete() idempotent.
// If clusterctlclient.Client.Delete() failed for the first time and deleted the Cluster API Clusters CRD, the 2nd run of Delete() will fail too
// because the Cluster API Clusters CRD doesn't exist. This bug is caused by https://github.com/kubernetes-sigs/cluster-api/blob/v1.2.3/cmd/clusterctl/client/delete.go#L62-L69).
func (s *lifecycle) ensureClusterAPICRDExist() error {
	capiCRD, err := k8sutil.NewManifestsParser(capiClustersCRDManifest, k8sutil.SortObject(s.client.RESTMapper())).Parse()
	if err != nil {
		return err
	}
	if err = capiCRD.Apply(s.ctx, s.client); err != nil {
		log.Printf("Creating CAPI clusters CRD failed")
		return err
	}
	return nil
}

[fabriziopandini]

I would prefer to explore deleting in an ordered fashion (we are already using an order between providers for init and upgrade) vs partially re-installing CAPI

[jessehu]

I have described the root cause in this issue. It's not about deleting in an ordered fashion.

[fabriziopandini]

I think that deleting in an ordered fashion with CAPI last will be enough to make the command idempotent, at least up to the point where CAPI CRD is deleted

[fabriziopandini]

/priority important-longterm