-
Notifications
You must be signed in to change notification settings - Fork 63
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
Store iptables when creating network attack #215
Changes from 6 commits
f4e1f42
8db22b3
8b3c320
15a185a
83eccfa
a0efa92
2d423a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,11 +79,9 @@ func (networkAttack) Attack(options core.AttackConfig, env Environment) (err err | |
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 +138,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 +156,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 +182,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 +389,20 @@ 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) | ||
} | ||
// `chaosdaemon.DeamonServer.SetTcs()` may build new iptables which will not be recorded in DB, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should update this comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this comment should be updated, the filter iptables still not be stored in the DB There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but here we recover all because chaos mesh will apply all network actions when we apply a new one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree |
||
// and network partition is not suppose to build iptables directly, `recoverIptables()` will not | ||
// be called when recovering a partition experiment. To avoid other cross-build situations, all these | ||
// three functions will be called. | ||
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) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can delete
NeedApplyIptables()
andNeedApplyIPSet
directly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think leaving them can make the code logic clearly, and they may be needed to add features later (by the way,
NeedApplyIPSet()
is working now)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the logic will be clearer If we delete them. The logic: Chaos Mesh will reset all network action when you apply the network actions. This logic will be more simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I think it's OK to be the same as chaos mesh,
NeedApplyIptables
can be deleted, but I findchaosd attack network delay -l 1s -d eth0
will not work withoutNeedApplyIPSet()