Skip to content

Commit

Permalink
Rebase on v0.18, fix comments with iptables-restore, fix unit tests a…
Browse files Browse the repository at this point in the history 
…nd review

-> Changes from the review:

- refactor protocol to proto
- fix check version for wait command available on iptables-restore
- apply "Error strings should not be capitalized"
- lower case for iptables-restore
- refactor function ApplyPartial to ApplyWithoutFlush
- remove unused function ApplyFully
- comments functions
  • Loading branch information
@louiznk
louiznk committed Jun 23, 2022
1 parent 510f150 commit 121db21
Show file tree
Hide file tree
Showing 6 changed files with 664 additions and 126 deletions.
37 changes: 36 additions & 1 deletion dist/functional-test-k8s.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ start_flannel() {
$FLANNEL_DOCKER_IMAGE \
-c "mkdir -p /etc/kube-flannel && \
echo '$flannel_conf' > /etc/kube-flannel/net-conf.json && \
/opt/bin/flanneld --kube-subnet-mgr --kube-api-url $k8s_endpt" >/dev/null
/opt/bin/flanneld --kube-subnet-mgr --ip-masq --kube-api-url $k8s_endpt" >/dev/null

while ! docker exec flannel-e2e-test-flannel$host_num ls /run/flannel/subnet.env >/dev/null 2>&1; do
status=$(docker inspect --format='{{.State.Status}}' flannel-e2e-test-flannel$host_num)
Expand Down Expand Up @@ -121,26 +121,30 @@ test_vxlan() {
start_flannel vxlan
create_ping_dest # creates ping_dest1 and ping_dest2 variables
pings
check_iptables
}

if [[ ${ARCH} == "amd64" ]]; then
test_udp() {
start_flannel udp
create_ping_dest # creates ping_dest1 and ping_dest2 variables
pings
check_iptables
}
fi

test_host-gw() {
start_flannel host-gw
create_ping_dest # creates ping_dest1 and ping_dest2 variables
pings
check_iptables
}

test_ipip() {
start_flannel ipip
create_ping_dest # creates ping_dest1 and ping_dest2 variables
pings
check_iptables
}

test_public-ip-overwrite(){
Expand Down Expand Up @@ -168,6 +172,37 @@ pings() {
assert "docker exec --privileged flannel-e2e-test-flannel2 /bin/ping -c 5 $ping_dest1" "Host 2 cannot ping host 1"
}

check_iptables() {
read -r -d '' POSTROUTING_RULES_FLANNEL1 << EOM
-P POSTROUTING ACCEPT
-A POSTROUTING -s 10.10.0.0/16 -d 10.10.0.0/16 -m comment --comment "flanneld masq" -j RETURN
-A POSTROUTING -s 10.10.0.0/16 ! -d 224.0.0.0/4 -m comment --comment "flanneld masq" -j MASQUERADE --random-fully
-A POSTROUTING ! -s 10.10.0.0/16 -d 10.10.1.0/24 -m comment --comment "flanneld masq" -j RETURN
-A POSTROUTING ! -s 10.10.0.0/16 -d 10.10.0.0/16 -m comment --comment "flanneld masq" -j MASQUERADE --random-fully
EOM
read -r -d '' POSTROUTING_RULES_FLANNEL2 << EOM
-P POSTROUTING ACCEPT
-A POSTROUTING -s 10.10.0.0/16 -d 10.10.0.0/16 -m comment --comment "flanneld masq" -j RETURN
-A POSTROUTING -s 10.10.0.0/16 ! -d 224.0.0.0/4 -m comment --comment "flanneld masq" -j MASQUERADE --random-fully
-A POSTROUTING ! -s 10.10.0.0/16 -d 10.10.2.0/24 -m comment --comment "flanneld masq" -j RETURN
-A POSTROUTING ! -s 10.10.0.0/16 -d 10.10.0.0/16 -m comment --comment "flanneld masq" -j MASQUERADE --random-fully
EOM
read -r -d '' FORWARD_RULES << EOM
-P FORWARD ACCEPT
-A FORWARD -s 10.10.0.0/16 -m comment --comment "flanneld forward" -j ACCEPT
-A FORWARD -d 10.10.0.0/16 -m comment --comment "flanneld forward" -j ACCEPT
EOM
# check masquerade & forward rules
assert_equals "$POSTROUTING_RULES_FLANNEL1" \
"$(docker exec --privileged flannel-e2e-test-flannel1 /sbin/iptables -t nat -S POSTROUTING)" "Host 1 has not expected postrouting rules"
assert_equals "$POSTROUTING_RULES_FLANNEL2" \
"$(docker exec --privileged flannel-e2e-test-flannel2 /sbin/iptables -t nat -S POSTROUTING)" "Host 2 has not expected postrouting rules"
assert_equals "$FORWARD_RULES" \
"$(docker exec --privileged flannel-e2e-test-flannel1 /sbin/iptables -t filter -S FORWARD)" "Host 1 has not expected forward rules"
assert_equals "$FORWARD_RULES" \
"$(docker exec --privileged flannel-e2e-test-flannel2 /sbin/iptables -t filter -S FORWARD)" "Host 2 has not expected forward rules"
}

test_manifest() {
# This just tests that the API server accepts the manifest, not that it actually acts on it correctly.
assert "cat ../Documentation/kube-flannel.yml | docker run -i --rm --net=host ${HYPERKUBE_IMG}:v$K8S_VERSION ${HYPERKUBE_CMD} kubectl create -f -"
Expand Down
6 changes: 3 additions & 3 deletions main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func main() {
os.Exit(1)
}
log.Infof("Setting up masking rules")
go network.SetupAndEnsureIPTables(network.MasqRules(config.Network, bn.Lease()), opts.iptablesResyncSeconds)
go network.SetupAndEnsureIP4Tables(network.MasqRules(config.Network, bn.Lease()), opts.iptablesResyncSeconds)

}
if config.EnableIPv6 {
Expand All @@ -360,7 +360,7 @@ func main() {
if opts.iptablesForwardRules {
if config.EnableIPv4 {
log.Infof("Changing default FORWARD chain policy to ACCEPT")
go network.SetupAndEnsureIPTables(network.ForwardRules(config.Network.String()), opts.iptablesResyncSeconds)
go network.SetupAndEnsureIP4Tables(network.ForwardRules(config.Network.String()), opts.iptablesResyncSeconds)
}
if config.EnableIPv6 {
log.Infof("IPv6: Changing default FORWARD chain policy to ACCEPT")
Expand Down Expand Up @@ -413,7 +413,7 @@ func recycleIPTables(nw ip.IP4Net, lease *subnet.Lease) error {
lease := &subnet.Lease{
Subnet: prevSubnet,
}
if err := network.DeleteIPTables(network.MasqRules(prevNetwork, lease)); err != nil {
if err := network.DeleteIP4Tables(network.MasqRules(prevNetwork, lease)); err != nil {
return err
}
}
Expand Down
156 changes: 114 additions & 42 deletions network/iptables.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package network

import (
"fmt"
"strings"
"time"

"github.com/coreos/go-iptables/iptables"
Expand Down Expand Up @@ -135,24 +134,81 @@ func ipTablesRulesExist(ipt IPTables, rules []IPTablesRule) (bool, error) {
return true, nil
}

func SetupAndEnsureIPTables(rules []IPTablesRule, resyncPeriod int) {
// ipTablesCleanAndBuild create from a list of iptables rules a transaction (as string) for iptables-restore for ordering the rules that effectively running
func ipTablesCleanAndBuild(ipt IPTables, rules []IPTablesRule) (IPTablesRestoreRules, error) {
tablesRules := IPTablesRestoreRules{}

// Build append and delete rules
for _, rule := range rules {
exists, err := ipt.Exists(rule.table, rule.chain, rule.rulespec...)
if err != nil {
// this shouldn't ever happen
return nil, fmt.Errorf("failed to check rule existence: %v", err)
}
if exists {
if _, ok := tablesRules[rule.table]; !ok {
tablesRules[rule.table] = []IPTablesRestoreRuleSpec{}
}
// if the rule exists it's safer to delete it and then create them
tablesRules[rule.table] = append(tablesRules[rule.table], append(IPTablesRestoreRuleSpec{"-D", rule.chain}, rule.rulespec...))
}
// with iptables-restore we can ensure that all rules created are in good order and have no external rule between them
tablesRules[rule.table] = append(tablesRules[rule.table], append(IPTablesRestoreRuleSpec{"-A", rule.chain}, rule.rulespec...))
}

return tablesRules, nil
}

// ipTablesBootstrap init iptables rules using iptables-restore (with some cleaning if some rules already exists)
func ipTablesBootstrap(ipt IPTables, iptRestore IPTablesRestore, rules []IPTablesRule) error {
tablesRules, err := ipTablesCleanAndBuild(ipt, rules)
if err != nil {
// if we can't find iptables or if we can check existing rules, give up and return
return fmt.Errorf("failed to setup iptables-restore payload: %v", err)
}

log.V(6).Infof("trying to run iptables-restore < %+v", tablesRules)

err = iptRestore.ApplyWithoutFlush(tablesRules)
if err != nil {
return fmt.Errorf("failed to apply partial iptables-restore %v", err)
}

log.Infof("bootstrap done")

return nil
}

func SetupAndEnsureIP4Tables(rules []IPTablesRule, resyncPeriod int) {
ipt, err := iptables.New()
if err != nil {
// if we can't find iptables, give up and return
log.Errorf("Failed to setup IPTables. iptables binary was not found: %v", err)
return
}
iptRestore, err := NewIPTablesRestoreWithProtocol(iptables.ProtocolIPv4)
if err != nil {
// if we can't find iptables-restore, give up and return
log.Errorf("Failed to setup IPTables. iptables-restore binary was not found: %v", err)
return
}

err = ipTablesBootstrap(ipt, iptRestore, rules)
if err != nil {
// if we can't find iptables, give up and return
log.Errorf("Failed to bootstrap IPTables: %v", err)
}

defer func() {
err := teardownIPTables(ipt, rules)
err := teardownIPTables(ipt, iptRestore, rules)
if err != nil {
log.Errorf("Failed to tear down IPTables: %v", err)
}
}()

for {
// Ensure that all the iptables rules exist every 5 seconds
if err := ensureIPTables(ipt, rules); err != nil {
if err := ensureIPTables(ipt, iptRestore, rules); err != nil {
log.Errorf("Failed to ensure iptables rules: %v", err)
}

Expand All @@ -167,35 +223,53 @@ func SetupAndEnsureIP6Tables(rules []IPTablesRule, resyncPeriod int) {
log.Errorf("Failed to setup IP6Tables. iptables binary was not found: %v", err)
return
}
iptRestore, err := NewIPTablesRestoreWithProtocol(iptables.ProtocolIPv6)
if err != nil {
// if we can't find iptables, give up and return
log.Errorf("Failed to setup iptables-restore: %v", err)
return
}

err = ipTablesBootstrap(ipt, iptRestore, rules)
if err != nil {
// if we can't find iptables, give up and return
log.Errorf("Failed to bootstrap IPTables: %v", err)
}

defer func() {
err := teardownIPTables(ipt, rules)
err := teardownIPTables(ipt, iptRestore, rules)
if err != nil {
log.Errorf("Failed to tear down IPTables: %v", err)
}
}()

for {
// Ensure that all the iptables rules exist every 5 seconds
if err := ensureIPTables(ipt, rules); err != nil {
if err := ensureIPTables(ipt, iptRestore, rules); err != nil {
log.Errorf("Failed to ensure iptables rules: %v", err)
}

time.Sleep(time.Duration(resyncPeriod) * time.Second)
}
}

// DeleteIPTables delete specified iptables rules
func DeleteIPTables(rules []IPTablesRule) error {
// DeleteIP4Tables delete specified iptables rules
func DeleteIP4Tables(rules []IPTablesRule) error {
ipt, err := iptables.New()
if err != nil {
// if we can't find iptables, give up and return
log.Errorf("Failed to setup IPTables. iptables binary was not found: %v", err)
return err
}
err = teardownIPTables(ipt, rules)
iptRestore, err := NewIPTablesRestoreWithProtocol(iptables.ProtocolIPv4)
if err != nil {
// if we can't find iptables, give up and return
log.Errorf("Failed to setup iptables-restore: %v", err)
return err
}
err = teardownIPTables(ipt, iptRestore, rules)
if err != nil {
log.Errorf("Failed to tear down IPTables: %v", err)
log.Errorf("Failed to teardown iptables: %v", err)
return err
}
return nil
Expand All @@ -209,18 +283,25 @@ func DeleteIP6Tables(rules []IPTablesRule) error {
log.Errorf("Failed to setup IP6Tables. iptables binary was not found: %v", err)
return err
}
err = teardownIPTables(ipt, rules)

iptRestore, err := NewIPTablesRestoreWithProtocol(iptables.ProtocolIPv6)
if err != nil {
// if we can't find iptables, give up and return
log.Errorf("Failed to setup iptables-restore: %v", err)
return err
}
err = teardownIPTables(ipt, iptRestore, rules)
if err != nil {
log.Errorf("Failed to tear down IPTables: %v", err)
log.Errorf("Failed to teardown iptables: %v", err)
return err
}
return nil
}

func ensureIPTables(ipt IPTables, rules []IPTablesRule) error {
func ensureIPTables(ipt IPTables, iptRestore IPTablesRestore, rules []IPTablesRule) error {
exists, err := ipTablesRulesExist(ipt, rules)
if err != nil {
return fmt.Errorf("Error checking rule existence: %v", err)
return fmt.Errorf("error checking rule existence: %v", err)
}
if exists {
// if all the rules already exist, no need to do anything
Expand All @@ -229,44 +310,35 @@ func ensureIPTables(ipt IPTables, rules []IPTablesRule) error {
// Otherwise, teardown all the rules and set them up again
// We do this because the order of the rules is important
log.Info("Some iptables rules are missing; deleting and recreating rules")
if err = teardownIPTables(ipt, rules); err != nil {
return fmt.Errorf("Error tearing down rules: %v", err)
}
if err = setupIPTables(ipt, rules); err != nil {
return fmt.Errorf("Error setting up rules: %v", err)
err = ipTablesBootstrap(ipt, iptRestore, rules)
if err != nil {
// if we can't find iptables, give up and return
return fmt.Errorf("error setting up rules: %v", err)
}
return nil
}

func setupIPTables(ipt IPTables, rules []IPTablesRule) error {
for _, rule := range rules {
log.Info("Adding iptables rule: ", strings.Join(rule.rulespec, " "))
err := ipt.AppendUnique(rule.table, rule.chain, rule.rulespec...)
if err != nil {
return fmt.Errorf("failed to insert IPTables rule: %v", err)
}
}
func teardownIPTables(ipt IPTables, iptr IPTablesRestore, rules []IPTablesRule) error {
tablesRules := IPTablesRestoreRules{}

return nil
}

func teardownIPTables(ipt IPTables, rules []IPTablesRule) error {
// Build delete rules to a transaction for iptables restore
for _, rule := range rules {
log.Info("Deleting iptables rule: ", strings.Join(rule.rulespec, " "))
err := ipt.Delete(rule.table, rule.chain, rule.rulespec...)
exists, err := ipt.Exists(rule.table, rule.chain, rule.rulespec...)
if err != nil {
e := err.(IPTablesError)
// If this error is because the rule is already deleted, the message from iptables will be
// "Bad rule (does a matching rule exist in that chain?)". These are safe to ignore.
// However other errors (like EAGAIN caused by other things not respecting the xtables.lock)
// should halt the ensure process. Otherwise rules can get out of order when a rule we think
// is deleted is actually still in the chain.
// This will leave the rules incomplete until the next successful reconciliation loop.
if !e.IsNotExist() {
return err
// this shouldn't ever happen
return fmt.Errorf("failed to check rule existence: %v", err)
}
if exists {
if _, ok := tablesRules[rule.table]; !ok {
tablesRules[rule.table] = []IPTablesRestoreRuleSpec{}
}
tablesRules[rule.table] = append(tablesRules[rule.table], append(IPTablesRestoreRuleSpec{"-D", rule.chain}, rule.rulespec...))
}
}
err := iptr.ApplyWithoutFlush(tablesRules) // ApplyWithoutFlush make a diff, Apply make a replace (desired state)
if err != nil {
return fmt.Errorf("unable to teardown iptables: %v", err)
}

return nil
}
Loading

0 comments on commit 121db21

Please sign in to comment.