Merge pull request #14724 from hakman/refactor_list-resources

Refactor ListResources to not require passing the Cluster object

cmd/kops/delete_cluster.go

@@ -126,7 +126,7 @@ func RunDeleteCluster(ctx context.Context, f *util.Factory, out io.Writer, optio
 		}
 
 		klog.Info("Looking for cloud resources to delete")
-		allResources, err := resourceops.ListResources(cloud, cluster, options.Region)
+		allResources, err := resourceops.ListResources(cloud, cluster)
 		if err != nil {
 			return err
 		}

cmd/kops/toolbox_dump.go

@@ -121,8 +121,7 @@ func RunToolboxDump(ctx context.Context, f commandutils.Factory, out io.Writer,
 		return err
 	}
 
-	region := "" // Use default
-	resourceMap, err := resourceops.ListResources(cloud, cluster, region)
+	resourceMap, err := resourceops.ListResources(cloud, cluster)
 	if err != nil {
 		return err
 	}

pkg/resources/aws/aws.go

@@ -32,7 +32,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/route53"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2"
-	kopsapi "k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/dns"
 	"k8s.io/kops/pkg/featureflag"
 	"k8s.io/kops/pkg/resources"
 	"k8s.io/kops/pkg/resources/spotinst"
@@ -50,8 +50,10 @@ const (
 
 type listFn func(fi.Cloud, string) ([]*resources.Resource, error)
 
-func ListResourcesAWS(cloud awsup.AWSCloud, cluster *kopsapi.Cluster) (map[string]*resources.Resource, error) {
-	clusterName := cluster.Name
+func ListResourcesAWS(cloud awsup.AWSCloud, clusterInfo resources.ClusterInfo) (map[string]*resources.Resource, error) {
+	clusterName := clusterInfo.Name
+	clusterUsesNoneDNS := clusterInfo.UsesNoneDNS
+
 	resourceTrackers := make(map[string]*resources.Resource)
 
 	// These are the functions that are used for looking up
@@ -87,7 +89,7 @@ func ListResourcesAWS(cloud awsup.AWSCloud, cluster *kopsapi.Cluster) (map[strin
 		ListEventBridgeRules,
 	}
 
-	if !cluster.IsGossip() && !cluster.UsesNoneDNS() {
+	if !dns.IsGossipClusterName(clusterName) && !clusterUsesNoneDNS {
 		// Route 53
 		listFunctions = append(listFunctions, ListRoute53Records)
 	}

pkg/resources/azure/azure.go

@@ -24,7 +24,6 @@ import (
 	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-05-01/network"
 	authz "github.com/Azure/azure-sdk-for-go/services/preview/authorization/mgmt/2020-04-01-preview/authorization"
 	azureresources "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2021-04-01/resources"
-	kopsapi "k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/resources"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/azure"
@@ -44,21 +43,21 @@ const (
 )
 
 // ListResourcesAzure lists all resources for the cluster by quering Azure.
-func ListResourcesAzure(cloud azure.AzureCloud, cluster *kopsapi.Cluster) (map[string]*resources.Resource, error) {
+func ListResourcesAzure(cloud azure.AzureCloud, clusterInfo resources.ClusterInfo) (map[string]*resources.Resource, error) {
 	g := resourceGetter{
-		cloud:   cloud,
-		cluster: cluster,
+		cloud:       cloud,
+		clusterInfo: clusterInfo,
 	}
 	return g.listResourcesAzure()
 }
 
 type resourceGetter struct {
-	cloud   azure.AzureCloud
-	cluster *kopsapi.Cluster
+	cloud       azure.AzureCloud
+	clusterInfo resources.ClusterInfo
 }
 
 func (g *resourceGetter) resourceGroupName() string {
-	return g.cluster.AzureResourceGroupName()
+	return g.clusterInfo.AzureResourceGroupName
 }
 
 func (g *resourceGetter) listResourcesAzure() (map[string]*resources.Resource, error) {
@@ -129,7 +128,7 @@ func (g *resourceGetter) toResourceGroupResource(rg *azureresources.Group) *reso
 		ID:      *rg.Name,
 		Name:    *rg.Name,
 		Deleter: g.deleteResourceGroup,
-		Shared:  g.cluster.IsSharedAzureResourceGroup(),
+		Shared:  g.clusterInfo.AzureResourceGroupShared,
 	}
 }
 
@@ -168,7 +167,7 @@ func (g *resourceGetter) toVirtualNetworkResource(vnet *network.VirtualNetwork)
 		Name:    *vnet.Name,
 		Deleter: g.deleteVirtualNetwork,
 		Blocks:  []string{toKey(typeResourceGroup, g.resourceGroupName())},
-		Shared:  g.cluster.SharedVPC(),
+		Shared:  g.clusterInfo.AzureNetworkShared,
 	}
 }
 
@@ -202,7 +201,7 @@ func (g *resourceGetter) toSubnetResource(subnet *network.Subnet, vnetName strin
 			toKey(typeVirtualNetwork, vnetName),
 			toKey(typeResourceGroup, g.resourceGroupName()),
 		},
-		Shared: g.cluster.SharedVPC(),
+		Shared: g.clusterInfo.AzureNetworkShared,
 	}
 }
 
@@ -235,7 +234,7 @@ func (g *resourceGetter) toRouteTableResource(rt *network.RouteTable) *resources
 		Name:    *rt.Name,
 		Deleter: g.deleteRouteTable,
 		Blocks:  []string{toKey(typeResourceGroup, g.resourceGroupName())},
-		Shared:  g.cluster.IsSharedAzureRouteTable(),
+		Shared:  g.clusterInfo.AzureRouteTableShared,
 	}
 }
 
@@ -469,7 +468,7 @@ func (g *resourceGetter) deletePublicIPAddress(_ fi.Cloud, r *resources.Resource
 // isOwnedByCluster returns true if the resource is owned by the cluster.
 func (g *resourceGetter) isOwnedByCluster(tags map[string]*string) bool {
 	for k, v := range tags {
-		if k == azure.TagClusterName && *v == g.cluster.Name {
+		if k == azure.TagClusterName && *v == g.clusterInfo.Name {
 			return true
 		}
 	}

pkg/resources/azure/azure_test.go

@@ -26,8 +26,6 @@ import (
 	authz "github.com/Azure/azure-sdk-for-go/services/preview/authorization/mgmt/2020-04-01-preview/authorization"
 	azureresources "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2021-04-01/resources"
 	"github.com/Azure/go-autorest/autorest/to"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/resources"
 	"k8s.io/kops/upup/pkg/fi/cloudup/azure"
 	"k8s.io/kops/upup/pkg/fi/cloudup/azuretasks"
@@ -176,17 +174,10 @@ func TestListResourcesAzure(t *testing.T) {
 	// Call listResourcesAzure.
 	g := resourceGetter{
 		cloud: cloud,
-		cluster: &kops.Cluster{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: clusterName,
-			},
-			Spec: kops.ClusterSpec{
-				CloudProvider: kops.CloudProviderSpec{
-					Azure: &kops.AzureSpec{
-						ResourceGroupName: rgName,
-					},
-				},
-			},
+		clusterInfo: resources.ClusterInfo{
+			Name:                     clusterName,
+			AzureResourceGroupName:   rgName,
+			AzureResourceGroupShared: true,
 		},
 	}
 	actual, err := g.listResourcesAzure()
@@ -310,10 +301,8 @@ func TestIsOwnedByCluster(t *testing.T) {
 	for i, tc := range testCases {
 		t.Run(fmt.Sprintf("test case %d", i), func(t *testing.T) {
 			g := &resourceGetter{
-				cluster: &kops.Cluster{
-					ObjectMeta: metav1.ObjectMeta{
-						Name: clusterName,
-					},
+				clusterInfo: resources.ClusterInfo{
+					Name: clusterName,
 				},
 			}
 			a := g.isOwnedByCluster(tc.tags)

pkg/resources/clusterinfo.go

@@ -0,0 +1,27 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package resources
+
+type ClusterInfo struct {
+	Name        string
+	UsesNoneDNS bool
+	// Azure specific
+	AzureResourceGroupName   string
+	AzureResourceGroupShared bool
+	AzureNetworkShared       bool
+	AzureRouteTableShared    bool
+}

pkg/resources/digitalocean/resources.go

@@ -45,7 +45,9 @@ const (
 
 type listFn func(fi.Cloud, string) ([]*resources.Resource, error)
 
-func ListResources(cloud do.DOCloud, clusterName string) (map[string]*resources.Resource, error) {
+func ListResources(cloud do.DOCloud, clusterInfo resources.ClusterInfo) (map[string]*resources.Resource, error) {
+	clusterName := clusterInfo.Name
+
 	resourceTrackers := make(map[string]*resources.Resource)
 
 	listFunctions := []listFn{

pkg/resources/gce/gce.go

@@ -61,12 +61,13 @@ const maxPrefixTokens = 5
 // Maximum length of a GCE route name
 const maxGCERouteNameLength = 63
 
-func ListResourcesGCE(gceCloud gce.GCECloud, clusterName string, region string) (map[string]*resources.Resource, error) {
+func ListResourcesGCE(gceCloud gce.GCECloud, clusterInfo resources.ClusterInfo) (map[string]*resources.Resource, error) {
+	clusterName := clusterInfo.Name
+	clusterUsesNoneDNS := clusterInfo.UsesNoneDNS
+
 	ctx := context.TODO()
 
-	if region == "" {
-		region = gceCloud.Region()
-	}
+	region := gceCloud.Region()
 
 	resources := make(map[string]*resources.Resource)
 
@@ -105,7 +106,6 @@ func ListResourcesGCE(gceCloud gce.GCECloud, clusterName string, region string)
 		d.listForwardingRules,
 		d.listFirewallRules,
 		d.listGCEDisks,
-		d.listGCEDNSZone,
 		// TODO: Find routes via instances (via instance groups)
 		d.listAddresses,
 		d.listSubnets,
@@ -115,6 +115,10 @@ func ListResourcesGCE(gceCloud gce.GCECloud, clusterName string, region string)
 		d.listBackendServices,
 		d.listHealthchecks,
 	}
+	if !dns.IsGossipClusterName(clusterName) && !clusterUsesNoneDNS {
+		listFunctions = append(listFunctions, d.listGCEDNSZone)
+	}
+
 	for _, fn := range listFunctions {
 		resourceTrackers, err := fn()
 		if err != nil {
@@ -1282,10 +1286,6 @@ func (d *clusterDiscoveryGCE) isKopsManagedDNSName(name string) bool {
 }
 
 func (d *clusterDiscoveryGCE) listGCEDNSZone() ([]*resources.Resource, error) {
-	if dns.IsGossipClusterName(d.clusterName) {
-		return nil, nil
-	}
-
 	var resourceTrackers []*resources.Resource
 
 	managedZones, err := d.gceCloud.CloudDNS().ManagedZones().List(d.gceCloud.Project())

pkg/resources/hetzner/resources.go

@@ -39,7 +39,9 @@ const (
 
 type listFn func(fi.Cloud, string) ([]*resources.Resource, error)
 
-func ListResources(cloud hetzner.HetznerCloud, clusterName string) (map[string]*resources.Resource, error) {
+func ListResources(cloud hetzner.HetznerCloud, clusterInfo resources.ClusterInfo) (map[string]*resources.Resource, error) {
+	clusterName := clusterInfo.Name
+
 	resourceTrackers := make(map[string]*resources.Resource)
 
 	listFunctions := []listFn{

pkg/resources/openstack/openstack.go

@@ -31,13 +31,13 @@ type clusterDiscoveryOS struct {
 }
 
 // ListResources lists the OpenStack resources kops manages
-func ListResources(cloud openstack.OpenstackCloud, clusterName string) (map[string]*resources.Resource, error) {
+func ListResources(cloud openstack.OpenstackCloud, clusterInfo resources.ClusterInfo) (map[string]*resources.Resource, error) {
 	resources := make(map[string]*resources.Resource)
 
 	os := &clusterDiscoveryOS{
 		cloud:       cloud,
 		osCloud:     cloud,
-		clusterName: clusterName,
+		clusterName: clusterInfo.Name,
 	}
 
 	listFunctions := []openstackListFn{

pkg/resources/ops/collector.go

@@ -37,21 +37,29 @@ import (
 )
 
 // ListResources collects the resources from the specified cloud
-func ListResources(cloud fi.Cloud, cluster *kops.Cluster, region string) (map[string]*resources.Resource, error) {
-	clusterName := cluster.Name
+func ListResources(cloud fi.Cloud, cluster *kops.Cluster) (map[string]*resources.Resource, error) {
+	clusterInfo := resources.ClusterInfo{
+		Name:        cluster.Name,
+		UsesNoneDNS: cluster.UsesNoneDNS(),
+	}
+
 	switch cloud.ProviderID() {
 	case kops.CloudProviderAWS:
-		return aws.ListResourcesAWS(cloud.(awsup.AWSCloud), cluster)
+		return aws.ListResourcesAWS(cloud.(awsup.AWSCloud), clusterInfo)
 	case kops.CloudProviderDO:
-		return digitalocean.ListResources(cloud.(clouddo.DOCloud), clusterName)
+		return digitalocean.ListResources(cloud.(clouddo.DOCloud), clusterInfo)
 	case kops.CloudProviderGCE:
-		return gce.ListResourcesGCE(cloud.(cloudgce.GCECloud), clusterName, region)
+		return gce.ListResourcesGCE(cloud.(cloudgce.GCECloud), clusterInfo)
 	case kops.CloudProviderHetzner:
-		return hetzner.ListResources(cloud.(cloudhetzner.HetznerCloud), clusterName)
+		return hetzner.ListResources(cloud.(cloudhetzner.HetznerCloud), clusterInfo)
 	case kops.CloudProviderOpenstack:
-		return openstack.ListResources(cloud.(cloudopenstack.OpenstackCloud), clusterName)
+		return openstack.ListResources(cloud.(cloudopenstack.OpenstackCloud), clusterInfo)
 	case kops.CloudProviderAzure:
-		return azure.ListResourcesAzure(cloud.(cloudazure.AzureCloud), cluster)
+		clusterInfo.AzureResourceGroupName = cluster.AzureResourceGroupName()
+		clusterInfo.AzureResourceGroupShared = cluster.IsSharedAzureResourceGroup()
+		clusterInfo.AzureNetworkShared = cluster.SharedVPC()
+		clusterInfo.AzureRouteTableShared = cluster.IsSharedAzureRouteTable()
+		return azure.ListResourcesAzure(cloud.(cloudazure.AzureCloud), clusterInfo)
 	default:
 		return nil, fmt.Errorf("delete on clusters on %q not (yet) supported", cloud.ProviderID())
 	}