Pick nits on iptables firewall rule code

Addressing some review comments on openshift#70. No functional changes, just some grammar/naming/efficiency improvements.
cybertron · Jul 9, 2020 · 793b819 · 793b819
1 parent d2973ab
commit 793b819
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 19 deletions.
diff --git a/cmd/dynkeepalived/dynkeepalived.go b/cmd/dynkeepalived/dynkeepalived.go
@@ -58,8 +58,8 @@ func main() {
 	rootCmd.Flags().IP("api-vip", nil, "Virtual IP Address to reach the OpenShift API")
 	rootCmd.PersistentFlags().IP("ingress-vip", nil, "Virtual IP Address to reach the OpenShift Ingress Routers")
 	rootCmd.PersistentFlags().IP("dns-vip", nil, "Virtual IP Address to reach an OpenShift node resolving DNS server")
-	rootCmd.Flags().Uint16("api-port", 6443, "Port where the OpenShift API listens at")
-	rootCmd.Flags().Uint16("lb-port", 9443, "Port where the API HAProxy LB will listen at")
+	rootCmd.Flags().Uint16("api-port", 6443, "Port where the OpenShift API listens")
+	rootCmd.Flags().Uint16("lb-port", 9443, "Port where the API HAProxy LB will listen")
 	if err := rootCmd.Execute(); err != nil {
 		log.Fatalf("Failed due to %s", err)
 	}

diff --git a/cmd/monitor/monitor.go b/cmd/monitor/monitor.go
@@ -51,9 +51,9 @@ func main() {
 			return monitor.Monitor(args[0], clusterName, clusterDomain, args[1], args[2], apiVip.String(), apiPort, lbPort, statPort, checkInterval)
 		},
 	}
-	rootCmd.Flags().Uint16("api-port", 6443, "Port where the OpenShift API listens at")
-	rootCmd.Flags().Uint16("lb-port", 9443, "Port where the API HAProxy LB will listen at")
-	rootCmd.Flags().Uint16("stat-port", 50000, "Port where the HAProxy stats API will listen at")
+	rootCmd.Flags().Uint16("api-port", 6443, "Port where the OpenShift API listens")
+	rootCmd.Flags().Uint16("lb-port", 9443, "Port where the API HAProxy LB will listen")
+	rootCmd.Flags().Uint16("stat-port", 50000, "Port where the HAProxy stats API will listen")
 	rootCmd.Flags().Duration("check-interval", time.Second*6, "Time between monitor checks")
 	rootCmd.Flags().IP("api-vip", nil, "Virtual IP Address to reach the OpenShift API")
 	if err := rootCmd.Execute(); err != nil {

diff --git a/pkg/monitor/dynkeepalived.go b/pkg/monitor/dynkeepalived.go
@@ -13,6 +13,7 @@ import (
 )
 
 const keepalivedControlSock = "/var/run/keepalived/keepalived.sock"
+const iptablesFilePath = "/var/run/keepalived/iptables-rule-exists"
 
 func KeepalivedWatch(kubeconfigPath, clusterConfigPath, templatePath, cfgPath string, apiVip, ingressVip, dnsVip net.IP, apiPort, lbPort uint16, interval time.Duration) error {
 	var prevConfig *config.Node
@@ -65,25 +66,24 @@ func KeepalivedWatch(kubeconfigPath, clusterConfigPath, templatePath, cfgPath st
 			prevConfig = &newConfig
 
 			// Signal to keepalived whether the haproxy firewall rule is in place
-			ruleExists, err := checkHAProxyPreRoutingRule(apiVip.String(), apiPort, lbPort)
+			ruleExists, err := checkHAProxyFirewallRules(apiVip.String(), apiPort, lbPort)
 			if err != nil {
 				log.Error("Failed to check for haproxy firewall rule")
 			} else {
-				filePath := "/var/run/keepalived/iptables-rule-exists"
-				_, err := os.Stat(filePath)
+				_, err := os.Stat(iptablesFilePath)
 				fileExists := !os.IsNotExist(err)
 				if ruleExists {
 					if !fileExists {
-						_, err := os.Create(filePath)
+						_, err := os.Create(iptablesFilePath)
 						if err != nil {
-							log.WithFields(logrus.Fields{"path": filePath}).Error("Failed to create file")
+							log.WithFields(logrus.Fields{"path": iptablesFilePath}).Error("Failed to create file")
 						}
 					}
 				} else {
 					if fileExists {
-						err := os.Remove(filePath)
+						err := os.Remove(iptablesFilePath)
 						if err != nil {
-							log.WithFields(logrus.Fields{"path": filePath}).Error("Failed to remove file")
+							log.WithFields(logrus.Fields{"path": iptablesFilePath}).Error("Failed to remove file")
 						}
 					}
 				}

diff --git a/pkg/monitor/iptables.go b/pkg/monitor/iptables.go
@@ -31,7 +31,7 @@ func getProtocolbyIp(ipStr string) iptables.Protocol {
 	return iptables.ProtocolIPv6
 }
 
-func cleanHAProxyPreRoutingRule(apiVip string, apiPort, lbPort uint16) error {
+func cleanHAProxyFirewallRules(apiVip string, apiPort, lbPort uint16) error {
 	ipt, err := iptables.NewWithProtocol(getProtocolbyIp(apiVip))
 	if err != nil {
 		return err
@@ -66,7 +66,7 @@ func cleanHAProxyPreRoutingRule(apiVip string, apiPort, lbPort uint16) error {
 	return nil
 }
 
-func ensureHAProxyPreRoutingRule(apiVip string, apiPort, lbPort uint16) error {
+func ensureHAProxyFirewallRules(apiVip string, apiPort, lbPort uint16) error {
 	ipt, err := iptables.NewWithProtocol(getProtocolbyIp(apiVip))
 	if err != nil {
 		return err
@@ -103,7 +103,7 @@ func ensureHAProxyPreRoutingRule(apiVip string, apiPort, lbPort uint16) error {
 	}
 }
 
-func checkHAProxyPreRoutingRule(apiVip string, apiPort, lbPort uint16) (bool, error) {
+func checkHAProxyFirewallRules(apiVip string, apiPort, lbPort uint16) (bool, error) {
 	ipt, err := iptables.NewWithProtocol(getProtocolbyIp(apiVip))
 	if err != nil {
 		return false, err

diff --git a/pkg/monitor/monitor.go b/pkg/monitor/monitor.go
@@ -53,7 +53,7 @@ func Monitor(kubeconfigPath, clusterName, clusterDomain, templatePath, cfgPath,
 	for {
 		select {
 		case <-done:
-			cleanHAProxyPreRoutingRule(apiVip, apiPort, lbPort)
+			cleanHAProxyFirewallRules(apiVip, apiPort, lbPort)
 			return nil
 		default:
 			config, err := config.GetLBConfig(kubeconfigPath, apiPort, lbPort, statPort, net.ParseIP(apiVip))
@@ -111,15 +111,15 @@ func Monitor(kubeconfigPath, clusterName, clusterDomain, templatePath, cfgPath,
 				if oldK8sHealthSts != K8sHealthSts {
 					log.Info("API is reachable through HAProxy")
 				}
-				err := ensureHAProxyPreRoutingRule(apiVip, apiPort, lbPort)
+				err := ensureHAProxyFirewallRules(apiVip, apiPort, lbPort)
 				if err != nil {
-					log.WithFields(logrus.Fields{"err": err}).Error("Failed to ensure HAProxy PREROUTING rule to direct traffic to the LB")
+					log.WithFields(logrus.Fields{"err": err}).Error("Failed to ensure HAProxy firewall rules to direct traffic to the LB")
 				}
 			} else {
 				if oldK8sHealthSts != K8sHealthSts {
 					log.Info("API is not reachable through HAProxy")
 				}
-				cleanHAProxyPreRoutingRule(apiVip, apiPort, lbPort)
+				cleanHAProxyFirewallRules(apiVip, apiPort, lbPort)
 			}
 			time.Sleep(interval)
 		}