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
  • Loading branch information
@louiznk
louiznk committed May 31, 2022
1 parent a1a24eb commit 04d157d
Show file tree
Hide file tree
Showing 5 changed files with 673 additions and 120 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
146 changes: 107 additions & 39 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,77 @@ func ipTablesRulesExist(ipt IPTables, rules []IPTablesRule) (bool, error) {
return true, nil
}

func ipTablesCleanAndBuild(ipt IPTables, rules []IPTablesRule) (IPTablesRestoreRules, error) {
tablesRules := IPTablesRestoreRules{}

// Build 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{}
}
tablesRules[rule.table] = append(tablesRules[rule.table], append(IPTablesRestoreRuleSpec{"-D", rule.chain}, rule.rulespec...))
}
tablesRules[rule.table] = append(tablesRules[rule.table], append(IPTablesRestoreRuleSpec{"-A", rule.chain}, rule.rulespec...))
}

return tablesRules, nil
}

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 SetupAndEnsureIPTables(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 := NewIPTablesRestore()
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,17 +219,29 @@ func SetupAndEnsureIP6Tables(rules []IPTablesRule, resyncPeriod int) {
log.Errorf("Failed to setup IP6Tables. iptables binary was not found: %v", err)
return
}
iptRestore, err := NewIPTablesRestore()
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)
}

Expand All @@ -193,9 +257,15 @@ func DeleteIPTables(rules []IPTablesRule) error {
log.Errorf("Failed to setup IPTables. iptables binary was not found: %v", err)
return err
}
err = teardownIPTables(ipt, rules)
iptRestore, err := NewIPTablesRestore()
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 +279,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 +306,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 04d157d

Please sign in to comment.