Merge pull request #105 from mandre/keepalived-iptables

Bug 1888301: Add check for iptables rule to keepalived-monitor

cmd/dynkeepalived/dynkeepalived.go

@@ -32,6 +32,14 @@ func main() {
 			if err != nil {
 				dnsVip = nil
 			}
+			apiPort, err := cmd.Flags().GetUint16("api-port")
+			if err != nil {
+				return err
+			}
+			lbPort, err := cmd.Flags().GetUint16("lb-port")
+			if err != nil {
+				return err
+			}
 
 			checkInterval, err := cmd.Flags().GetDuration("check-interval")
 			if err != nil {
@@ -42,14 +50,16 @@ func main() {
 				return err
 			}
 
-			return monitor.KeepalivedWatch(args[0], clusterConfigPath, args[1], args[2], apiVip, ingressVip, dnsVip, checkInterval)
+			return monitor.KeepalivedWatch(args[0], clusterConfigPath, args[1], args[2], apiVip, ingressVip, dnsVip, apiPort, lbPort, checkInterval)
 		},
 	}
 	rootCmd.PersistentFlags().StringP("cluster-config", "c", "", "Path to cluster-config ConfigMap to retrieve ControlPlane info")
 	rootCmd.Flags().Duration("check-interval", time.Second*10, "Time between keepalived watch checks")
 	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")
+	rootCmd.Flags().Uint16("lb-port", 9445, "Port where the API HAProxy LB will listen")
 	if err := rootCmd.Execute(); err != nil {
 		log.Fatalf("Failed due to %s", err)
 	}

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", 9445, "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", 9445, "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 {

pkg/config/node.go

@@ -258,6 +258,14 @@ func GetConfig(kubeconfigPath, clusterConfigPath, resolvConfPath string, apiVip
 	node.Cluster.VIPNetmask = prefix
 	node.VRRPInterface = vipIface.Name
 
+	// We can't populate this with GetLBConfig because in many cases the
+	// backends won't be available yet.
+	node.LBConfig = ApiLBConfig{
+		ApiPort:  apiPort,
+		LbPort:   lbPort,
+		StatPort: statPort,
+	}
+
 	return node, err
 }
 

pkg/monitor/dynkeepalived.go

@@ -12,9 +12,12 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-const keepalivedControlSock = "/var/run/keepalived/keepalived.sock"
+const (
+	keepalivedControlSock = "/var/run/keepalived/keepalived.sock"
+	iptablesFilePath      = "/var/run/keepalived/iptables-rule-exists"
+)
 
-func KeepalivedWatch(kubeconfigPath, clusterConfigPath, templatePath, cfgPath string, apiVip, ingressVip, dnsVip net.IP, interval time.Duration) error {
+func KeepalivedWatch(kubeconfigPath, clusterConfigPath, templatePath, cfgPath string, apiVip, ingressVip, dnsVip net.IP, apiPort, lbPort uint16, interval time.Duration) error {
 	var prevConfig *config.Node
 
 	signals := make(chan os.Signal, 1)
@@ -63,6 +66,30 @@ func KeepalivedWatch(kubeconfigPath, clusterConfigPath, templatePath, cfgPath st
 				}
 			}
 			prevConfig = &newConfig
+
+			// Signal to keepalived whether the haproxy firewall rule is in place
+			ruleExists, err := checkHAProxyFirewallRules(apiVip.String(), apiPort, lbPort)
+			if err != nil {
+				log.Error("Failed to check for haproxy firewall rule")
+			} else {
+				_, err := os.Stat(iptablesFilePath)
+				fileExists := !os.IsNotExist(err)
+				if ruleExists {
+					if !fileExists {
+						_, err := os.Create(iptablesFilePath)
+						if err != nil {
+							log.WithFields(logrus.Fields{"path": iptablesFilePath}).Error("Failed to create file")
+						}
+					}
+				} else {
+					if fileExists {
+						err := os.Remove(iptablesFilePath)
+						if err != nil {
+							log.WithFields(logrus.Fields{"path": iptablesFilePath}).Error("Failed to remove file")
+						}
+					}
+				}
+			}
 			time.Sleep(interval)
 		}
 	}

pkg/monitor/iptables.go

@@ -10,14 +10,18 @@ import (
 )
 
 const (
-	table = "nat"
-	chain = "PREROUTING"
+	table       = "nat"
+	isLoopback  = true
+	notLoopback = false
 )
 
-func getHAProxyRuleSpec(apiVip string, apiPort, lbPort uint16) (ruleSpec []string, err error) {
+func getHAProxyRuleSpec(apiVip string, apiPort, lbPort uint16, loopback bool) (ruleSpec []string, err error) {
 	apiPortStr := strconv.Itoa(int(apiPort))
 	lbPortStr := strconv.Itoa(int(lbPort))
 	ruleSpec = []string{"--dst", apiVip, "-p", "tcp", "--dport", apiPortStr, "-j", "REDIRECT", "--to-ports", lbPortStr, "-m", "comment", "--comment", "OCP_API_LB_REDIRECT"}
+	if loopback {
+		ruleSpec = append(ruleSpec, "-o", "lo")
+	}
 	return ruleSpec, err
 }
 
@@ -29,42 +33,92 @@ 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
 	}
 
-	ruleSpec, err := getHAProxyRuleSpec(apiVip, apiPort, lbPort)
+	ruleSpec, err := getHAProxyRuleSpec(apiVip, apiPort, lbPort, notLoopback)
 	if err != nil {
 		return err
 	}
 
+	chain := "PREROUTING"
 	if exists, _ := ipt.Exists(table, chain, ruleSpec...); exists {
 		log.WithFields(logrus.Fields{
 			"spec": strings.Join(ruleSpec, " "),
 		}).Info("Removing existing nat PREROUTING rule")
+		err = ipt.Delete(table, chain, ruleSpec...)
+		if err != nil {
+			return err
+		}
+	}
+	ruleSpec, err = getHAProxyRuleSpec(apiVip, apiPort, lbPort, isLoopback)
+	if err != nil {
+		return err
+	}
+	chain = "OUTPUT"
+	if exists, _ := ipt.Exists(table, chain, ruleSpec...); exists {
+		log.WithFields(logrus.Fields{
+			"spec": strings.Join(ruleSpec, " "),
+		}).Info("Removing existing nat OUTPUT rule")
 		return ipt.Delete(table, chain, ruleSpec...)
 	}
 	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
 	}
 
-	ruleSpec, err := getHAProxyRuleSpec(apiVip, apiPort, lbPort)
+	ruleSpec, err := getHAProxyRuleSpec(apiVip, apiPort, lbPort, notLoopback)
 	if err != nil {
 		return err
 	}
+	chain := "PREROUTING"
 	if exists, _ := ipt.Exists(table, chain, ruleSpec...); exists {
 		return nil
-	} else {
-		log.WithFields(logrus.Fields{
-			"spec": strings.Join(ruleSpec, " "),
-		}).Info("Inserting nat PREROUTING rule")
-		return ipt.Insert(table, chain, 1, ruleSpec...)
 	}
+	log.WithFields(logrus.Fields{
+		"spec": strings.Join(ruleSpec, " "),
+	}).Info("Inserting nat PREROUTING rule")
+	err = ipt.Insert(table, chain, 1, ruleSpec...)
+	if err != nil {
+		return err
+	}
+	ruleSpec, err = getHAProxyRuleSpec(apiVip, apiPort, lbPort, isLoopback)
+	if err != nil {
+		return err
+	}
+	chain = "OUTPUT"
+	if exists, _ := ipt.Exists(table, chain, ruleSpec...); exists {
+		return nil
+	}
+	log.WithFields(logrus.Fields{
+		"spec": strings.Join(ruleSpec, " "),
+	}).Info("Inserting nat OUTPUT rule")
+	return ipt.Insert(table, chain, 1, ruleSpec...)
+}
+
+func checkHAProxyFirewallRules(apiVip string, apiPort, lbPort uint16) (bool, error) {
+	ipt, err := iptables.NewWithProtocol(getProtocolbyIp(apiVip))
+	if err != nil {
+		return false, err
+	}
+
+	ruleSpec, err := getHAProxyRuleSpec(apiVip, apiPort, lbPort, notLoopback)
+	if err != nil {
+		return false, err
+	}
+	preroutingExists, _ := ipt.Exists(table, "PREROUTING", ruleSpec...)
+
+	ruleSpec, err = getHAProxyRuleSpec(apiVip, apiPort, lbPort, isLoopback)
+	if err != nil {
+		return false, err
+	}
+	outputExists, _ := ipt.Exists(table, "OUTPUT", ruleSpec...)
+	return (preroutingExists && outputExists), nil
 }

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)
 		}

pkg/render/render.go

@@ -34,6 +34,27 @@ func RenderFile(renderPath, templatePath string, cfg interface{}) error {
 	}
 	defer renderFile.Close()
 
+	// NOTE For some reason, the template file for keepalived.conf has the
+	// executable bit set. Let's skip it because keepalived refuses to
+	// start when its configuration file is executable.
+	if renderPath != "/etc/keepalived/keepalived.conf" {
+		// Make sure we propagate any special permissions
+		templateStat, err := os.Stat(templatePath)
+		if err != nil {
+			log.WithFields(logrus.Fields{
+				"path": templatePath,
+			}).Error("Failed to stat template")
+			return err
+		}
+		err = os.Chmod(renderPath, templateStat.Mode())
+		if err != nil {
+			log.WithFields(logrus.Fields{
+				"path": renderPath,
+			}).Error("Failed to set permissions on file")
+			return err
+		}
+	}
+
 	log.WithFields(logrus.Fields{
 		"path": renderPath,
 	}).Info("Runtimecfg rendering template")