Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fix clusterctl delete when deleting providers with cluster-wide resources #5420

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion cmd/clusterctl/client/cluster/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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)

Expand All @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/clusterctl_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
26 changes: 26 additions & 0 deletions test/framework/clusterctl/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down