From b31874fe826844af62d92fcfd5c5a43c2cc6d1ba Mon Sep 17 00:00:00 2001
From: Minhan Xia <mixia@google.com>
Date: Tue, 16 Aug 2016 10:56:26 -0700
Subject: [PATCH] bug fixes and nits

---
 pkg/kubelet/kubelet_network.go |  2 +-
 pkg/proxy/iptables/proxier.go  | 39 +++++++++++++++++-----------------
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/pkg/kubelet/kubelet_network.go b/pkg/kubelet/kubelet_network.go
index ed668e951d4a9..69008d384d543 100644
--- a/pkg/kubelet/kubelet_network.go
+++ b/pkg/kubelet/kubelet_network.go
@@ -43,7 +43,7 @@ const (
 	// kubernetes postrouting rules
 	KubePostroutingChain utiliptables.Chain = "KUBE-POSTROUTING"
 
-	// kubernetes postrouting rules
+	// kubernetes firewall rules
 	KubeFirewallChain utiliptables.Chain = "KUBE-FIREWALL"
 )
 
diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go
index 1405d492bb963..23eb097fd7742 100644
--- a/pkg/proxy/iptables/proxier.go
+++ b/pkg/proxy/iptables/proxier.go
@@ -581,11 +581,11 @@ func servicePortChainName(s proxy.ServicePortName, protocol string) utiliptables
 	return utiliptables.Chain("KUBE-SVC-" + portProtoHash(s, protocol))
 }
 
-// servicePortChainName takes the ServicePortName for a service and
+// externalLoadBalancerChainName takes the ServicePortName for a service and
 // returns the associated iptables chain.  This is computed by hashing (sha256)
-// then encoding to base32 and truncating with the prefix "KUBE-FW-".
-func serviceFirewallChainName(s proxy.ServicePortName, protocol string) utiliptables.Chain {
-	return utiliptables.Chain("KUBE-FW-" + portProtoHash(s, protocol))
+// then encoding to base32 and truncating with the prefix "KUBE-XLB-".
+func externalLoadBalancerChainName(s proxy.ServicePortName, protocol string) utiliptables.Chain {
+	return utiliptables.Chain("KUBE-XLB-" + portProtoHash(s, protocol))
 }
 
 // This is the same as servicePortChainName but with the endpoint included.
@@ -872,33 +872,32 @@ func (proxier *Proxier) syncProxyRules() {
 		// Capture load-balancer ingress.
 		for _, ingress := range svcInfo.loadBalancerStatus.Ingress {
 			if ingress.IP != "" {
-				args := []string{
-					"-A", string(kubeServicesChain),
-					"-m", "comment", "--comment", fmt.Sprintf(`"%s loadbalancer IP"`, svcName.String()),
-					"-m", protocol, "-p", protocol,
-					"-d", fmt.Sprintf("%s/32", ingress.IP),
-					"--dport", fmt.Sprintf("%d", svcInfo.port),
-				}
-				// create service firewall chain
-				fwChain := serviceFirewallChainName(svcName, protocol)
-				if chain, ok := existingNATChains[fwChain]; ok {
+				// create service external loadbalancer chain
+				xlbChain := externalLoadBalancerChainName(svcName, protocol)
+				if chain, ok := existingNATChains[xlbChain]; ok {
 					writeLine(natChains, chain)
 				} else {
-					writeLine(natChains, utiliptables.MakeChainLine(fwChain))
+					writeLine(natChains, utiliptables.MakeChainLine(xlbChain))
 				}
-				// jump to service firewall chain
+				activeNATChains[xlbChain] = true
 				// The service firewall rules are created based on ServiceSpec.loadBalancerSourceRanges field.
 				// This currently works for loadbalancers that preserves source ips.
 				// For loadbalancers which direct traffic to service NodePort, the firewall rules will not apply.
-				writeLine(natRules, append(args, "-j", string(fwChain))...)
 
-				args = []string{
-					"-A", string(fwChain),
+				args := []string{
+					"-A", string(kubeServicesChain),
 					"-m", "comment", "--comment", fmt.Sprintf(`"%s loadbalancer IP"`, svcName.String()),
 					"-m", protocol, "-p", protocol,
 					"-d", fmt.Sprintf("%s/32", ingress.IP),
 					"--dport", fmt.Sprintf("%d", svcInfo.port),
 				}
+				// jump to service firewall chain
+				writeLine(natRules, append(args, "-j", string(xlbChain))...)
+
+				args = []string{
+					"-A", string(xlbChain),
+					"-m", "comment", "--comment", fmt.Sprintf(`"%s loadbalancer IP"`, svcName.String()),
+				}
 				// We have to SNAT packets from external IPs.
 				writeLine(natRules, append(args, "-j", string(KubeMarkMasqChain))...)
 
@@ -1056,7 +1055,7 @@ func (proxier *Proxier) syncProxyRules() {
 	for chain := range existingNATChains {
 		if !activeNATChains[chain] {
 			chainString := string(chain)
-			if !strings.HasPrefix(chainString, "KUBE-SVC-") && !strings.HasPrefix(chainString, "KUBE-SEP-") {
+			if !strings.HasPrefix(chainString, "KUBE-SVC-") && !strings.HasPrefix(chainString, "KUBE-SEP-") && !strings.HasPrefix(chainString, "KUBE-XLB-") {
 				// Ignore chains that aren't ours.
 				continue
 			}