From d982a335216df16de2c78f6f4449495c7d70ea95 Mon Sep 17 00:00:00 2001 From: Slavik Panasovets Date: Tue, 28 Jun 2022 11:10:32 +0000 Subject: [PATCH] Delete RBS annotations as a last step on garbage collection Deleting annotations updates service configuration and other controllers see this update. We should delete annotation last, so other controllers see updated service with all other fields (especially finalizer) removed --- pkg/l4lb/l4netlbcontroller.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/l4lb/l4netlbcontroller.go b/pkg/l4lb/l4netlbcontroller.go index d0a5f7aaba..f9cbb1a51c 100644 --- a/pkg/l4lb/l4netlbcontroller.go +++ b/pkg/l4lb/l4netlbcontroller.go @@ -499,11 +499,12 @@ func (lc *L4NetLBController) garbageCollectRBSNetLB(key string, svc *v1.Service) return result } - // Remove LB annotations from the Service when processing the finalizer. - if err := deleteL4RBSAnnotations(lc.ctx, svc); err != nil { - lc.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeWarning, "DeleteLoadBalancer", - "Error removing resource annotations: %v: %v", err) - result.Error = fmt.Errorf("Failed to reset resource annotations, err: %w", err) + // Try to delete instance group, instancePool.DeleteInstanceGroup ignores errors if resource is in use or not found. + // TODO(cezarygerard) replace with multi-IG management + if err := lc.instancePool.DeleteInstanceGroup(lc.namer.InstanceGroup()); err != nil { + lc.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeWarning, "DeleteInstanceGroupFailed", + "Error deleting delete Instance Group from L4 External LoadBalancer, err: %v", err) + result.Error = fmt.Errorf("Failed to delete Instance Group, err: %w", err) return result } @@ -514,12 +515,11 @@ func (lc *L4NetLBController) garbageCollectRBSNetLB(key string, svc *v1.Service) return result } - // Try to delete instance group, instancePool.DeleteInstanceGroup ignores errors if resource is in use or not found. - // TODO(cezarygerard) replace with multi-IG management - if err := lc.instancePool.DeleteInstanceGroup(lc.namer.InstanceGroup()); err != nil { - lc.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeWarning, "DeleteInstanceGroupFailed", - "Error deleting delete Instance Group from L4 External LoadBalancer, err: %v", err) - result.Error = fmt.Errorf("Failed to delete Instance Group, err: %w", err) + // IMPORTANT: Remove LB annotations from the Service LAST, cause changing service fields are emitted as service update to other controllers + if err := deleteL4RBSAnnotations(lc.ctx, svc); err != nil { + lc.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeWarning, "DeleteLoadBalancer", + "Error removing resource annotations: %v: %v", err) + result.Error = fmt.Errorf("Failed to reset resource annotations, err: %w", err) return result }