Skip to content

Commit

Permalink
OCPBUGS-44193: Use client side filtering for gcp destroy
Browse files Browse the repository at this point in the history 
** The destroy code is using server side filtering on resources. The number of resources that
are filtered out (server side) are causing quota limits to be reached. Moving the filtering
to the client side will limit quota max errors.
  • Loading branch information
@barbacbd
barbacbd authored and openshift-cherrypick-robot committed Dec 6, 2024
1 parent d5b9686 commit 179cd9a
Show file tree
Hide file tree
Showing 21 changed files with 522 additions and 439 deletions.
92 changes: 47 additions & 45 deletions pkg/destroy/gcp/address.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,53 @@ const (
)

func (o *ClusterUninstaller) listAddresses(ctx context.Context, typeName string) ([]cloudResource, error) {
return o.listAddressesWithFilter(ctx, typeName, "items(name,region,addressType),nextPageToken", o.clusterIDFilter())
return o.listAddressesWithFilter(ctx, typeName, "items(name,region,addressType),nextPageToken", o.isClusterResource)
}

// listAddressesWithFilter lists addresses in the project that satisfy the filter criteria.
// The fields parameter specifies which fields should be returned in the result, the filter string contains
// a filter string passed to the API to filter results.
func (o *ClusterUninstaller) listAddressesWithFilter(ctx context.Context, typeName, fields, filter string) ([]cloudResource, error) {
func (o *ClusterUninstaller) listAddressesWithFilter(ctx context.Context, typeName, fields string, filterFunc resourceFilterFunc) ([]cloudResource, error) {
o.Logger.Debugf("Listing addresses")
result := []cloudResource{}

pagesFunc := func(list *compute.AddressList) error {
for _, item := range list.Items {
o.Logger.Debugf("Found address (%s): %s", typeName, item.Name)
if filterFunc(item.Name) {
var quota []gcp.QuotaUsage
if item.AddressType == "INTERNAL" {
quota = []gcp.QuotaUsage{{
Metric: &gcp.Metric{
Service: gcp.ServiceComputeEngineAPI,
Limit: "internal_addresses",
Dimensions: map[string]string{
"region": getNameFromURL("regions", item.Region),
},
},
Amount: 1,
}}
}
result = append(result, cloudResource{
key: item.Name,
name: item.Name,
typeName: typeName,
quota: quota,
})
}
}
return nil
}

ctx, cancel := context.WithTimeout(ctx, defaultTimeout)
defer cancel()

var err error
var list *compute.AddressList
switch typeName {
case globalAddressResource:
list, err = o.computeSvc.GlobalAddresses.List(o.ProjectID).Filter(filter).Fields(googleapi.Field(fields)).Context(ctx).Do()
err = o.computeSvc.GlobalAddresses.List(o.ProjectID).Fields(googleapi.Field(fields)).Pages(ctx, pagesFunc)
case regionalAddressResource:
list, err = o.computeSvc.Addresses.List(o.ProjectID, o.Region).Filter(filter).Fields(googleapi.Field(fields)).Context(ctx).Do()
err = o.computeSvc.Addresses.List(o.ProjectID, o.Region).Fields(googleapi.Field(fields)).Pages(ctx, pagesFunc)
default:
return nil, fmt.Errorf("invalid address type %q", typeName)
}
Expand All @@ -43,25 +71,6 @@ func (o *ClusterUninstaller) listAddressesWithFilter(ctx context.Context, typeNa
return nil, fmt.Errorf("failed to list addresses: %w", err)
}

result := []cloudResource{}
for _, item := range list.Items {
o.Logger.Debugf("Found address: %s, type: %s", item.Name, item.AddressType)
result = append(result, cloudResource{
key: item.Name,
name: item.Name,
typeName: typeName,
quota: []gcp.QuotaUsage{{
Metric: &gcp.Metric{
Service: gcp.ServiceComputeEngineAPI,
Limit: "addresses",
Dimensions: map[string]string{
"region": getNameFromURL("regions", item.Region),
},
},
Amount: 1,
}},
})
}
return result, nil
}

Expand Down Expand Up @@ -100,31 +109,24 @@ func (o *ClusterUninstaller) deleteAddress(ctx context.Context, item cloudResour
// destroyAddresses removes all address resources that have a name prefixed
// with the cluster's infra ID.
func (o *ClusterUninstaller) destroyAddresses(ctx context.Context) error {
found, err := o.listAddresses(ctx, globalAddressResource)
if err != nil {
return err
}
items := o.insertPendingItems(globalAddressResource, found)

found, err = o.listAddresses(ctx, regionalAddressResource)
if err != nil {
return err
}
items = append(items, o.insertPendingItems(regionalAddressResource, found)...)

for _, item := range items {
err := o.deleteAddress(ctx, item)
addressTypes := []string{globalAddressResource, regionalAddressResource}
for _, addressType := range addressTypes {
found, err := o.listAddresses(ctx, addressType)
if err != nil {
o.errorTracker.suppressWarning(item.key, err, o.Logger)
return err
}
}
items := o.insertPendingItems(addressType, found)

if items = o.getPendingItems(globalAddressResource); len(items) > 0 {
return fmt.Errorf("%d global addresses pending", len(items))
}
for _, item := range items {
err := o.deleteAddress(ctx, item)
if err != nil {
o.errorTracker.suppressWarning(item.key, err, o.Logger)
}
}

if items = o.getPendingItems(regionalAddressResource); len(items) > 0 {
return fmt.Errorf("%d region addresses pending", len(items))
if items := o.getPendingItems(addressType); len(items) > 0 {
return fmt.Errorf("%d %s resources pending", len(items), addressType)
}
}

return nil
Expand Down
74 changes: 30 additions & 44 deletions pkg/destroy/gcp/backendservice.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"google.golang.org/api/compute/v1"
"google.golang.org/api/googleapi"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/openshift/installer/pkg/types/gcp"
)
Expand All @@ -17,41 +16,48 @@ const (
)

func (o *ClusterUninstaller) listBackendServices(ctx context.Context, typeName string) ([]cloudResource, error) {
return o.listBackendServicesWithFilter(ctx, typeName, "items(name),nextPageToken", o.clusterIDFilter(), nil)
}

func backendServiceBelongsToInstanceGroup(item *compute.BackendService, igURLs sets.Set[string]) bool {
if igURLs == nil {
return true
}

if len(item.Backends) == 0 {
return false
}
for _, backend := range item.Backends {
if !igURLs.Has(backend.Group) {
return false
}
}
return true
return o.listBackendServicesWithFilter(ctx, typeName, "items(name),nextPageToken",
func(item *compute.BackendService) bool {
return o.isClusterResource(item.Name)
})
}

// listBackendServicesWithFilter lists backend services in the project that satisfy the filter criteria.
// The fields parameter specifies which fields should be returned in the result, the filter string contains
// a filter string passed to the API to filter results.
func (o *ClusterUninstaller) listBackendServicesWithFilter(ctx context.Context, typeName, fields, filter string, urls sets.Set[string]) ([]cloudResource, error) {
// The fields parameter specifies which fields should be returned in the result.
func (o *ClusterUninstaller) listBackendServicesWithFilter(ctx context.Context, typeName, fields string, filterFunc func(item *compute.BackendService) bool) ([]cloudResource, error) {
o.Logger.Debugf("Listing backend services")
result := []cloudResource{}

ctx, cancel := context.WithTimeout(ctx, defaultTimeout)
defer cancel()

pagesFunc := func(list *compute.BackendServiceList) error {
for _, item := range list.Items {
o.Logger.Debugf("Found backend service: %s", item.Name)
if filterFunc(item) {
result = append(result, cloudResource{
key: item.Name,
name: item.Name,
typeName: typeName,
quota: []gcp.QuotaUsage{{
Metric: &gcp.Metric{
Service: gcp.ServiceComputeEngineAPI,
Limit: "backend_services",
},
Amount: 1,
}},
})
}
}
return nil
}

var err error
var list *compute.BackendServiceList
switch typeName {
case globalBackendServiceResource:
list, err = o.computeSvc.BackendServices.List(o.ProjectID).Filter(filter).Fields(googleapi.Field(fields)).Context(ctx).Do()
err = o.computeSvc.BackendServices.List(o.ProjectID).Fields(googleapi.Field(fields)).Pages(ctx, pagesFunc)
case regionBackendServiceResource:
list, err = o.computeSvc.RegionBackendServices.List(o.ProjectID, o.Region).Filter(filter).Fields(googleapi.Field(fields)).Context(ctx).Do()
err = o.computeSvc.RegionBackendServices.List(o.ProjectID, o.Region).Fields(googleapi.Field(fields)).Pages(ctx, pagesFunc)
default:
return nil, fmt.Errorf("invalid backend service type %q", typeName)
}
Expand All @@ -60,26 +66,6 @@ func (o *ClusterUninstaller) listBackendServicesWithFilter(ctx context.Context,
return nil, fmt.Errorf("failed to list backend services: %w", err)
}

result := []cloudResource{}
for _, item := range list.Items {
o.Logger.Debugf("Found backend service: %s", item.Name)
if !backendServiceBelongsToInstanceGroup(item, urls) {
o.Logger.Debug("No matching instance group for backend service: %s", item.Name)
continue
}
result = append(result, cloudResource{
key: item.Name,
name: item.Name,
typeName: typeName,
quota: []gcp.QuotaUsage{{
Metric: &gcp.Metric{
Service: gcp.ServiceComputeEngineAPI,
Limit: "backend_services",
},
Amount: 1,
}},
})
}
return result, nil
}

Expand Down
31 changes: 19 additions & 12 deletions pkg/destroy/gcp/bucket.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package gcp

import (
"context"
"fmt"
"regexp"
"strings"

"github.com/pkg/errors"
"google.golang.org/api/googleapi"
Expand All @@ -14,39 +16,44 @@ var (
multiDashes = regexp.MustCompile(`-{2,}`)
)

const (
bucketResourceName = "bucket"
)

func (o *ClusterUninstaller) listBuckets(ctx context.Context) ([]cloudResource, error) {
return o.listBucketsWithFilter(ctx, "items(name),nextPageToken", o.ClusterID+"-", nil)
return o.listBucketsWithFilter(ctx, "items(name),nextPageToken",
func(itemName string) bool {
prefix := multiDashes.ReplaceAllString(o.ClusterID+"-", "-")
return strings.HasPrefix(itemName, prefix)
})
}

// listBucketsWithFilter lists buckets in the project that satisfy the filter criteria.
// The fields parameter specifies which fields should be returned in the result, the filter string contains
// a prefix string passed to the API to filter results. The filterFunc is a client-side filtering function
// that determines whether a particular result should be returned or not.
func (o *ClusterUninstaller) listBucketsWithFilter(ctx context.Context, fields string, prefix string, filterFunc func(*storage.Bucket) bool) ([]cloudResource, error) {
func (o *ClusterUninstaller) listBucketsWithFilter(ctx context.Context, fields string, filterFunc resourceFilterFunc) ([]cloudResource, error) {
o.Logger.Debug("Listing storage buckets")
ctx, cancel := context.WithTimeout(ctx, defaultTimeout)
defer cancel()
result := []cloudResource{}
req := o.storageSvc.Buckets.List(o.ProjectID).Fields(googleapi.Field(fields))
if len(prefix) > 0 {
prefix = multiDashes.ReplaceAllString(prefix, "-")
req = req.Prefix(prefix)
}

err := req.Pages(ctx, func(list *storage.Buckets) error {
for _, item := range list.Items {
if filterFunc == nil || filterFunc != nil && filterFunc(item) {
if filterFunc(item.Name) {
o.Logger.Debugf("Found bucket: %s", item.Name)
result = append(result, cloudResource{
key: item.Name,
name: item.Name,
typeName: "bucket",
typeName: bucketResourceName,
})
}
}
return nil
})
if err != nil {
return nil, errors.Wrap(err, "failed to fetch object storage buckets")
return nil, fmt.Errorf("failed to fetch object storage buckets: %w", err)
}
return result, nil
}
Expand All @@ -71,13 +78,13 @@ func (o *ClusterUninstaller) destroyBuckets(ctx context.Context) error {
if err != nil {
return err
}
items := o.insertPendingItems("bucket", found)
items := o.insertPendingItems(bucketResourceName, found)
for _, item := range items {
foundObjects, err := o.listBucketObjects(ctx, item)
if err != nil {
return err
}
objects := o.insertPendingItems("bucketobject", foundObjects)
objects := o.insertPendingItems(bucketObjectResourceName, foundObjects)
for _, object := range objects {
err = o.deleteBucketObject(ctx, item, object)
if err != nil {
Expand All @@ -89,7 +96,7 @@ func (o *ClusterUninstaller) destroyBuckets(ctx context.Context) error {
o.errorTracker.suppressWarning(item.key, err, o.Logger)
}
}
if items = o.getPendingItems("bucket"); len(items) > 0 {
if items = o.getPendingItems(bucketResourceName); len(items) > 0 {
return errors.Errorf("%d items pending", len(items))
}
return nil
Expand Down
6 changes: 5 additions & 1 deletion pkg/destroy/gcp/bucketobject.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import (
storage "google.golang.org/api/storage/v1"
)

const (
bucketObjectResourceName = "bucketobject"
)

func (o *ClusterUninstaller) listBucketObjects(ctx context.Context, bucket cloudResource) ([]cloudResource, error) {
o.Logger.Debugf("Listing objects for storage bucket %s", bucket.name)
ctx, cancel := context.WithTimeout(ctx, defaultTimeout)
Expand All @@ -19,7 +23,7 @@ func (o *ClusterUninstaller) listBucketObjects(ctx context.Context, bucket cloud
result = append(result, cloudResource{
key: object.Name,
name: object.Name,
typeName: "bucketobject",
typeName: bucketObjectResourceName,
})
}
return nil
Expand Down
Loading

0 comments on commit 179cd9a

Please sign in to comment.