Merge pull request #1705 from cezarygerard/awesome-firewall-healthchecks

Rewrite L4 healthchecks creation and deletion

cmd/glbc/main.go

@@ -26,6 +26,7 @@ import (
 	flag "github.com/spf13/pflag"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/ingress-gce/pkg/frontendconfig"
+	"k8s.io/ingress-gce/pkg/healthchecks"
 	"k8s.io/ingress-gce/pkg/ingparams"
 	"k8s.io/ingress-gce/pkg/l4lb"
 	"k8s.io/ingress-gce/pkg/psc"
@@ -274,6 +275,8 @@ func runControllers(ctx *ingctx.ControllerContext) {
 
 	fwc := firewalls.NewFirewallController(ctx, flags.F.NodePortRanges.Values())
 
+	healthchecks.InitializeL4(ctx.Cloud, ctx)
+
 	if flags.F.RunL4Controller {
 		l4Controller := l4lb.NewILBController(ctx, stopCh)
 		go l4Controller.Run()

pkg/firewalls/firewalls_l4.go

@@ -17,16 +17,14 @@ 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"
 	"k8s.io/client-go/tools/record"
 	"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
@@ -50,7 +48,7 @@ func EnsureL4FirewallRule(cloud *gce.Cloud, nsName string, params *FirewallParam
 	if err != nil {
 		return err
 	}
-	fwDesc, err := utils.MakeL4LBServiceDescription(nsName, params.IP, meta.VersionGA, sharedRule, params.L4Type)
+	fwDesc, err := utils.MakeL4LBFirewallDescription(nsName, params.IP, meta.VersionGA, sharedRule)
 	if err != nil {
 		klog.Warningf("EnsureL4FirewallRule(%v): failed to generate description for L4 %s rule, err: %v", params.Name, params.L4Type.ToString(), err)
 	}
@@ -78,7 +76,9 @@ func EnsureL4FirewallRule(cloud *gce.Cloud, nsName string, params *FirewallParam
 		}
 		return err
 	}
-	if firewallRuleEqual(expectedFw, existingFw) {
+
+	// Don't compare the "description" field for shared firewall rules
+	if firewallRuleEqual(expectedFw, existingFw, sharedRule) {
 		return nil
 	}
 	klog.V(2).Infof("EnsureL4FirewallRule(%v): updating L4 %s firewall", params.Name, params.L4Type.ToString())
@@ -103,13 +103,33 @@ func EnsureL4FirewallRuleDeleted(cloud *gce.Cloud, fwName string) error {
 	return nil
 }
 
-func firewallRuleEqual(a, b *compute.Firewall) bool {
-	return a.Description == b.Description &&
-		len(a.Allowed) == 1 && len(a.Allowed) == len(b.Allowed) &&
-		a.Allowed[0].IPProtocol == b.Allowed[0].IPProtocol &&
-		utils.EqualStringSets(a.Allowed[0].Ports, b.Allowed[0].Ports) &&
-		utils.EqualStringSets(a.SourceRanges, b.SourceRanges) &&
-		utils.EqualStringSets(a.TargetTags, b.TargetTags)
+func firewallRuleEqual(a, b *compute.Firewall, skipDescription bool) bool {
+	if len(a.Allowed) != len(b.Allowed) {
+		return false
+	}
+	for i := range a.Allowed {
+		if !allowRulesEqual(a.Allowed[i], b.Allowed[i]) {
+			return false
+		}
+	}
+
+	if !utils.EqualStringSets(a.SourceRanges, b.SourceRanges) {
+		return false
+	}
+
+	if !utils.EqualStringSets(a.TargetTags, b.TargetTags) {
+		return false
+	}
+
+	if !skipDescription && a.Description != b.Description {
+		return false
+	}
+	return true
+}
+
+func allowRulesEqual(a *compute.FirewallAllowed, b *compute.FirewallAllowed) bool {
+	return a.IPProtocol == b.IPProtocol &&
+		utils.EqualStringSets(a.Ports, b.Ports)
 }
 
 func ensureFirewall(svc *v1.Service, shared bool, params *FirewallParams, cloud *gce.Cloud, recorder record.EventRecorder) error {
@@ -126,12 +146,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)
 }