From a46af84e6c1dfcc6643ca14501ab79a879059d70 Mon Sep 17 00:00:00 2001 From: Slavik Panasovets Date: Tue, 13 Sep 2022 15:10:46 +0000 Subject: [PATCH] Small refactor healthchecksl4, change functions order, rename function --- pkg/healthchecksl4/healthchecksl4.go | 56 ++++++++++++++-------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/pkg/healthchecksl4/healthchecksl4.go b/pkg/healthchecksl4/healthchecksl4.go index 7df47cc70c..9752ce2d2f 100644 --- a/pkg/healthchecksl4/healthchecksl4.go +++ b/pkg/healthchecksl4/healthchecksl4.go @@ -121,7 +121,7 @@ func (l4hc *l4HealthChecks) EnsureHealthCheck(svc *corev1.Service, namer namer.L } klog.V(3).Infof("L4 Healthcheck %s, path: %q, port %d", hcName, hcPath, hcPort) - _, hcLink, err := l4hc.ensureL4HealthCheckInternal(hcName, namespacedName, sharedHC, hcPath, hcPort, scope, l4Type) + _, hcLink, err := l4hc.ensureHealthCheck(hcName, namespacedName, sharedHC, hcPath, hcPort, scope, l4Type) if err != nil { return &EnsureL4HealthCheckResult{ GceResourceInError: annotations.HealthcheckResource, @@ -144,33 +144,7 @@ func (l4hc *l4HealthChecks) EnsureHealthCheck(svc *corev1.Service, namer namer.L } } -// DeleteHealthCheck deletes health check (and firewall rule) for l4 service. Checks if shared resources are safe to delete. -func (l4hc *l4HealthChecks) DeleteHealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) { - hcName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC) - hcFwName := namer.L4HealthCheckFirewall(svc.Namespace, svc.Name, sharedHC) - namespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} - klog.V(3).Infof("Trying to delete L4 healthcheck: %s and firewall rule %s from service %s, shared: %v", hcName, hcFwName, namespacedName.String(), sharedHC) - if sharedHC { - // We need to acquire a controller-wide mutex to ensure that in the case of a healthcheck shared between loadbalancers that the sync of the GCE resources is not performed in parallel. - l4hc.sharedResourcesLock.Lock() - defer l4hc.sharedResourcesLock.Unlock() - } - - err := l4hc.hcProvider.Delete(hcName, scope) - if err != nil { - // Ignore deletion error due to health check in use by another resource. - if !utils.IsInUsedByError(err) { - klog.Errorf("Failed to delete healthcheck for service %s - %v", namespacedName.String(), err) - return annotations.HealthcheckResource, err - } - klog.V(2).Infof("Failed to delete healthcheck %s: shared health check in use.", hcName) - return "", nil - } - // Health check deleted, now delete the firewall rule - return l4hc.deleteHealthCheckFirewall(svc, hcName, hcFwName, sharedHC, l4Type) -} - -func (l4hc *l4HealthChecks) ensureL4HealthCheckInternal(hcName string, svcName types.NamespacedName, shared bool, path string, port int32, scope meta.KeyType, l4Type utils.L4LBType) (*composite.HealthCheck, string, error) { +func (l4hc *l4HealthChecks) ensureHealthCheck(hcName string, svcName types.NamespacedName, shared bool, path string, port int32, scope meta.KeyType, l4Type utils.L4LBType) (*composite.HealthCheck, string, error) { hc, err := l4hc.hcProvider.Get(hcName, scope) if err != nil { return nil, "", err @@ -226,6 +200,32 @@ func (l4hc *l4HealthChecks) ensureFirewall(svc *corev1.Service, hcFwName string, return firewalls.EnsureL4LBFirewallForHc(svc, sharedHC, &hcFWRParams, l4hc.cloud, l4hc.recorderFactory.Recorder(svc.Namespace)) } +// DeleteHealthCheck deletes health check (and firewall rule) for l4 service. Checks if shared resources are safe to delete. +func (l4hc *l4HealthChecks) DeleteHealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) { + hcName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC) + hcFwName := namer.L4HealthCheckFirewall(svc.Namespace, svc.Name, sharedHC) + namespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} + klog.V(3).Infof("Trying to delete L4 healthcheck: %s and firewall rule %s from service %s, shared: %v", hcName, hcFwName, namespacedName.String(), sharedHC) + if sharedHC { + // We need to acquire a controller-wide mutex to ensure that in the case of a healthcheck shared between loadbalancers that the sync of the GCE resources is not performed in parallel. + l4hc.sharedResourcesLock.Lock() + defer l4hc.sharedResourcesLock.Unlock() + } + + err := l4hc.hcProvider.Delete(hcName, scope) + if err != nil { + // Ignore deletion error due to health check in use by another resource. + if !utils.IsInUsedByError(err) { + klog.Errorf("Failed to delete healthcheck for service %s - %v", namespacedName.String(), err) + return annotations.HealthcheckResource, err + } + klog.V(2).Infof("Failed to delete healthcheck %s: shared health check in use.", hcName) + return "", nil + } + // Health check deleted, now delete the firewall rule + return l4hc.deleteHealthCheckFirewall(svc, hcName, hcFwName, sharedHC, l4Type) +} + func (l4hc *l4HealthChecks) deleteHealthCheckFirewall(svc *corev1.Service, hcName, hcFwName string, sharedHC bool, l4Type utils.L4LBType) (string, error) { namespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace}