Store iptables when creating network attack (#215)

* store iptables Signed-off-by: root <[email protected]> * make check Signed-off-by: root <[email protected]> * update network test Signed-off-by: root <[email protected]> * move NeedApplyTC() at the begin of applying to the middle Signed-off-by: root <[email protected]> * remove NeedApplyIptables Signed-off-by: root <[email protected]> Signed-off-by: root <[email protected]> Co-authored-by: root <[email protected]>
chaos-mesh · Sep 14, 2022 · 98f3ca4 · 98f3ca4
1 parent 5ecc6c2
commit 98f3ca4
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 54 deletions.
diff --git a/pkg/core/network.go b/pkg/core/network.go
@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/chaos-mesh/chaos-mesh/api/v1alpha1"
+	"github.com/chaos-mesh/chaos-mesh/controllers/podnetworkchaos/netutils"
 	"github.com/chaos-mesh/chaos-mesh/pkg/chaosdaemon/pb"
 	"github.com/chaos-mesh/chaos-mesh/pkg/netem"
 	"github.com/pingcap/errors"
@@ -510,10 +511,6 @@ func (n *NetworkCommand) NeedApplyIPSet() bool {
 	return false
 }
 
-func (n *NetworkCommand) NeedApplyIptables() bool {
-	return true
-}
-
 func (n *NetworkCommand) NeedApplyTC() bool {
 	switch n.Action {
 	case NetworkDelayAction, NetworkLossAction, NetworkCorruptAction, NetworkDuplicateAction, NetworkBandwidthAction:
@@ -523,20 +520,20 @@ func (n *NetworkCommand) NeedApplyTC() bool {
 	}
 }
 
-func (n *NetworkCommand) AdditionalChain(ipset string) ([]*pb.Chain, error) {
+func (n *NetworkCommand) AdditionalChain(ipset string, uid string) ([]*pb.Chain, error) {
 	chains := make([]*pb.Chain, 0, 2)
 	var toChains, fromChains []*pb.Chain
 	var err error
 
 	if n.Direction == "to" || n.Direction == "both" {
-		toChains, err = n.getAdditionalChain(ipset, "to")
+		toChains, err = n.getAdditionalChain(ipset, "to", uid)
 		if err != nil {
 			return nil, err
 		}
 	}
 
 	if n.Direction == "from" || n.Direction == "both" {
-		fromChains, err = n.getAdditionalChain(ipset, "from")
+		fromChains, err = n.getAdditionalChain(ipset, "from", uid)
 		if err != nil {
 			return nil, err
 		}
@@ -548,7 +545,7 @@ func (n *NetworkCommand) AdditionalChain(ipset string) ([]*pb.Chain, error) {
 	return chains, nil
 }
 
-func (n *NetworkCommand) getAdditionalChain(ipset, direction string) ([]*pb.Chain, error) {
+func (n *NetworkCommand) getAdditionalChain(ipset, direction string, uid string) ([]*pb.Chain, error) {
 	var directionStr string
 	var directionChain pb.Chain_Direction
 	if direction == "to" {
@@ -562,9 +559,11 @@ func (n *NetworkCommand) getAdditionalChain(ipset, direction string) ([]*pb.Chai
 	}
 
 	chains := make([]*pb.Chain, 0, 2)
+	// The `targetLength`s in `netutils.CompressName()` are different because of
+	// the need to distinguish between the different chains.
 	if len(n.AcceptTCPFlags) > 0 {
 		chains = append(chains, &pb.Chain{
-			Name:      fmt.Sprintf("%s/0", directionStr),
+			Name:      fmt.Sprintf("%s/%s", directionStr, netutils.CompressName(uid, 19, "")),
 			Ipsets:    []string{ipset},
 			Direction: directionChain,
 			Protocol:  n.IPProtocol,
@@ -575,7 +574,7 @@ func (n *NetworkCommand) getAdditionalChain(ipset, direction string) ([]*pb.Chai
 
 	if n.Action == NetworkPartitionAction {
 		chains = append(chains, &pb.Chain{
-			Name:      fmt.Sprintf("%s/1", directionStr),
+			Name:      fmt.Sprintf("%s/%s", directionStr, netutils.CompressName(uid, 20, "")),
 			Ipsets:    []string{ipset},
 			Direction: directionChain,
 			Protocol:  n.IPProtocol,

diff --git a/pkg/core/network_test.go b/pkg/core/network_test.go
@@ -35,7 +35,7 @@ func TestPatitionChain(t *testing.T) {
 				},
 				chains: []*pb.Chain{
 					{
-						Name:      "OUTPUT/1",
+						Name:      "OUTPUT/3c552_e0172bc4fd046_",
 						Ipsets:    []string{"test"},
 						Direction: pb.Chain_OUTPUT,
 						Protocol:  "tcp",
@@ -53,7 +53,7 @@ func TestPatitionChain(t *testing.T) {
 				},
 				chains: []*pb.Chain{
 					{
-						Name:      "INPUT/1",
+						Name:      "INPUT/3c552_e0172bc4fd046_",
 						Ipsets:    []string{"test"},
 						Direction: pb.Chain_INPUT,
 						Protocol:  "tcp",
@@ -71,14 +71,14 @@ func TestPatitionChain(t *testing.T) {
 				},
 				chains: []*pb.Chain{
 					{
-						Name:      "OUTPUT/1",
+						Name:      "OUTPUT/3c552_e0172bc4fd046_",
 						Ipsets:    []string{"test"},
 						Direction: pb.Chain_OUTPUT,
 						Protocol:  "tcp",
 						Target:    "DROP",
 					},
 					{
-						Name:      "INPUT/1",
+						Name:      "INPUT/3c552_e0172bc4fd046_",
 						Ipsets:    []string{"test"},
 						Direction: pb.Chain_INPUT,
 						Protocol:  "tcp",
@@ -97,30 +97,30 @@ func TestPatitionChain(t *testing.T) {
 				},
 				chains: []*pb.Chain{
 					{
-						Name:      "OUTPUT/0",
+						Name:      "OUTPUT/3c552_e0172bc4fd04_",
 						Ipsets:    []string{"test"},
 						Direction: pb.Chain_OUTPUT,
 						Protocol:  "tcp",
 						TcpFlags:  "SYN,ACK SYN,ACK",
 						Target:    "ACCEPT",
 					},
 					{
-						Name:      "OUTPUT/1",
+						Name:      "OUTPUT/3c552_e0172bc4fd046_",
 						Ipsets:    []string{"test"},
 						Direction: pb.Chain_OUTPUT,
 						Protocol:  "tcp",
 						Target:    "DROP",
 					},
 					{
-						Name:      "INPUT/0",
+						Name:      "INPUT/3c552_e0172bc4fd04_",
 						Ipsets:    []string{"test"},
 						Direction: pb.Chain_INPUT,
 						Protocol:  "tcp",
 						TcpFlags:  "SYN,ACK SYN,ACK",
 						Target:    "ACCEPT",
 					},
 					{
-						Name:      "INPUT/1",
+						Name:      "INPUT/3c552_e0172bc4fd046_",
 						Ipsets:    []string{"test"},
 						Direction: pb.Chain_INPUT,
 						Protocol:  "tcp",
@@ -130,7 +130,7 @@ func TestPatitionChain(t *testing.T) {
 			},
 		}
 		for _, tc := range testCases {
-			chains, err := tc.cmd.AdditionalChain("test")
+			chains, err := tc.cmd.AdditionalChain("test", "3c5528e1-4c32-4f80-983c-913ad7e860e2")
 			if err != nil {
 				t.Errorf("failed to partition chain: %v", err)
 			}

diff --git a/pkg/server/chaosd/network.go b/pkg/server/chaosd/network.go
@@ -74,16 +74,13 @@ func (networkAttack) Attack(options core.AttackConfig, env Environment) (err err
 			}
 		}
 
-		if attack.NeedApplyIptables() {
-			if err = env.Chaos.applyIptables(attack, ipsetName, env.AttackUid); err != nil {
-				return perrors.WithStack(err)
-			}
+		if err = env.Chaos.applyIptables(attack, ipsetName, env.AttackUid); err != nil {
+			return perrors.WithStack(err)
 		}
 
-		if attack.NeedApplyTC() {
-			if err = env.Chaos.applyTC(attack, ipsetName, env.AttackUid); err != nil {
-				return perrors.WithStack(err)
-			}
+		// Because some tcs add filter iptables which will not be stored in the DB, we must re-apply these tcs to add the iptables.
+		if err = env.Chaos.applyTC(attack, ipsetName, env.AttackUid); err != nil {
+			return perrors.WithStack(err)
 		}
 
 	case core.NetworkNICDownAction:
@@ -140,9 +137,11 @@ func (s *Server) applyIptables(attack *core.NetworkCommand, ipset, uid string) e
 		return perrors.WithStack(err)
 	}
 	chains := core.IptablesRuleList(iptables).ToChains()
+
+	var newChains []*pb.Chain
 	// Presently, only partition and delay with `accept-tcp-flags` need to add additional chains
 	if attack.NeedAdditionalChains() {
-		newChains, err := attack.AdditionalChain(ipset)
+		newChains, err = attack.AdditionalChain(ipset, uid)
 		if err != nil {
 			return perrors.WithStack(err)
 		}
@@ -156,15 +155,17 @@ func (s *Server) applyIptables(attack *core.NetworkCommand, ipset, uid string) e
 		return perrors.WithStack(err)
 	}
 
-	// TODO: cwen0
-	//if err := s.iptablesRule.Set(context.Background(), &core.IptablesRule{
-	//	Name:       newChain.Name,
-	//	IPSets:     strings.Join(newChain.Ipsets, ","),
-	//	Direction:  pb.Chain_Direction_name[int32(newChain.Direction)],
-	//	Experiment: uid,
-	//}); err != nil {
-	//	return perrors.WithStack(err)
-	//}
+	for _, newChain := range newChains {
+		if err := s.iptablesRule.Set(context.Background(), &core.IptablesRule{
+			Name:       newChain.Name,
+			IPSets:     strings.Join(newChain.Ipsets, ","),
+			Direction:  pb.Chain_Direction_name[int32(newChain.Direction)],
+			Protocol:   newChain.Protocol,
+			Experiment: uid,
+		}); err != nil {
+			return perrors.WithStack(err)
+		}
+	}
 
 	return nil
 }
@@ -180,17 +181,24 @@ func (s *Server) applyTC(attack *core.NetworkCommand, ipset string, uid string)
 		return perrors.WithStack(err)
 	}
 
-	newTC, err := attack.ToTC(ipset)
-	if err != nil {
-		return perrors.WithStack(err)
-	}
+	var newTC *pb.Tc
+	if attack.NeedApplyTC() {
+		newTC, err = attack.ToTC(ipset)
+		if err != nil {
+			return perrors.WithStack(err)
+		}
 
-	tcs = append(tcs, newTC)
+		tcs = append(tcs, newTC)
+	}
 
 	if _, err := s.svr.SetTcs(context.Background(), &pb.TcsRequest{Tcs: tcs, EnterNS: false}); err != nil {
 		return perrors.WithStack(err)
 	}
 
+	if !attack.NeedApplyTC() {
+		return nil
+	}
+
 	tc := &core.TcParameter{
 		Device: attack.Device,
 	}
@@ -380,22 +388,16 @@ func (networkAttack) Recover(exp core.Experiment, env Environment) error {
 	case core.NetworkPortOccupiedAction:
 		return env.Chaos.recoverPortOccupied(attack, env.AttackUid)
 	case core.NetworkDelayAction, core.NetworkLossAction, core.NetworkCorruptAction, core.NetworkDuplicateAction, core.NetworkPartitionAction, core.NetworkBandwidthAction:
-		if attack.NeedApplyIPSet() {
-			if err := env.Chaos.recoverIPSet(env.AttackUid); err != nil {
-				return perrors.WithStack(err)
-			}
+		if err := env.Chaos.recoverIPSet(env.AttackUid); err != nil {
+			return perrors.WithStack(err)
 		}
 
-		if attack.NeedApplyIptables() {
-			if err := env.Chaos.recoverIptables(env.AttackUid); err != nil {
-				return perrors.WithStack(err)
-			}
+		if err := env.Chaos.recoverIptables(env.AttackUid); err != nil {
+			return perrors.WithStack(err)
 		}
 
-		if attack.NeedApplyTC() {
-			if err := env.Chaos.recoverTC(env.AttackUid, attack.Device); err != nil {
-				return perrors.WithStack(err)
-			}
+		if err := env.Chaos.recoverTC(env.AttackUid, attack.Device); err != nil {
+			return perrors.WithStack(err)
 		}
 	case core.NetworkNICDownAction:
 		return env.Chaos.recoverNICDown(attack)