Merge pull request #4216 from zac-nixon/main

[bug fix] Make SG deletion more responsive

Author: k8s-ci-robot

controllers/gateway/gateway_controller.go

@@ -48,21 +48,21 @@ const (
 var _ Reconciler = &gatewayReconciler{}
 
 // NewNLBGatewayReconciler constructs a gateway reconciler to handle specifically for NLB gateways
-func NewNLBGatewayReconciler(routeLoader routeutils.Loader, cloud services.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, finalizerManager k8s.FinalizerManager, networkingSGReconciler networking.SecurityGroupReconciler, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager, subnetResolver networking.SubnetsResolver, vpcInfoProvider networking.VPCInfoProvider, backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, reconcileCounters *metricsutil.ReconcileCounters, routeReconciler routeutils.RouteReconciler) Reconciler {
-return newGatewayReconciler(constants.NLBGatewayController, elbv2model.LoadBalancerTypeNetwork, controllerConfig.NLBGatewayMaxConcurrentReconciles, constants.NLBGatewayTagPrefix, shared_constants.NLBGatewayFinalizer, routeLoader, routeutils.L4RouteFilter, cloud, k8sClient, eventRecorder, controllerConfig, finalizerManager, networkingSGReconciler, networkingSGManager, elbv2TaggingManager, subnetResolver, vpcInfoProvider, backendSGProvider, sgResolver, logger, metricsCollector, reconcileCounters.IncrementNLBGateway, routeReconciler)
+func NewNLBGatewayReconciler(routeLoader routeutils.Loader, cloud services.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, finalizerManager k8s.FinalizerManager, networkingManager networking.NetworkingManager, networkingSGReconciler networking.SecurityGroupReconciler, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager, subnetResolver networking.SubnetsResolver, vpcInfoProvider networking.VPCInfoProvider, backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, reconcileCounters *metricsutil.ReconcileCounters, routeReconciler routeutils.RouteReconciler) Reconciler {
+return newGatewayReconciler(constants.NLBGatewayController, elbv2model.LoadBalancerTypeNetwork, controllerConfig.NLBGatewayMaxConcurrentReconciles, constants.NLBGatewayTagPrefix, shared_constants.NLBGatewayFinalizer, routeLoader, routeutils.L4RouteFilter, cloud, k8sClient, eventRecorder, controllerConfig, finalizerManager, networkingSGReconciler, networkingManager, networkingSGManager, elbv2TaggingManager, subnetResolver, vpcInfoProvider, backendSGProvider, sgResolver, logger, metricsCollector, reconcileCounters.IncrementNLBGateway, routeReconciler)
 }
 
 // NewALBGatewayReconciler constructs a gateway reconciler to handle specifically for ALB gateways
-func NewALBGatewayReconciler(routeLoader routeutils.Loader, cloud services.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, finalizerManager k8s.FinalizerManager, networkingSGReconciler networking.SecurityGroupReconciler, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager, subnetResolver networking.SubnetsResolver, vpcInfoProvider networking.VPCInfoProvider, backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, reconcileCounters *metricsutil.ReconcileCounters, routeReconciler routeutils.RouteReconciler) Reconciler {
-return newGatewayReconciler(constants.ALBGatewayController, elbv2model.LoadBalancerTypeApplication, controllerConfig.ALBGatewayMaxConcurrentReconciles, constants.ALBGatewayTagPrefix, shared_constants.ALBGatewayFinalizer, routeLoader, routeutils.L7RouteFilter, cloud, k8sClient, eventRecorder, controllerConfig, finalizerManager, networkingSGReconciler, networkingSGManager, elbv2TaggingManager, subnetResolver, vpcInfoProvider, backendSGProvider, sgResolver, logger, metricsCollector, reconcileCounters.IncrementALBGateway, routeReconciler)
+func NewALBGatewayReconciler(routeLoader routeutils.Loader, cloud services.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, finalizerManager k8s.FinalizerManager, networkingManager networking.NetworkingManager, networkingSGReconciler networking.SecurityGroupReconciler, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager, subnetResolver networking.SubnetsResolver, vpcInfoProvider networking.VPCInfoProvider, backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, reconcileCounters *metricsutil.ReconcileCounters, routeReconciler routeutils.RouteReconciler) Reconciler {
+return newGatewayReconciler(constants.ALBGatewayController, elbv2model.LoadBalancerTypeApplication, controllerConfig.ALBGatewayMaxConcurrentReconciles, constants.ALBGatewayTagPrefix, shared_constants.ALBGatewayFinalizer, routeLoader, routeutils.L7RouteFilter, cloud, k8sClient, eventRecorder, controllerConfig, finalizerManager, networkingSGReconciler, networkingManager, networkingSGManager, elbv2TaggingManager, subnetResolver, vpcInfoProvider, backendSGProvider, sgResolver, logger, metricsCollector, reconcileCounters.IncrementALBGateway, routeReconciler)
 }
 
 // newGatewayReconciler constructs a reconciler that responds to gateway object changes
 func newGatewayReconciler(controllerName string, lbType elbv2model.LoadBalancerType, maxConcurrentReconciles int,
 gatewayTagPrefix string, finalizer string, routeLoader routeutils.Loader, routeFilter routeutils.LoadRouteFilter,
 cloud services.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig,
 finalizerManager k8s.FinalizerManager, networkingSGReconciler networking.SecurityGroupReconciler,
-networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager,
+networkingManager networking.NetworkingManager, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager,
 subnetResolver networking.SubnetsResolver, vpcInfoProvider networking.VPCInfoProvider, backendSGProvider networking.BackendSGProvider,
 sgResolver networking.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector,
 reconcileTracker func(namespaceName types.NamespacedName), routeReconciler routeutils.RouteReconciler) Reconciler {
@@ -71,7 +71,7 @@ func newGatewayReconciler(controllerName string, lbType elbv2model.LoadBalancerT
 modelBuilder := gatewaymodel.NewModelBuilder(subnetResolver, vpcInfoProvider, cloud.VpcID(), lbType, trackingProvider, elbv2TaggingManager, controllerConfig, cloud.EC2(), cloud.ACM(), controllerConfig.FeatureGates, controllerConfig.ClusterName, controllerConfig.DefaultTags, sets.New(controllerConfig.ExternalManagedTags...), controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.DefaultLoadBalancerScheme, backendSGProvider, sgResolver, controllerConfig.EnableBackendSecurityGroup, controllerConfig.DisableRestrictedSGRules, controllerConfig.IngressConfig.AllowedCertificateAuthorityARNs, logger)
 
 stackMarshaller := deploy.NewDefaultStackMarshaller()
-stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler, elbv2TaggingManager, controllerConfig, gatewayTagPrefix, logger, metricsCollector, controllerName)
+stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingManager, networkingSGManager, networkingSGReconciler, elbv2TaggingManager, controllerConfig, gatewayTagPrefix, logger, metricsCollector, controllerName)
 
 cfgResolver := newGatewayConfigResolver()
 

controllers/ingress/group_controller.go

@@ -50,7 +50,7 @@ const (
 // NewGroupReconciler constructs new GroupReconciler
 func NewGroupReconciler(cloud services.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder,
 finalizerManager k8s.FinalizerManager, networkingSGManager networkingpkg.SecurityGroupManager,
-networkingSGReconciler networkingpkg.SecurityGroupReconciler, subnetsResolver networkingpkg.SubnetsResolver,
+networkingManager networkingpkg.NetworkingManager, networkingSGReconciler networkingpkg.SecurityGroupReconciler, subnetsResolver networkingpkg.SubnetsResolver,
 elbv2TaggingManager elbv2deploy.TaggingManager, controllerConfig config.ControllerConfig, backendSGProvider networkingpkg.BackendSGProvider,
 sgResolver networkingpkg.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, reconcileCounters *metricsutil.ReconcileCounters) *groupReconciler {
 
@@ -67,7 +67,7 @@ func NewGroupReconciler(cloud services.Cloud, k8sClient client.Client, eventReco
 controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.DefaultLoadBalancerScheme, backendSGProvider, sgResolver,
 controllerConfig.EnableBackendSecurityGroup, controllerConfig.EnableManageBackendSecurityGroupRules, controllerConfig.DisableRestrictedSGRules, controllerConfig.IngressConfig.AllowedCertificateAuthorityARNs, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), logger, metricsCollector)
 stackMarshaller := deploy.NewDefaultStackMarshaller()
-stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler, elbv2TaggingManager,
+stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingManager, networkingSGManager, networkingSGReconciler, elbv2TaggingManager,
 controllerConfig, ingressTagPrefix, logger, metricsCollector, controllerName)
 classLoader := ingress.NewDefaultClassLoader(k8sClient, true)
 classAnnotationMatcher := ingress.NewDefaultClassAnnotationMatcher(controllerConfig.IngressConfig.IngressClass)

controllers/service/service_controller.go

@@ -40,7 +40,7 @@ const (
 )
 
 func NewServiceReconciler(cloud services.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder,
-finalizerManager k8s.FinalizerManager, networkingSGManager networking.SecurityGroupManager,
+finalizerManager k8s.FinalizerManager, networkingManager networking.NetworkingManager, networkingSGManager networking.SecurityGroupManager,
 networkingSGReconciler networking.SecurityGroupReconciler, subnetsResolver networking.SubnetsResolver,
 vpcInfoProvider networking.VPCInfoProvider, elbv2TaggingManager elbv2deploy.TaggingManager, controllerConfig config.ControllerConfig,
 backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, reconcileCounters *metricsutil.ReconcileCounters) *serviceReconciler {
@@ -53,7 +53,7 @@ func NewServiceReconciler(cloud services.Cloud, k8sClient client.Client, eventRe
 controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.DefaultLoadBalancerScheme, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), serviceUtils,
 backendSGProvider, sgResolver, controllerConfig.EnableBackendSecurityGroup, controllerConfig.EnableManageBackendSecurityGroupRules, controllerConfig.DisableRestrictedSGRules, logger, metricsCollector, controllerConfig.FeatureGates.Enabled(config.EnableTCPUDPListenerType))
 stackMarshaller := deploy.NewDefaultStackMarshaller()
-stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler, elbv2TaggingManager, controllerConfig, serviceTagPrefix, logger, metricsCollector, controllerName)
+stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingManager, networkingSGManager, networkingSGReconciler, elbv2TaggingManager, controllerConfig, serviceTagPrefix, logger, metricsCollector, controllerName)
 return &serviceReconciler{
 k8sClient: k8sClient,
 eventRecorder: eventRecorder,

main.go

@@ -101,6 +101,7 @@ type gatewayControllerConfig struct {
 metricsCollector lbcmetrics.MetricCollector
 reconcileCounters *metricsutil.ReconcileCounters
 routeReconciler routeutils.RouteReconciler
+networkingManager networking.NetworkingManager
 }
 
 func main() {
@@ -165,19 +166,26 @@ func main() {
 controllerCFG.FeatureGates.Enabled(config.SubnetDiscoveryByReachability),
 ctrl.Log.WithName("subnets-resolver"))
 multiClusterManager := targetgroupbinding.NewMultiClusterManager(mgr.GetClient(), mgr.GetAPIReader(), ctrl.Log)
-tgbResManager := targetgroupbinding.NewDefaultResourceManager(mgr.GetClient(), cloud.ELBV2(), cloud.EC2(),
-podInfoRepo, sgManager, sgReconciler, vpcInfoProvider, multiClusterManager, lbcMetricsCollector,
-cloud.VpcID(), controllerCFG.ClusterName, controllerCFG.FeatureGates.Enabled(config.EndpointsFailOpen), controllerCFG.EnableEndpointSlices, controllerCFG.DisableRestrictedSGRules,
-controllerCFG.ServiceTargetENISGTags, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log)
+
+nodeInfoProvider := networking.NewDefaultNodeInfoProvider(cloud.EC2(), ctrl.Log)
+podENIResolver := networking.NewDefaultPodENIInfoResolver(mgr.GetClient(), cloud.EC2(), nodeInfoProvider, cloud.VpcID(), ctrl.Log)
+nodeENIResolver := networking.NewDefaultNodeENIInfoResolver(nodeInfoProvider, ctrl.Log)
+
+networkingManager := networking.NewDefaultNetworkingManager(mgr.GetClient(), podENIResolver, nodeENIResolver, sgManager, sgReconciler, cloud.VpcID(), controllerCFG.ClusterName, controllerCFG.ServiceTargetENISGTags, ctrl.Log, controllerCFG.DisableRestrictedSGRules)
+
+tgbResManager := targetgroupbinding.NewDefaultResourceManager(mgr.GetClient(), cloud.ELBV2(),
+podInfoRepo, networkingManager, vpcInfoProvider, multiClusterManager, lbcMetricsCollector,
+cloud.VpcID(), controllerCFG.FeatureGates.Enabled(config.EndpointsFailOpen), controllerCFG.EnableEndpointSlices,
+mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log)
 backendSGProvider := networking.NewBackendSGProvider(controllerCFG.ClusterName, controllerCFG.BackendSecurityGroup,
 cloud.VpcID(), cloud.EC2(), mgr.GetClient(), controllerCFG.DefaultTags, nlbGatewayEnabled || albGatewayEnabled, ctrl.Log.WithName("backend-sg-provider"))
 sgResolver := networking.NewDefaultSecurityGroupResolver(cloud.EC2(), cloud.VpcID())
 elbv2TaggingManager := elbv2deploy.NewDefaultTaggingManager(cloud.ELBV2(), cloud.VpcID(), controllerCFG.FeatureGates, cloud.RGT(), ctrl.Log)
 ingGroupReconciler := ingress.NewGroupReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("ingress"),
-finalizerManager, sgManager, sgReconciler, subnetResolver, elbv2TaggingManager,
+finalizerManager, sgManager, networkingManager, sgReconciler, subnetResolver, elbv2TaggingManager,
 controllerCFG, backendSGProvider, sgResolver, ctrl.Log.WithName("controllers").WithName("ingress"), lbcMetricsCollector, reconcileCounters)
 svcReconciler := service.NewServiceReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("service"),
-finalizerManager, sgManager, sgReconciler, subnetResolver, vpcInfoProvider, elbv2TaggingManager,
+finalizerManager, networkingManager, sgManager, sgReconciler, subnetResolver, vpcInfoProvider, elbv2TaggingManager,
 controllerCFG, backendSGProvider, sgResolver, ctrl.Log.WithName("controllers").WithName("service"), lbcMetricsCollector, reconcileCounters)
 
 delayingQueue := workqueue.NewDelayingQueueWithConfig(workqueue.DelayingQueueConfig{
@@ -231,6 +239,7 @@ func main() {
 metricsCollector: lbcMetricsCollector,
 reconcileCounters: reconcileCounters,
 routeReconciler: routeReconciler,
+networkingManager: networkingManager,
 }
 
 enabledControllers := sets.Set[string]{}
@@ -361,6 +370,7 @@ func setupGatewayController(ctx context.Context, mgr ctrl.Manager, cfg *gatewayC
 mgr.GetEventRecorderFor(controllerType),
 cfg.controllerCFG,
 cfg.finalizerManager,
+cfg.networkingManager,
 cfg.sgReconciler,
 cfg.sgManager,
 cfg.elbv2TaggingManager,
@@ -381,6 +391,7 @@ func setupGatewayController(ctx context.Context, mgr ctrl.Manager, cfg *gatewayC
 mgr.GetEventRecorderFor(controllerType),
 cfg.controllerCFG,
 cfg.finalizerManager,
+cfg.networkingManager,
 cfg.sgReconciler,
 cfg.sgManager,
 cfg.elbv2TaggingManager,

pkg/deploy/ec2/security_group_manager.go

@@ -13,12 +13,13 @@ import (
 ec2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/ec2"
 "sigs.k8s.io/aws-load-balancer-controller/pkg/networking"
 "sigs.k8s.io/aws-load-balancer-controller/pkg/runtime"
+"sync"
 "time"
 )
 
 const (
 defaultWaitSGDeletionPollInterval = 2 * time.Second
-defaultWaitSGDeletionTimeout = 2 * time.Minute
+defaultWaitSGDeletionTimeout = 10 * time.Second
 )
 
 // SecurityGroupManager is responsible for create/update/delete SecurityGroup resources.
@@ -31,10 +32,11 @@ type SecurityGroupManager interface {
 }
 
 // NewDefaultSecurityGroupManager constructs new defaultSecurityGroupManager.
-func NewDefaultSecurityGroupManager(ec2Client services.EC2, trackingProvider tracking.Provider, taggingManager TaggingManager,
+func NewDefaultSecurityGroupManager(ec2Client services.EC2, networkingManager networking.NetworkingManager, trackingProvider tracking.Provider, taggingManager TaggingManager,
 networkingSGReconciler networking.SecurityGroupReconciler, vpcID string, externalManagedTags []string, logger logr.Logger) *defaultSecurityGroupManager {
 return &defaultSecurityGroupManager{
 ec2Client: ec2Client,
+networkingManager: networkingManager,
 trackingProvider: trackingProvider,
 taggingManager: taggingManager,
 networkingSGReconciler: networkingSGReconciler,
@@ -50,6 +52,7 @@ func NewDefaultSecurityGroupManager(ec2Client services.EC2, trackingProvider tra
 // default implementation for SecurityGroupManager.
 type defaultSecurityGroupManager struct {
 ec2Client services.EC2
+networkingManager networking.NetworkingManager
 trackingProvider tracking.Provider
 taggingManager TaggingManager
 networkingSGReconciler networking.SecurityGroupReconciler
@@ -123,8 +126,17 @@ func (m *defaultSecurityGroupManager) Delete(ctx context.Context, sdkSG networki
 
 m.logger.Info("deleting securityGroup",
 "securityGroupID", sdkSG.SecurityGroupID)
+
+var once sync.Once
+cleanUpFunction := func() {
+m.networkingManager.AttemptGarbageCollection(ctx)
+}
+
 if err := runtime.RetryImmediateOnError(m.waitSGDeletionPollInterval, m.waitSGDeletionTimeout, isSecurityGroupDependencyViolationError, func() error {
 _, err := m.ec2Client.DeleteSecurityGroupWithContext(ctx, req)
+if err != nil && isSecurityGroupDependencyViolationError(err) {
+once.Do(cleanUpFunction)
+}
 return err
 }); err != nil {
 return errors.Wrap(err, "failed to delete securityGroup")