From e2a43418524568d3b96b97d93b4df9a59e346547 Mon Sep 17 00:00:00 2001 From: Brent Barbachem Date: Wed, 7 Aug 2024 15:09:15 -0400 Subject: [PATCH 1/3] OCPBUGS-37683: Resource deletion is holding up deletion process ** The Address, backend service, and tcp proxies were not found and deleted properly. This was holding up the deletion of certain other resources such as health checks. --- pkg/destroy/gcp/address.go | 154 +++++++++++++------------- pkg/destroy/gcp/backendservice.go | 163 +++++++++++++++------------- pkg/destroy/gcp/cloudcontroller.go | 23 ++-- pkg/destroy/gcp/targetTCPProxies.go | 110 ++++++++++++------- 4 files changed, 247 insertions(+), 203 deletions(-) diff --git a/pkg/destroy/gcp/address.go b/pkg/destroy/gcp/address.go index 94c73ff605c..febe3d78c86 100644 --- a/pkg/destroy/gcp/address.go +++ b/pkg/destroy/gcp/address.go @@ -4,26 +4,54 @@ import ( "context" "fmt" - "github.com/sirupsen/logrus" "google.golang.org/api/compute/v1" "google.golang.org/api/googleapi" "github.com/openshift/installer/pkg/types/gcp" ) -func (o *ClusterUninstaller) listAddresses(ctx context.Context, scope resourceScope) ([]cloudResource, error) { - return o.listAddressesWithFilter(ctx, "items(name,region,addressType),nextPageToken", o.clusterIDFilter(), nil, scope) +const ( + globalAddressResource = "address" + regionalAddressResource = "regionaddress" +) + +func (o *ClusterUninstaller) listAddresses(ctx context.Context, typeName string) ([]cloudResource, error) { + return o.listAddressesWithFilter(ctx, typeName, "items(name,region,addressType),nextPageToken", o.clusterIDFilter()) } -func createAddressCloudResources(filterFunc func(address *compute.Address) bool, list *compute.AddressList) []cloudResource { - result := []cloudResource{} +// 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) { + o.Logger.Debugf("Listing addresses") + + 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() + case regionalAddressResource: + list, err = o.computeSvc.Addresses.List(o.ProjectID, o.Region).Filter(filter).Fields(googleapi.Field(fields)).Context(ctx).Do() + default: + return nil, fmt.Errorf("invalid address type %q", typeName) + } + + if err != nil { + return nil, fmt.Errorf("failed to list addresses: %w", err) + } + result := []cloudResource{} for _, item := range list.Items { - if filterFunc == nil || filterFunc(item) { - logrus.Debugf("Found address: %s", item.Name) - var quota []gcp.QuotaUsage - if item.AddressType == "INTERNAL" { - quota = []gcp.QuotaUsage{{ + o.Logger.Debugf("Found address: %s", item.Name) + if item.AddressType == "INTERNAL" { + result = append(result, cloudResource{ + key: item.Name, + name: item.Name, + typeName: typeName, + quota: []gcp.QuotaUsage{{ Metric: &gcp.Metric{ Service: gcp.ServiceComputeEngineAPI, Limit: "internal_addresses", @@ -32,77 +60,36 @@ func createAddressCloudResources(filterFunc func(address *compute.Address) bool, }, }, Amount: 1, - }} - } - result = append(result, cloudResource{ - key: item.Name, - name: item.Name, - typeName: "address", - quota: quota, + }}, }) } } - - return result -} - -// 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. The filterFunc is a client-side filtering function -// that determines whether a particular result should be returned or not. -func (o *ClusterUninstaller) listAddressesWithFilter(ctx context.Context, fields string, filter string, filterFunc func(*compute.Address) bool, scope resourceScope) ([]cloudResource, error) { - o.Logger.Debugf("Listing %s addresses", scope) - ctx, cancel := context.WithTimeout(ctx, defaultTimeout) - defer cancel() - result := []cloudResource{} - - if scope == gcpGlobalResource { - req := o.computeSvc.GlobalAddresses.List(o.ProjectID).Fields(googleapi.Field(fields)) - if len(filter) > 0 { - req = req.Filter(filter) - } - err := req.Pages(ctx, func(list *compute.AddressList) error { - result = append(result, createAddressCloudResources(filterFunc, list)...) - return nil - }) - if err != nil { - return nil, fmt.Errorf("failed to list global addresses: %w", err) - } - return result, nil - } - - // Regional addresses - req := o.computeSvc.Addresses.List(o.ProjectID, o.Region).Fields(googleapi.Field(fields)) - if len(filter) > 0 { - req = req.Filter(filter) - } - err := req.Pages(ctx, func(list *compute.AddressList) error { - result = append(result, createAddressCloudResources(filterFunc, list)...) - return nil - }) - if err != nil { - return nil, fmt.Errorf("failed to list regional addresses: %w", err) - } - return result, nil } -func (o *ClusterUninstaller) deleteAddress(ctx context.Context, item cloudResource, scope resourceScope) error { +func (o *ClusterUninstaller) deleteAddress(ctx context.Context, item cloudResource) error { o.Logger.Debugf("Deleting address %s", item.name) ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() - var op *compute.Operation var err error - if scope == gcpGlobalResource { + var op *compute.Operation + switch item.typeName { + case globalAddressResource: op, err = o.computeSvc.GlobalAddresses.Delete(o.ProjectID, item.name).RequestId(o.requestID(item.typeName, item.name)).Context(ctx).Do() - } else { + case regionalAddressResource: op, err = o.computeSvc.Addresses.Delete(o.ProjectID, o.Region, item.name).RequestId(o.requestID(item.typeName, item.name)).Context(ctx).Do() + default: + return fmt.Errorf("invalid address type %q", item.typeName) } + if err != nil && !isNoOp(err) { + o.resetRequestID(item.typeName, item.name) + return fmt.Errorf("failed to delete address %s: %w", item.name, err) + } if op != nil && op.Status == "DONE" && isErrorStatus(op.HttpErrorStatusCode) { o.resetRequestID(item.typeName, item.name) - return fmt.Errorf("failed to delete address %s with error: %s: %w", item.name, operationErrorMessage(op), err) + return fmt.Errorf("failed to delete address %s with error: %s", item.name, operationErrorMessage(op)) } if (err != nil && isNoOp(err)) || (op != nil && op.Status == "DONE") { o.resetRequestID(item.typeName, item.name) @@ -115,23 +102,32 @@ 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 { - for _, scope := range []resourceScope{gcpGlobalResource, gcpRegionalResource} { - found, err := o.listAddresses(ctx, scope) + 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) if err != nil { - return fmt.Errorf("failed to list %s addresses: %w", scope, err) - } - items := o.insertPendingItems("address", found) - for _, item := range items { - err := o.deleteAddress(ctx, item, scope) - if err != nil { - o.errorTracker.suppressWarning(item.key, err, o.Logger) - } - } - for _, item := range o.getPendingItems("address") { - if err := o.deleteAddress(ctx, item, scope); err != nil { - return fmt.Errorf("error deleting pending address %s: %w", item.name, err) - } + o.errorTracker.suppressWarning(item.key, err, o.Logger) } } + + if items = o.getPendingItems(globalAddressResource); len(items) > 0 { + return fmt.Errorf("%d global addresses pending", len(items)) + } + + if items = o.getPendingItems(regionalAddressResource); len(items) > 0 { + return fmt.Errorf("%d region addresses pending", len(items)) + } + return nil } diff --git a/pkg/destroy/gcp/backendservice.go b/pkg/destroy/gcp/backendservice.go index c3882c247ed..07945f24bf8 100644 --- a/pkg/destroy/gcp/backendservice.go +++ b/pkg/destroy/gcp/backendservice.go @@ -4,98 +4,108 @@ import ( "context" "fmt" - "github.com/sirupsen/logrus" "google.golang.org/api/compute/v1" "google.golang.org/api/googleapi" + "k8s.io/apimachinery/pkg/util/sets" "github.com/openshift/installer/pkg/types/gcp" ) -func (o *ClusterUninstaller) listBackendServices(ctx context.Context, scope resourceScope) ([]cloudResource, error) { - return o.listBackendServicesWithFilter(ctx, "items(name),nextPageToken", o.clusterIDFilter(), nil, scope) +const ( + globalBackendServiceResource = "backendservice" + regionBackendServiceResource = "regionbackendservice" +) + +func (o *ClusterUninstaller) listBackendServices(ctx context.Context, typeName string) ([]cloudResource, error) { + return o.listBackendServicesWithFilter(ctx, typeName, "items(name),nextPageToken", o.clusterIDFilter(), nil) } -func createBackendServiceCloudResources(filterFunc func(*compute.BackendService) bool, list *compute.BackendServiceList) []cloudResource { - result := []cloudResource{} +func backendServiceBelongsToInstanceGroup(item *compute.BackendService, igURLs sets.Set[string]) bool { + if igURLs == nil { + return true + } - for _, item := range list.Items { - if filterFunc == nil || filterFunc(item) { - logrus.Debugf("Found backend service: %s", item.Name) - result = append(result, cloudResource{ - key: item.Name, - name: item.Name, - typeName: "backendservice", - quota: []gcp.QuotaUsage{{ - Metric: &gcp.Metric{ - Service: gcp.ServiceComputeEngineAPI, - Limit: "backend_services", - }, - Amount: 1, - }}, - }) + if len(item.Backends) == 0 { + return false + } + for _, backend := range item.Backends { + if !igURLs.Has(backend.Group) { + return false } } - - return result + return true } // 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. The filterFunc is a client-side filtering function -// that determines whether a particular result should be returned or not. -func (o *ClusterUninstaller) listBackendServicesWithFilter(ctx context.Context, fields string, filter string, filterFunc func(*compute.BackendService) bool, scope resourceScope) ([]cloudResource, error) { - o.Logger.Debugf("Listing %s backend services", scope) +// 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) { + o.Logger.Debugf("Listing backend services") + ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() - result := []cloudResource{} - if scope == gcpGlobalResource { - req := o.computeSvc.BackendServices.List(o.ProjectID).Fields(googleapi.Field(fields)) - if len(filter) > 0 { - req = req.Filter(filter) - } - err := req.Pages(ctx, func(list *compute.BackendServiceList) error { - result = append(result, createBackendServiceCloudResources(filterFunc, list)...) - return nil - }) - if err != nil { - return nil, fmt.Errorf("failed to list global backend services: %w", err) - } - return result, 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() + case regionBackendServiceResource: + list, err = o.computeSvc.RegionBackendServices.List(o.ProjectID, o.Region).Filter(filter).Fields(googleapi.Field(fields)).Context(ctx).Do() + default: + return nil, fmt.Errorf("invalid backend service type %q", typeName) } - // Regional backend services - req := o.computeSvc.RegionBackendServices.List(o.ProjectID, o.Region).Fields(googleapi.Field(fields)) - if len(filter) > 0 { - req = req.Filter(filter) - } - err := req.Pages(ctx, func(list *compute.BackendServiceList) error { - result = append(result, createBackendServiceCloudResources(filterFunc, list)...) - return nil - }) if err != nil { - return nil, fmt.Errorf("failed to list regional backend services: %w", err) + 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 } -func (o *ClusterUninstaller) deleteBackendService(ctx context.Context, item cloudResource, scope resourceScope) error { +func (o *ClusterUninstaller) deleteBackendService(ctx context.Context, item cloudResource) error { o.Logger.Debugf("Deleting backend service %s", item.name) ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() - var op *compute.Operation var err error - if scope == gcpGlobalResource { + var op *compute.Operation + switch item.typeName { + case globalBackendServiceResource: op, err = o.computeSvc.BackendServices.Delete(o.ProjectID, item.name).RequestId(o.requestID(item.typeName, item.name)).Context(ctx).Do() - } else { + case regionBackendServiceResource: op, err = o.computeSvc.RegionBackendServices.Delete(o.ProjectID, o.Region, item.name).RequestId(o.requestID(item.typeName, item.name)).Context(ctx).Do() + default: + return fmt.Errorf("invalid backend service type %q", item.typeName) } + if err != nil && !isNoOp(err) { + o.resetRequestID(item.typeName, item.name) + return fmt.Errorf("failed to delete backend service %s: %w", item.name, err) + } if op != nil && op.Status == "DONE" && isErrorStatus(op.HttpErrorStatusCode) { o.resetRequestID(item.typeName, item.name) - return fmt.Errorf("failed to delete backend service %s with error: %s: %w", item.name, operationErrorMessage(op), err) + return fmt.Errorf("failed to delete backend service %s with error: %s", item.name, operationErrorMessage(op)) } if (err != nil && isNoOp(err)) || (op != nil && op.Status == "DONE") { o.resetRequestID(item.typeName, item.name) @@ -105,28 +115,35 @@ func (o *ClusterUninstaller) deleteBackendService(ctx context.Context, item clou return nil } -// destroyBackendServices removes backend services with a name prefixed +// destroyBackendServices removes all backend services resources that have a name prefixed // with the cluster's infra ID. func (o *ClusterUninstaller) destroyBackendServices(ctx context.Context) error { - for _, scope := range []resourceScope{gcpGlobalResource, gcpRegionalResource} { - found, err := o.listBackendServices(ctx, scope) + found, err := o.listBackendServices(ctx, globalBackendServiceResource) + if err != nil { + return err + } + items := o.insertPendingItems(globalBackendServiceResource, found) + + found, err = o.listBackendServices(ctx, regionBackendServiceResource) + if err != nil { + return err + } + items = append(items, o.insertPendingItems(regionBackendServiceResource, found)...) + + for _, item := range items { + err := o.deleteBackendService(ctx, item) if err != nil { - return fmt.Errorf("failed to list backend services: %w", err) - } - items := o.insertPendingItems("backendservice", found) - for _, item := range items { - err := o.deleteBackendService(ctx, item, scope) - if err != nil { - o.errorTracker.suppressWarning(item.key, err, o.Logger) - } - } - if items = o.getPendingItems("backendservice"); len(items) > 0 { - for _, item := range items { - if err := o.deleteBackendService(ctx, item, scope); err != nil { - return fmt.Errorf("error deleting pending backend service %s: %w", item.name, err) - } - } + o.errorTracker.suppressWarning(item.key, err, o.Logger) } } + + if items = o.getPendingItems(globalBackendServiceResource); len(items) > 0 { + return fmt.Errorf("%d global backend service pending", len(items)) + } + + if items = o.getPendingItems(regionBackendServiceResource); len(items) > 0 { + return fmt.Errorf("%d region backend service pending", len(items)) + } + return nil } diff --git a/pkg/destroy/gcp/cloudcontroller.go b/pkg/destroy/gcp/cloudcontroller.go index 28ca497e3c6..c1f0496723d 100644 --- a/pkg/destroy/gcp/cloudcontroller.go +++ b/pkg/destroy/gcp/cloudcontroller.go @@ -21,22 +21,12 @@ func (o *ClusterUninstaller) listCloudControllerInstanceGroups(ctx context.Conte // It list all backend services matching the cloud controller name convention that contain // only cluster instance groups. func (o *ClusterUninstaller) listCloudControllerBackendServices(ctx context.Context, instanceGroups []cloudResource) ([]cloudResource, error) { - urls := sets.NewString() + urls := sets.Set[string]{} for _, instanceGroup := range instanceGroups { urls.Insert(instanceGroup.url) } filter := "name eq \"a[0-9a-f]{30,50}\"" - return o.listBackendServicesWithFilter(ctx, "items(name,backends),nextPageToken", filter, func(item *compute.BackendService) bool { - if len(item.Backends) == 0 { - return false - } - for _, backend := range item.Backends { - if !urls.Has(backend.Group) { - return false - } - } - return true - }, gcpRegionalResource) + return o.listBackendServicesWithFilter(ctx, regionBackendServiceResource, "items(name,backends),nextPageToken", filter, urls) } // listCloudControllerTargetPools returns target pools created by the cloud controller or owned by the cloud controller. @@ -76,7 +66,7 @@ func (o *ClusterUninstaller) discoverCloudControllerLoadBalancerResources(ctx co loadBalancerNameFilter := fmt.Sprintf("name eq \"%s\"", loadBalancerName) // Discover associated addresses: loadBalancerName - found, err := o.listAddressesWithFilter(ctx, "items(name),nextPageToken", loadBalancerNameFilter, nil, gcpRegionalResource) + found, err := o.listAddressesWithFilter(ctx, "regionaddress", "items(name),nextPageToken", loadBalancerNameFilter) if err != nil { return err } @@ -120,6 +110,13 @@ func (o *ClusterUninstaller) discoverCloudControllerLoadBalancerResources(ctx co } o.insertPendingItems("forwardingrule", found) + // Discover associated target tcp proxies: loadBalancerName + found, err = o.listTargetTCPProxiesWithFilter(ctx, globalTargetTCPProxyResource, "items(name),nextPageToken", loadBalancerNameFilter) + if err != nil { + return err + } + o.insertPendingItems(globalTargetTCPProxyResource, found) + // Discover associated health checks: loadBalancerName found, err = o.listHealthChecksWithFilter(ctx, "healthcheck", "items(name),nextPageToken", loadBalancerNameFilter, o.healthCheckList) if err != nil { diff --git a/pkg/destroy/gcp/targetTCPProxies.go b/pkg/destroy/gcp/targetTCPProxies.go index b51b61cc37e..820f74c12e2 100644 --- a/pkg/destroy/gcp/targetTCPProxies.go +++ b/pkg/destroy/gcp/targetTCPProxies.go @@ -10,83 +10,117 @@ import ( "github.com/openshift/installer/pkg/types/gcp" ) -func (o *ClusterUninstaller) listTargetTCPProxies(ctx context.Context) ([]cloudResource, error) { - return o.listTargetTCPProxiesWithFilter(ctx, "items(name),nextPageToken", o.clusterIDFilter(), nil) +const ( + globalTargetTCPProxyResource = "targettcpproxy" + regionalTargetTCPProxyResource = "regiontargettcpproxy" +) + +func (o *ClusterUninstaller) listTargetTCPProxies(ctx context.Context, typeName string) ([]cloudResource, error) { + return o.listTargetTCPProxiesWithFilter(ctx, typeName, "items(name),nextPageToken", o.clusterIDFilter()) } // listTargetTCPProxiesWithFilter lists target TCP Proxies in the project that satisfy the filter criteria. -func (o *ClusterUninstaller) listTargetTCPProxiesWithFilter(ctx context.Context, fields string, filter string, filterFunc func(list *compute.TargetTcpProxy) bool) ([]cloudResource, error) { +func (o *ClusterUninstaller) listTargetTCPProxiesWithFilter(ctx context.Context, typeName, fields, filter string) ([]cloudResource, error) { o.Logger.Debugf("Listing target tcp proxies") ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() - result := []cloudResource{} - req := o.computeSvc.TargetTcpProxies.List(o.ProjectID).Fields(googleapi.Field(fields)) - if len(filter) > 0 { - req = req.Filter(filter) + + var err error + var list *compute.TargetTcpProxyList + switch typeName { + case globalTargetTCPProxyResource: + list, err = o.computeSvc.TargetTcpProxies.List(o.ProjectID).Filter(filter).Fields(googleapi.Field(fields)).Context(ctx).Do() + case regionalTargetTCPProxyResource: + list, err = o.computeSvc.RegionTargetTcpProxies.List(o.ProjectID, o.Region).Filter(filter).Fields(googleapi.Field(fields)).Context(ctx).Do() + default: + return nil, fmt.Errorf("invalid target tcp proxy type %q", typeName) } - err := req.Pages(ctx, func(list *compute.TargetTcpProxyList) error { - for _, item := range list.Items { - if filterFunc == nil || (filterFunc != nil && filterFunc(item)) { - o.Logger.Debugf("Found target TCP proxy: %s", item.Name) - result = append(result, cloudResource{ - key: item.Name, - name: item.Name, - typeName: "targettcpproxy", - quota: []gcp.QuotaUsage{{ - Metric: &gcp.Metric{ - Service: gcp.ServiceComputeEngineAPI, - Limit: "target_tcp_proxy", - }, - Amount: 1, - }}, - }) - } - } - return nil - }) + if err != nil { return nil, fmt.Errorf("failed to list target tcp proxies: %w", err) } + + result := []cloudResource{} + for _, item := range list.Items { + o.Logger.Debugf("Found target TCP proxy: %s", item.Name) + result = append(result, cloudResource{ + key: item.Name, + name: item.Name, + typeName: typeName, + quota: []gcp.QuotaUsage{{ + Metric: &gcp.Metric{ + Service: gcp.ServiceComputeEngineAPI, + Limit: "target_tcp_proxy", + }, + Amount: 1, + }}, + }) + } + return result, nil } func (o *ClusterUninstaller) deleteTargetTCPProxy(ctx context.Context, item cloudResource) error { - o.Logger.Debugf("Deleting target TCP Proxies %s", item.name) + o.Logger.Debugf("Deleting target tcp proxy %s", item.name) ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() - op, err := o.computeSvc.TargetTcpProxies.Delete(o.ProjectID, item.name).RequestId(o.requestID(item.typeName, item.name)).Context(ctx).Do() + + var err error + var op *compute.Operation + switch item.typeName { + case globalTargetTCPProxyResource: + op, err = o.computeSvc.TargetTcpProxies.Delete(o.ProjectID, item.name).RequestId(o.requestID(item.typeName, item.name)).Context(ctx).Do() + case regionalTargetTCPProxyResource: + op, err = o.computeSvc.RegionTargetTcpProxies.Delete(o.ProjectID, o.Region, item.name).RequestId(o.requestID(item.typeName, item.name)).Context(ctx).Do() + default: + return fmt.Errorf("invalid target tcp proxy type %q", item.typeName) + } + if err != nil && !isNoOp(err) { o.resetRequestID(item.typeName, item.name) - return fmt.Errorf("failed to delete target TCP proxy %s: %w", item.name, err) + return fmt.Errorf("failed to target tcp proxy %s: %w", item.name, err) } if op != nil && op.Status == "DONE" && isErrorStatus(op.HttpErrorStatusCode) { o.resetRequestID(item.typeName, item.name) - return fmt.Errorf("failed to delete target TCP proxy %s with error: %s: %w", item.name, operationErrorMessage(op), err) + return fmt.Errorf("failed to delete target tcp proxy %s with error: %s", item.name, operationErrorMessage(op)) } if (err != nil && isNoOp(err)) || (op != nil && op.Status == "DONE") { o.resetRequestID(item.typeName, item.name) o.deletePendingItems(item.typeName, []cloudResource{item}) - o.Logger.Infof("Deleted target TCP proxy %s", item.name) + o.Logger.Infof("Deleted target tcp proxy %s", item.name) } return nil } -// destroyTargetTCPProxies removes target tcp proxies with a name prefixed +// destroyTargetTCPProxies removes all target tcp proxy resources that have a name prefixed // with the cluster's infra ID. func (o *ClusterUninstaller) destroyTargetTCPProxies(ctx context.Context) error { - found, err := o.listTargetTCPProxies(ctx) + found, err := o.listTargetTCPProxies(ctx, globalTargetTCPProxyResource) + if err != nil { + return err + } + items := o.insertPendingItems(globalTargetTCPProxyResource, found) + + found, err = o.listTargetTCPProxies(ctx, regionalTargetTCPProxyResource) if err != nil { - return fmt.Errorf("failed to list target TCP proxies: %w", err) + return err } - items := o.insertPendingItems("targettcpproxy", found) + items = append(items, o.insertPendingItems(regionalTargetTCPProxyResource, found)...) + for _, item := range items { err := o.deleteTargetTCPProxy(ctx, item) if err != nil { o.errorTracker.suppressWarning(item.key, err, o.Logger) } } - if items = o.getPendingItems("targettcpproxy"); len(items) > 0 { - return fmt.Errorf("%d items pending", len(items)) + + if items = o.getPendingItems(globalTargetTCPProxyResource); len(items) > 0 { + return fmt.Errorf("%d global target tcp proxy pending", len(items)) } + + if items = o.getPendingItems(regionalTargetTCPProxyResource); len(items) > 0 { + return fmt.Errorf("%d region target tcp proxy pending", len(items)) + } + return nil } From efccb5f0be3c8d08a20ced218f33d61ca6d0d767 Mon Sep 17 00:00:00 2001 From: barbacbd Date: Mon, 22 Jun 2026 14:49:18 -0400 Subject: [PATCH 2/3] OCPBUGS-46605: Find instances by label GCP This fixes a regression where finding instances by label was not working correctly. The labels field must be requested when querying for instance information. --- pkg/destroy/gcp/instance.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/destroy/gcp/instance.go b/pkg/destroy/gcp/instance.go index 5159fd076aa..712a08538da 100644 --- a/pkg/destroy/gcp/instance.go +++ b/pkg/destroy/gcp/instance.go @@ -27,12 +27,12 @@ func (o *ClusterUninstaller) getInstanceNameAndZone(instanceURL string) (string, } func (o *ClusterUninstaller) listInstances(ctx context.Context) ([]cloudResource, error) { - byName, err := o.listInstancesWithFilter(ctx, "items/*/instances(name,zone,status,machineType),nextPageToken", o.clusterIDFilter(), nil) + byName, err := o.listInstancesWithFilter(ctx, "items/*/instances(name,zone,status,machineType,labels),nextPageToken", o.clusterIDFilter(), nil) if err != nil { return nil, err } - byLabel, err := o.listInstancesWithFilter(ctx, "items/*/instances(name,zone,status,machineType),nextPageToken", o.clusterLabelFilter(), nil) + byLabel, err := o.listInstancesWithFilter(ctx, "items/*/instances(name,zone,status,machineType,labels),nextPageToken", o.clusterLabelFilter(), nil) if err != nil { return nil, err } From 476bb3528133dbbdb95241c20c3d9f4052237bd2 Mon Sep 17 00:00:00 2001 From: barbacbd Date: Mon, 22 Jun 2026 14:49:20 -0400 Subject: [PATCH 3/3] OCPBUGS-90711: Destroy all private cluster backend service resources When a private cluster was created, the destroy process would not find and destroy backend services. This was happening specifically when no backends were created/found. Enhanced backend service discovery to check firewall rules for cluster ID tags when backends are empty. This allows proper identification of orphaned backend services that should be deleted during cluster destroy. Also added global health check discovery to ensure all related resources are properly cleaned up. --- pkg/destroy/gcp/cloudcontroller.go | 75 +++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/pkg/destroy/gcp/cloudcontroller.go b/pkg/destroy/gcp/cloudcontroller.go index c1f0496723d..d8ce9587c48 100644 --- a/pkg/destroy/gcp/cloudcontroller.go +++ b/pkg/destroy/gcp/cloudcontroller.go @@ -3,9 +3,13 @@ package gcp import ( "context" "fmt" + "strings" compute "google.golang.org/api/compute/v1" + "google.golang.org/api/googleapi" "k8s.io/apimachinery/pkg/util/sets" + + "github.com/openshift/installer/pkg/types/gcp" ) // listCloudControllerInstanceGroups returns instance groups created by the cloud controller. @@ -21,12 +25,81 @@ func (o *ClusterUninstaller) listCloudControllerInstanceGroups(ctx context.Conte // It list all backend services matching the cloud controller name convention that contain // only cluster instance groups. func (o *ClusterUninstaller) listCloudControllerBackendServices(ctx context.Context, instanceGroups []cloudResource) ([]cloudResource, error) { + o.Logger.Debugf("Listing cloud controller backend services") urls := sets.Set[string]{} for _, instanceGroup := range instanceGroups { urls.Insert(instanceGroup.url) } + filter := "name eq \"a[0-9a-f]{30,50}\"" - return o.listBackendServicesWithFilter(ctx, regionBackendServiceResource, "items(name,backends),nextPageToken", filter, urls) + result := []cloudResource{} + + // Fetch firewall rules once (fully paginated); used to validate backend + // services that have no backends. + var firewalls []*compute.Firewall + if err := o.computeSvc.Firewalls.List(o.ProjectID). + Fields(googleapi.Field("items(name,targetTags),nextPageToken")). + Pages(ctx, func(list *compute.FirewallList) error { + firewalls = append(firewalls, list.Items...) + return nil + }); err != nil { + return nil, err + } + + req := o.computeSvc.RegionBackendServices.List(o.ProjectID, o.Region).Fields(googleapi.Field("items(name,backends),nextPageToken")).Filter(filter) + err := req.Pages(ctx, func(list *compute.BackendServiceList) error { + for _, item := range list.Items { + if len(item.Backends) == 0 { + o.Logger.Debugf("Backend service %s has no backends, checking firewall rules for cluster ID", item.Name) + found := false + for _, fw := range firewalls { + if strings.Contains(fw.Name, item.Name) { + for _, tag := range fw.TargetTags { + if strings.Contains(tag, o.ClusterID) { + found = true + break + } + } + } + if found { + break + } + } + if !found { + continue + } + } else { + allBackendsMatch := true + for _, backend := range item.Backends { + if !urls.Has(backend.Group) { + allBackendsMatch = false + break + } + } + if !allBackendsMatch { + continue + } + } + o.Logger.Debugf("Found backend service: %s", item.Name) + result = append(result, cloudResource{ + key: item.Name, + name: item.Name, + typeName: regionBackendServiceResource, + quota: []gcp.QuotaUsage{{ + Metric: &gcp.Metric{ + Service: gcp.ServiceComputeEngineAPI, + Limit: "backend_services", + }, + Amount: 1, + }}, + }) + } + return nil + }) + if err != nil { + return nil, err + } + return result, nil } // listCloudControllerTargetPools returns target pools created by the cloud controller or owned by the cloud controller.