diff --git a/pkg/firewalls/firewalls_l4.go b/pkg/firewalls/firewalls_l4.go index db3f5525bc..f004d0649e 100644 --- a/pkg/firewalls/firewalls_l4.go +++ b/pkg/firewalls/firewalls_l4.go @@ -17,9 +17,6 @@ limitations under the License. package firewalls import ( - "strings" - "sync" - "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" "google.golang.org/api/compute/v1" v1 "k8s.io/api/core/v1" @@ -27,6 +24,7 @@ import ( "k8s.io/ingress-gce/pkg/utils" "k8s.io/klog" "k8s.io/legacy-cloud-providers/gce" + "strings" ) // FirewallParams holds all data needed to create firewall for L4 LB @@ -126,12 +124,8 @@ func ensureFirewall(svc *v1.Service, shared bool, params *FirewallParams, cloud } // EnsureL4LBFirewallForHc creates or updates firewall rule for shared or non-shared health check to nodes -func EnsureL4LBFirewallForHc(svc *v1.Service, shared bool, params *FirewallParams, cloud *gce.Cloud, sharedResourcesLock *sync.Mutex, recorder record.EventRecorder) error { +func EnsureL4LBFirewallForHc(svc *v1.Service, shared bool, params *FirewallParams, cloud *gce.Cloud, recorder record.EventRecorder) error { params.SourceRanges = gce.L4LoadBalancerSrcRanges() - if shared { - sharedResourcesLock.Lock() - defer sharedResourcesLock.Unlock() - } return ensureFirewall(svc, shared, params, cloud, recorder) } diff --git a/pkg/healthchecks/healthchecks_l4.go b/pkg/healthchecks/healthchecks_l4.go index f9c9734159..f016263390 100644 --- a/pkg/healthchecks/healthchecks_l4.go +++ b/pkg/healthchecks/healthchecks_l4.go @@ -61,6 +61,7 @@ func Initialize(cloud *gce.Cloud, recorderFactory events.RecorderProducer) { recorderFactory: recorderFactory, } } + func Fake(cloud *gce.Cloud, recorderFactory events.RecorderProducer) *l4HealthChecks { return &l4HealthChecks{ cloud: cloud, @@ -75,15 +76,17 @@ func GetL4() *l4HealthChecks { // EnsureL4HealthCheck creates a new HTTP health check for an L4 LoadBalancer service, based on the parameters provided. // If the healthcheck already exists, it is updated as needed. func (l4hc *l4HealthChecks) EnsureL4HealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) (string, string, string, string, error) { - // mutex? hcName, hcFwName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC) hcPath, hcPort := gce.GetNodesHealthCheckPath(), gce.GetNodesHealthCheckPort() - if !sharedHC { - hcPath, hcPort = helpers.GetServiceHealthCheckPathPort(svc) + + if sharedHC { + // lock out entire EnsureL4HealthCheck process l4hc.mutex.Lock() defer l4hc.mutex.Unlock() - + } else { + hcPath, hcPort = helpers.GetServiceHealthCheckPathPort(svc) } + namespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} _, hcLink, err := l4hc.ensureL4HealthCheckInternal(hcName, namespacedName, sharedHC, hcPath, hcPort, scope, l4Type) if err != nil { @@ -98,16 +101,18 @@ func (l4hc *l4HealthChecks) EnsureL4HealthCheck(svc *corev1.Service, namer namer } func (l4hc *l4HealthChecks) DeleteHealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) { - hcName, hcFwName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC) if sharedHC { + // lock out entire DeleteHealthCheck process l4hc.mutex.Lock() defer l4hc.mutex.Unlock() } - NamespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} + + hcName, hcFwName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC) + namespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} err := utils.IgnoreHTTPNotFound(l4hc.deleteHealthCheck(hcName, scope)) if err != nil { if !utils.IsInUsedByError(err) { - klog.Errorf("Failed to delete healthcheck for service %s - %v", NamespacedName.String(), err) + klog.Errorf("Failed to delete healthcheck for service %s - %v", namespacedName.String(), err) return annotations.HealthcheckResource, err } // Ignore deletion error due to health check in use by another resource. @@ -169,7 +174,7 @@ func (l4hc *l4HealthChecks) ensureFirewall(svc *corev1.Service, hcFwName string, Name: hcFwName, NodeNames: nodeNames, } - err := firewalls.EnsureL4LBFirewallForHc(svc, sharedHC, &hcFWRParams, l4hc.cloud, &l4hc.mutex, l4hc.recorderFactory.Recorder(svc.Namespace)) + err := firewalls.EnsureL4LBFirewallForHc(svc, sharedHC, &hcFWRParams, l4hc.cloud, l4hc.recorderFactory.Recorder(svc.Namespace)) if err != nil { return err }