Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 3 additions & 12 deletions pkg/chaos/iptables/loss.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,16 @@ func (n *lossCommand) Run(ctx context.Context, random bool) error {
// run iptables loss command for selected containers
var wg sync.WaitGroup
errs := make([]error, len(containers))
cancels := make([]context.CancelFunc, len(containers))
for i, c := range containers {
log.WithFields(log.Fields{
"container": *c,
}).Debug("adding network random packet loss for container")
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, n.duration)
cancels[i] = cancel
iptCtx, cancel := context.WithTimeout(ctx, n.duration)
wg.Add(1)
go func(i int, c *container.Container) {
defer wg.Done()
errs[i] = runIPTables(ctx, n.client, c, addCmdPrefix, delCmdPrefix, cmdSuffix, n.srcIPs, n.dstIPs, n.sports, n.dports, n.duration, n.image, n.pull, n.dryRun)
defer cancel()
Comment on lines +122 to +126
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is incorrect. All these files use := (short variable declaration), not = (assignment). In Go, := inside a for loop body creates new block-scoped variables per iteration — each goroutine closure captures its own distinct netemCtx/cancel (or iptCtx/cancel). There is no sharing across iterations.

The old iptables/loss.go code did have this bug (it used ctx, cancel = ... with =), which this PR fixes by switching to iptCtx, cancel := ... with :=.

See: https://go.dev/doc/faq#closures_and_goroutines

errs[i] = runIPTables(iptCtx, n.client, c, addCmdPrefix, delCmdPrefix, cmdSuffix, n.srcIPs, n.dstIPs, n.sports, n.dports, n.duration, n.image, n.pull, n.dryRun)
if errs[i] != nil {
log.WithError(errs[i]).Warn("failed to set packet loss for container")
}
Expand All @@ -136,13 +134,6 @@ func (n *lossCommand) Run(ctx context.Context, random bool) error {
// Wait for all iptables delay commands to complete
wg.Wait()

// cancel context to avoid leaks
defer func() {
for _, cancel := range cancels {
cancel()
}
}()

// scan through all errors in goroutines
for _, err = range errs {
// take first found error
Expand Down
10 changes: 1 addition & 9 deletions pkg/chaos/netem/corrupt.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,17 @@ func (n *corruptCommand) Run(ctx context.Context, random bool) error {
// run netem corrupt command for selected containers
var wg sync.WaitGroup
errs := make([]error, len(containers))
cancels := make([]context.CancelFunc, len(containers))

//nolint:dupl
for i, c := range containers {
log.WithFields(log.Fields{
"container": c,
}).Debug("adding network random packet corrupt for container")
netemCtx, cancel := context.WithTimeout(ctx, n.duration)
cancels[i] = cancel
wg.Add(1)
go func(i int, c *container.Container) {
defer wg.Done()
defer cancel()
Comment on lines 85 to +89
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is incorrect. All these files use := (short variable declaration), not = (assignment). In Go, := inside a for loop body creates new block-scoped variables per iteration — each goroutine closure captures its own distinct netemCtx/cancel (or iptCtx/cancel). There is no sharing across iterations.

The old iptables/loss.go code did have this bug (it used ctx, cancel = ... with =), which this PR fixes by switching to iptCtx, cancel := ... with :=.

See: https://go.dev/doc/faq#closures_and_goroutines

errs[i] = runNetem(netemCtx, n.client, c, n.iface, netemCmd, n.ips, n.sports, n.dports, n.duration, n.image, n.pull, n.dryRun)
if errs[i] != nil {
log.WithError(errs[i]).Warn("failed to set packet corrupt for container")
Expand All @@ -98,13 +97,6 @@ func (n *corruptCommand) Run(ctx context.Context, random bool) error {
// Wait for all netem commands to complete
wg.Wait()

// cancel context to avoid leaks
defer func() {
for _, cancel := range cancels {
cancel()
}
}()

// scan through all errors in goroutines
for _, err = range errs {
// take first reported error
Expand Down
10 changes: 1 addition & 9 deletions pkg/chaos/netem/delay.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,15 @@ func (n *delayCommand) Run(ctx context.Context, random bool) error {
// run netem delay command for selected containers
var wg sync.WaitGroup
errs := make([]error, len(containers))
cancels := make([]context.CancelFunc, len(containers))
for i, c := range containers {
log.WithFields(log.Fields{
"container": c,
}).Debug("adding network delay for container")
netemCtx, cancel := context.WithCancel(ctx)
cancels[i] = cancel
wg.Add(1)
go func(i int, c *container.Container) {
defer wg.Done()
defer cancel()
Comment on lines 109 to +113
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is incorrect. All these files use := (short variable declaration), not = (assignment). In Go, := inside a for loop body creates new block-scoped variables per iteration — each goroutine closure captures its own distinct netemCtx/cancel (or iptCtx/cancel). There is no sharing across iterations.

The old iptables/loss.go code did have this bug (it used ctx, cancel = ... with =), which this PR fixes by switching to iptCtx, cancel := ... with :=.

See: https://go.dev/doc/faq#closures_and_goroutines

errs[i] = runNetem(netemCtx, n.client, c, n.iface, netemCmd, n.ips, n.sports, n.dports, n.duration, n.image, n.pull, n.dryRun)
if errs[i] != nil {
log.WithError(errs[i]).Warn("failed to delay network for container")
Expand All @@ -122,13 +121,6 @@ func (n *delayCommand) Run(ctx context.Context, random bool) error {
// Wait for all netem delay commands to complete
wg.Wait()

// cancel context to avoid leaks
defer func() {
for _, cancel := range cancels {
cancel()
}
}()

// scan through all errors in goroutines
for _, err := range errs {
// take first found error
Expand Down
10 changes: 1 addition & 9 deletions pkg/chaos/netem/duplicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,17 @@ func (n *duplicateCommand) Run(ctx context.Context, random bool) error {
// run netem duplicate command for selected containers
var wg sync.WaitGroup
errs := make([]error, len(containers))
cancels := make([]context.CancelFunc, len(containers))

//nolint:dupl
for i, c := range containers {
log.WithFields(log.Fields{
"container": c,
}).Debug("adding network random packet duplicates for container")
netemCtx, cancel := context.WithTimeout(ctx, n.duration)
cancels[i] = cancel
wg.Add(1)
go func(i int, c *container.Container) {
defer wg.Done()
defer cancel()
Comment on lines 89 to +93
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is incorrect. All these files use := (short variable declaration), not = (assignment). In Go, := inside a for loop body creates new block-scoped variables per iteration — each goroutine closure captures its own distinct netemCtx/cancel (or iptCtx/cancel). There is no sharing across iterations.

The old iptables/loss.go code did have this bug (it used ctx, cancel = ... with =), which this PR fixes by switching to iptCtx, cancel := ... with :=.

See: https://go.dev/doc/faq#closures_and_goroutines

errs[i] = runNetem(netemCtx, n.client, c, n.iface, netemCmd, n.ips, n.sports, n.dports, n.duration, n.image, n.pull, n.dryRun)
if errs[i] != nil {
log.WithError(errs[i]).Warn("failed to set packet duplicates for container")
Expand All @@ -102,13 +101,6 @@ func (n *duplicateCommand) Run(ctx context.Context, random bool) error {
// Wait for all netem commands to complete
wg.Wait()

// cancel context to avoid leaks
defer func() {
for _, cancel := range cancels {
cancel()
}
}()

// scan through all errors in goroutines
for _, err = range errs {
// take first found error
Expand Down
10 changes: 1 addition & 9 deletions pkg/chaos/netem/loss.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,15 @@ func (n *lossCommand) Run(ctx context.Context, random bool) error {
// run netem loss command for selected containers
var wg sync.WaitGroup
errs := make([]error, len(containers))
cancels := make([]context.CancelFunc, len(containers))
for i, c := range containers {
log.WithFields(log.Fields{
"container": *c,
}).Debug("adding network random packet loss for container")
netemCtx, cancel := context.WithTimeout(ctx, n.duration)
cancels[i] = cancel
wg.Add(1)
go func(i int, c *container.Container) {
defer wg.Done()
defer cancel()
Comment on lines 84 to +88
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is incorrect. All these files use := (short variable declaration), not = (assignment). In Go, := inside a for loop body creates new block-scoped variables per iteration — each goroutine closure captures its own distinct netemCtx/cancel (or iptCtx/cancel). There is no sharing across iterations.

The old iptables/loss.go code did have this bug (it used ctx, cancel = ... with =), which this PR fixes by switching to iptCtx, cancel := ... with :=.

See: https://go.dev/doc/faq#closures_and_goroutines

errs[i] = runNetem(netemCtx, n.client, c, n.iface, netemCmd, n.ips, n.sports, n.dports, n.duration, n.image, n.pull, n.dryRun)
if errs[i] != nil {
log.WithError(errs[i]).Warn("failed to set packet loss for container")
Expand All @@ -97,13 +96,6 @@ func (n *lossCommand) Run(ctx context.Context, random bool) error {
// Wait for all netem delay commands to complete
wg.Wait()

// cancel context to avoid leaks
defer func() {
for _, cancel := range cancels {
cancel()
}
}()

// scan through all errors in goroutines
for _, err = range errs {
// take first found error
Expand Down
10 changes: 1 addition & 9 deletions pkg/chaos/netem/loss_ge.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,17 @@ func (n *lossGECommand) Run(ctx context.Context, random bool) error {
// run netem loss command for selected containers
var wg sync.WaitGroup
errs := make([]error, len(containers))
cancels := make([]context.CancelFunc, len(containers))

//nolint:dupl
for i, c := range containers {
log.WithFields(log.Fields{
"container": c,
}).Debug("adding network random packet loss for container")
netemCtx, cancel := context.WithTimeout(ctx, n.duration)
cancels[i] = cancel
wg.Add(1)
go func(i int, c *container.Container) {
defer wg.Done()
defer cancel()
Comment on lines 101 to +105
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is incorrect. All these files use := (short variable declaration), not = (assignment). In Go, := inside a for loop body creates new block-scoped variables per iteration — each goroutine closure captures its own distinct netemCtx/cancel (or iptCtx/cancel). There is no sharing across iterations.

The old iptables/loss.go code did have this bug (it used ctx, cancel = ... with =), which this PR fixes by switching to iptCtx, cancel := ... with :=.

See: https://go.dev/doc/faq#closures_and_goroutines

errs[i] = runNetem(netemCtx, n.client, c, n.iface, netemCmd, n.ips, n.sports, n.dports, n.duration, n.image, n.pull, n.dryRun)
if errs[i] != nil {
log.WithError(errs[i]).Warn("failed to set packet loss for container")
Expand All @@ -114,13 +113,6 @@ func (n *lossGECommand) Run(ctx context.Context, random bool) error {
// Wait for all netem delay commands to complete
wg.Wait()

// cancel context to avoid leaks
defer func() {
for _, cancel := range cancels {
cancel()
}
}()

// scan through all errors in goroutines
for _, err := range errs {
// take first found error
Expand Down
10 changes: 1 addition & 9 deletions pkg/chaos/netem/loss_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,18 +100,17 @@ func (n *lossStateCommand) Run(ctx context.Context, random bool) error {
// run netem loss command for selected containers
var wg sync.WaitGroup
errs := make([]error, len(containers))
cancels := make([]context.CancelFunc, len(containers))

//nolint:dupl
for i, c := range containers {
log.WithFields(log.Fields{
"container": c,
}).Debug("adding network 4-state packet loss for container")
netemCtx, cancel := context.WithTimeout(ctx, n.duration)
cancels[i] = cancel
wg.Add(1)
go func(i int, c *container.Container) {
defer wg.Done()
defer cancel()
Comment on lines 109 to +113
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is incorrect. All these files use := (short variable declaration), not = (assignment). In Go, := inside a for loop body creates new block-scoped variables per iteration — each goroutine closure captures its own distinct netemCtx/cancel (or iptCtx/cancel). There is no sharing across iterations.

The old iptables/loss.go code did have this bug (it used ctx, cancel = ... with =), which this PR fixes by switching to iptCtx, cancel := ... with :=.

See: https://go.dev/doc/faq#closures_and_goroutines

errs[i] = runNetem(netemCtx, n.client, c, n.iface, netemCmd, n.ips, n.sports, n.dports, n.duration, n.image, n.pull, n.dryRun)
if errs[i] != nil {
log.WithError(errs[i]).Warn("failed to set packet loss for container")
Expand All @@ -122,13 +121,6 @@ func (n *lossStateCommand) Run(ctx context.Context, random bool) error {
// Wait for all netem delay commands to complete
wg.Wait()

// cancel context to avoid leaks
defer func() {
for _, cancel := range cancels {
cancel()
}
}()

// scan through all errors in goroutines
for _, err = range errs {
// take first found error
Expand Down
10 changes: 1 addition & 9 deletions pkg/chaos/netem/rate.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,16 @@ func (n *rateCommand) Run(ctx context.Context, random bool) error {
// run netem loss command for selected containers
var wg sync.WaitGroup
errs := make([]error, len(containers))
cancels := make([]context.CancelFunc, len(containers))
for i, c := range containers {
log.WithFields(log.Fields{
"container": c,
"command": netemCmd,
}).Debug("setting network rate for container")
netemCtx, cancel := context.WithTimeout(ctx, n.duration)
cancels[i] = cancel
wg.Add(1)
go func(i int, c *container.Container) {
defer wg.Done()
defer cancel()
Comment on lines 113 to +117
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is incorrect. All these files use := (short variable declaration), not = (assignment). In Go, := inside a for loop body creates new block-scoped variables per iteration — each goroutine closure captures its own distinct netemCtx/cancel (or iptCtx/cancel). There is no sharing across iterations.

The old iptables/loss.go code did have this bug (it used ctx, cancel = ... with =), which this PR fixes by switching to iptCtx, cancel := ... with :=.

See: https://go.dev/doc/faq#closures_and_goroutines

errs[i] = runNetem(netemCtx, n.client, c, n.iface, netemCmd, n.ips, n.sports, n.dports, n.duration, n.image, n.pull, n.dryRun)
if errs[i] != nil {
log.WithError(errs[i]).Warn("failed to set network rate for container")
Expand All @@ -126,13 +125,6 @@ func (n *rateCommand) Run(ctx context.Context, random bool) error {
// Wait for all netem delay commands to complete
wg.Wait()

// cancel context to avoid leaks
defer func() {
for _, cancel := range cancels {
cancel()
}
}()

// scan through all errors in goroutines
for _, err = range errs {
// take first found error
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/util.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package util //nolint:revive // existing package name, renaming is out of scope
package util

import (
"fmt"
Expand Down
Loading