update

cmd/clusterctl/client/cluster/components.go

@@ -17,7 +17,6 @@ limitations under the License.
 package cluster
 
 import (
-	"context"
 	"fmt"
 	"strings"
 
@@ -28,6 +27,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/internal/util"
 	logf "sigs.k8s.io/cluster-api/cmd/clusterctl/log"
@@ -132,7 +132,13 @@ func (p *providerComponents) Delete(options DeleteOptions) error {
 
 	// Fetch all the components belonging to a provider.
 	// We want that the delete operation is able to clean-up everything.
-	resources, err := p.proxy.ListProviderResources(context.TODO(), options.Provider.ManifestLabel(), options.Provider.Namespace)
+	labels := map[string]string{
+		clusterctlv1.ClusterctlLabelName: "",
+		clusterv1.ProviderLabelName:      options.Provider.ManifestLabel(),
+	}
+
+	namespaces := []string{options.Provider.Namespace}
+	resources, err := p.proxy.ListResources(labels, namespaces...)
 	if err != nil {
 		return err
 	}

cmd/clusterctl/client/cluster/proxy.go

@@ -17,7 +17,6 @@ limitations under the License.
 package cluster
 
 import (
-	"context"
 	"fmt"
 	"strings"
 	"time"
@@ -35,7 +34,6 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
-	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/internal/scheme"
 	"sigs.k8s.io/cluster-api/version"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -62,12 +60,12 @@ 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)
 
-	// ListProviderResources lists Kubernetes objects of a specific provider.
-	ListProviderResources(ctx context.Context, provider, namespace string) ([]unstructured.Unstructured, error)
-
 	// GetContexts returns the list of contexts in kubeconfig which begin with prefix.
 	GetContexts(prefix string) ([]string, error)
 
@@ -207,69 +205,18 @@ func (k *proxy) CheckClusterAvailable() error {
 	return nil
 }
 
-func (k *proxy) ListResources(labelss map[string]string, namespaces ...string) ([]unstructured.Unstructured, error) {
-	return k.list(context.TODO(), &listOptions{Labels: labelss, Namespaces: namespaces})
-}
-
-func (k *proxy) ListProviderResources(ctx context.Context, provider, namespace string) ([]unstructured.Unstructured, error) {
-	return k.list(ctx, &listOptions{
-		Labels: map[string]string{
-			clusterctlv1.ClusterctlLabelName: "",
-			clusterv1.ProviderLabelName:      provider,
-		},
-		Namespaces: []string{namespace},
-		ExcludeCRD: func(crd apiextensionsv1.CustomResourceDefinition) bool {
-			if v, ok := crd.Labels[clusterv1.ProviderLabelName]; ok && v != provider {
-				return true
-			}
-			return false
-		},
-	})
-}
-
-// listOption is some configuration that modifies options for a list request.
-type listOption interface {
-	// ApplyToList applies this configuration to the given list options.
-	ApplyToList(*listOptions)
-}
-
-type listOptions struct {
-	// Labels filters resources by labels.
-	Labels map[string]string
-
-	// Namespaces represents the namespaces to list for.
-	Namespaces []string
-
-	// ExcludeCRD excludes resources of CRDs if the func returns true for them.
-	ExcludeCRD func(definition apiextensionsv1.CustomResourceDefinition) bool
-}
-
-// ApplyToList implements listOption for listOptions.
-func (o *listOptions) ApplyToList(lo *listOptions) {
-	if o.Labels != nil {
-		lo.Labels = o.Labels
-	}
-	if o.Namespaces != nil {
-		lo.Namespaces = o.Namespaces
-	}
-	if o.ExcludeCRD != nil {
-		lo.ExcludeCRD = o.ExcludeCRD
-	}
-}
-
-// ApplyOptions applies the given list options on these options,
-// and then returns itself (for convenient chaining).
-func (o *listOptions) ApplyOptions(opts []listOption) *listOptions {
-	for _, opt := range opts {
-		opt.ApplyToList(o)
-	}
-	return o
-}
-
-func (k *proxy) list(ctx context.Context, opts ...listOption) ([]unstructured.Unstructured, error) {
-	listOpts := listOptions{}
-	listOpts.ApplyOptions(opts)
-
+// 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 {
 		return nil, err
@@ -290,19 +237,20 @@ func (k *proxy) list(ctx context.Context, opts ...listOption) ([]unstructured.Un
 		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 listOpts.ExcludeCRD != nil {
-		// Get all CRDs in the cluster.
-		crdListBackoff := newReadBackoff()
+	if providerName, ok := labels[clusterv1.ProviderLabelName]; ok {
+		// List all CRDs in the cluster.
 		crdList := &apiextensionsv1.CustomResourceDefinitionList{}
-		if err := retryWithExponentialBackoff(crdListBackoff, func() error {
+		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 listOpts.ExcludeCRD(crd) {
+			if v, ok := crd.Labels[clusterv1.ProviderLabelName]; ok && v != providerName {
 				for _, version := range crd.Spec.Versions {
 					crdsToExclude.Insert(metav1.GroupVersionKind{
 						Group:   crd.Spec.Group,
@@ -326,6 +274,7 @@ func (k *proxy) list(ctx context.Context, opts ...listOption) ([]unstructured.Un
 				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")
@@ -340,15 +289,15 @@ func (k *proxy) list(ctx context.Context, opts ...listOption) ([]unstructured.Un
 
 			// List all the object instances of this resourceKind with the given labels
 			if resourceKind.Namespaced {
-				for _, namespace := range listOpts.Namespaces {
-					objList, err := listObjByGVK(c, resourceGroup.GroupVersion, resourceKind.Kind, []client.ListOption{client.MatchingLabels(listOpts.Labels), client.InNamespace(namespace)})
+				for _, namespace := range namespaces {
+					objList, err := listObjByGVK(c, resourceGroup.GroupVersion, resourceKind.Kind, []client.ListOption{client.MatchingLabels(labels), client.InNamespace(namespace)})
 					if err != nil {
 						return nil, err
 					}
 					ret = append(ret, objList.Items...)
 				}
 			} else {
-				objList, err := listObjByGVK(c, resourceGroup.GroupVersion, resourceKind.Kind, []client.ListOption{client.MatchingLabels(listOpts.Labels)})
+				objList, err := listObjByGVK(c, resourceGroup.GroupVersion, resourceKind.Kind, []client.ListOption{client.MatchingLabels(labels)})
 				if err != nil {
 					return nil, err
 				}

cmd/clusterctl/internal/test/fake_proxy.go

@@ -17,7 +17,6 @@ limitations under the License.
 package test
 
 import (
-	"context"
 	"errors"
 
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -136,14 +135,6 @@ func (f *FakeProxy) ListResources(labels map[string]string, namespaces ...string
 	return ret, nil
 }
 
-func (f *FakeProxy) ListProviderResources(_ context.Context, provider, namespace string) ([]unstructured.Unstructured, error) {
-	// TODO: this func should filter out objects of CRDs of other providers
-	return f.ListResources(map[string]string{
-		clusterctlv1.ClusterctlLabelName: "",
-		clusterv1.ProviderLabelName:      provider,
-	}, namespace)
-}
-
 func (f *FakeProxy) GetContexts(prefix string) ([]string, error) {
 	return nil, nil
 }

test/e2e/clusterctl_upgrade.go

@@ -319,6 +319,13 @@ func ClusterctlUpgradeSpec(ctx context.Context, inputGetter func() ClusterctlUpg
 
 		By("THE UPGRADED MANAGEMENT CLUSTER WORKS!")
 
+		By("Deleting providers")
+		clusterctl.Delete(ctx, clusterctl.DeleteInput{
+			LogFolder:            filepath.Join(input.ArtifactFolder, "clusters", cluster.Name),
+			ClusterctlConfigPath: input.ClusterctlConfigPath,
+			KubeconfigPath:       managementClusterProxy.GetKubeconfigPath(),
+		})
+
 		By("PASSED!")
 	})
 

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(ctx 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