Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check for iptables rule to keepalived-monitor #70

Merged
12 changes: 11 additions & 1 deletion cmd/dynkeepalived/dynkeepalived.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/monitor/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 8 additions & 0 deletions pkg/config/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,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
}

Expand Down
27 changes: 26 additions & 1 deletion pkg/monitor/dynkeepalived.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const keepalivedControlSock = "/var/run/keepalived/keepalived.sock"
const cfgKeepalivedChangeThreshold uint8 = 3
const dummyPortNum uint16 = 123
const unicastPatternInCfgFile = "unicast_peer"
const iptablesFilePath = "/var/run/keepalived/iptables-rule-exists"

var (
g_BootstrapIP string
Expand Down Expand Up @@ -94,7 +95,7 @@ func retrieveBootstrapIpAddr(apiVip string) {
}
}

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 appliedConfig, curConfig, prevConfig *config.Node
var configChangeCtr uint8 = 0

Expand Down Expand Up @@ -172,6 +173,30 @@ func KeepalivedWatch(kubeconfigPath, clusterConfigPath, templatePath, cfgPath st
configChangeCtr = 0
}
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)
}
}
Expand Down
66 changes: 60 additions & 6 deletions pkg/monitor/iptables.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ import (

const (
table = "nat"
chain = "PREROUTING"
)

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
}

Expand All @@ -29,42 +31,94 @@ 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, false)
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, true)
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, false)
if err != nil {
return err
}
chain := "PREROUTING"
if exists, _ := ipt.Exists(table, chain, ruleSpec...); exists {
return nil
} else {
cybertron marked this conversation as resolved.
Show resolved Hide resolved
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, true)
if err != nil {
return err
}
chain = "OUTPUT"
if exists, _ := ipt.Exists(table, chain, ruleSpec...); exists {
return nil
} else {
cybertron marked this conversation as resolved.
Show resolved Hide resolved
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, false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic param at the end doesn't really convey what it is for. One would have to look at the function definition to know, and even then it's not totally clear. Can we change it to maybe add a type for it?

if err != nil {
return false, err
}
preroutingExists, _ := ipt.Exists(table, "PREROUTING", ruleSpec...)

ruleSpec, err = getHAProxyRuleSpec(apiVip, apiPort, lbPort, true)
if err != nil {
return false, err
}
outputExists, _ := ipt.Exists(table, "OUTPUT", ruleSpec...)
return (preroutingExists && outputExists), nil
}
8 changes: 4 additions & 4 deletions pkg/monitor/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)
}
Expand Down
16 changes: 16 additions & 0 deletions pkg/render/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,22 @@ func RenderFile(renderPath, templatePath string, cfg interface{}) error {
}
defer renderFile.Close()

// 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")
Expand Down