From f5a9d76759c6aace9cd47ebda69b921d8357bf76 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Tue, 12 Oct 2021 14:56:48 +0200 Subject: [PATCH] Fix clusterctl delete when deleting providers with cluster-wide resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stefan Büringer buringerst@vmware.com --- cmd/clusterctl/client/cluster/proxy.go | 58 +++++++++++++++++++++++++- test/e2e/clusterctl_upgrade.go | 7 ++++ test/framework/clusterctl/client.go | 26 ++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/cmd/clusterctl/client/cluster/proxy.go b/cmd/clusterctl/client/cluster/proxy.go index 7a1123301184..eec7495ec959 100644 --- a/cmd/clusterctl/client/cluster/proxy.go +++ b/cmd/clusterctl/client/cluster/proxy.go @@ -22,14 +22,18 @@ import ( "time" "github.com/pkg/errors" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" utilversion "k8s.io/apimachinery/pkg/util/version" "k8s.io/client-go/discovery" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/cmd/clusterctl/internal/scheme" "sigs.k8s.io/cluster-api/version" "sigs.k8s.io/controller-runtime/pkg/client" @@ -56,7 +60,10 @@ type Proxy interface { // CheckClusterAvailable checks if a a cluster is available and reachable. CheckClusterAvailable() error - // ListResources returns all the Kubernetes objects with the given labels existing the listed namespaces. + // ListResources lists namespaced and cluster-wide resources matching the labels. Namespaced resources are only listed + // in the given namespaces. + // If labels contains the ProviderLabelName label, CRDs of other providers are excluded. + // This is done to avoid errors when listing resources of providers which have already been deleted. ListResources(labels map[string]string, namespaces ...string) ([]unstructured.Unstructured, error) // GetContexts returns the list of contexts in kubeconfig which begin with prefix. @@ -198,6 +205,17 @@ func (k *proxy) CheckClusterAvailable() error { return nil } +// ListResources lists namespaced and cluster-wide resources matching the labels. Namespaced resources are only listed +// in the given namespaces. +// If labels contains the ProviderLabelName label, CRDs of other providers are excluded. +// This is done to avoid errors when listing resources of providers which have already been deleted. +// For example: +// * The AWS provider has already been deleted, but there are still cluster-wide resources of AWSClusterControllerIdentity. +// * The AWSClusterControllerIdentity resources are still stored in an older version (e.g. v1alpha4, when the preferred +// version is v1beta1) +// * If we now want to delete e.g. the kubeadm bootstrap provider, we cannot list AWSClusterControllerIdentity resources +// as the conversion would fail, because the AWS controller hosting the conversion webhook has already been deleted. +// * Thus we exclude resources of other providers if we detect that ListResources is called to list resources of a provider. func (k *proxy) ListResources(labels map[string]string, namespaces ...string) ([]unstructured.Unstructured, error) { cs, err := k.newClientSet() if err != nil { @@ -219,6 +237,31 @@ func (k *proxy) ListResources(labels map[string]string, namespaces ...string) ([ return nil, errors.Wrap(err, "failed to list api resources") } + // If labels indicates that resources of a specific provider should be listed, exclude CRDs of other providers. + crdsToExclude := sets.String{} + if providerName, ok := labels[clusterv1.ProviderLabelName]; ok { + // List all CRDs in the cluster. + crdList := &apiextensionsv1.CustomResourceDefinitionList{} + if err := retryWithExponentialBackoff(newReadBackoff(), func() error { + return c.List(ctx, crdList) + }); err != nil { + return nil, errors.Wrap(err, "failed to list CRDs") + } + + // Exclude CRDs of other providers. + for _, crd := range crdList.Items { + if v, ok := crd.Labels[clusterv1.ProviderLabelName]; ok && v != providerName { + for _, version := range crd.Spec.Versions { + crdsToExclude.Insert(metav1.GroupVersionKind{ + Group: crd.Spec.Group, + Version: version.Name, + Kind: crd.Spec.Names.Kind, + }.String()) + } + } + } + } + // Select resources with list and delete methods (list is required by this method, delete by the callers of this method) resourceList = discovery.FilteredBy(discovery.SupportsAllVerbs{Verbs: []string{"list", "delete"}}, resourceList) @@ -231,6 +274,19 @@ func (k *proxy) ListResources(labels map[string]string, namespaces ...string) ([ continue } + // Continue if the resource is an excluded CRD. + gv, err := schema.ParseGroupVersion(resourceGroup.GroupVersion) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse GroupVersion") + } + if crdsToExclude.Has(metav1.GroupVersionKind{ + Group: gv.Group, + Version: gv.Version, + Kind: resourceKind.Kind, + }.String()) { + continue + } + // List all the object instances of this resourceKind with the given labels if resourceKind.Namespaced { for _, namespace := range namespaces { diff --git a/test/e2e/clusterctl_upgrade.go b/test/e2e/clusterctl_upgrade.go index 19e7f9f12729..b2973b69a93c 100644 --- a/test/e2e/clusterctl_upgrade.go +++ b/test/e2e/clusterctl_upgrade.go @@ -362,6 +362,13 @@ func ClusterctlUpgradeSpec(ctx context.Context, inputGetter func() ClusterctlUpg Deleter: managementClusterProxy.GetClient(), Name: testNamespace.Name, }) + + Byf("Deleting providers") + clusterctl.Delete(ctx, clusterctl.DeleteInput{ + LogFolder: filepath.Join(input.ArtifactFolder, "clusters", managementClusterResources.Cluster.Name), + ClusterctlConfigPath: input.ClusterctlConfigPath, + KubeconfigPath: managementClusterProxy.GetKubeconfigPath(), + }) } testCancelWatches() } diff --git a/test/framework/clusterctl/client.go b/test/framework/clusterctl/client.go index 80fa7ce036a3..34c95b1dbdb3 100644 --- a/test/framework/clusterctl/client.go +++ b/test/framework/clusterctl/client.go @@ -137,6 +137,32 @@ func Upgrade(ctx context.Context, input UpgradeInput) { Expect(err).ToNot(HaveOccurred(), "failed to run clusterctl upgrade") } +// DeleteInput is the input for Delete. +type DeleteInput struct { + LogFolder string + ClusterctlConfigPath string + KubeconfigPath string +} + +// Delete calls clusterctl delete --all. +func Delete(_ context.Context, input DeleteInput) { + log.Logf("clusterctl delete --all") + + deleteOpts := clusterctlclient.DeleteOptions{ + Kubeconfig: clusterctlclient.Kubeconfig{ + Path: input.KubeconfigPath, + Context: "", + }, + DeleteAll: true, + } + + clusterctlClient, log := getClusterctlClientWithLogger(input.ClusterctlConfigPath, "clusterctl-delete.log", input.LogFolder) + defer log.Close() + + err := clusterctlClient.Delete(deleteOpts) + Expect(err).ToNot(HaveOccurred(), "failed to run clusterctl upgrade") +} + // ConfigClusterInput is the input for ConfigCluster. type ConfigClusterInput struct { LogFolder string