Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support reorder in network delay chaos #252

Merged
merged 4 commits into from
Mar 2, 2020
Merged

Support reorder in network delay chaos #252

merged 4 commits into from
Mar 2, 2020

Conversation

yeya24
Copy link
Contributor

@yeya24 yeya24 commented Feb 17, 2020 •
edited
Loading

Signed-off-by: yeya24 [email protected]

What problem does this PR solve?

What is changed and how does it work?

Add a reorder fields in network delay chaos

Check List

Tests

  • Unit test
  • E2E test
  • Manual test (add detailed scripts or steps below)
  • No code

Code changes

  • Has Go code change
  • Has CI related scripts change
  • Has Terraform scripts change

Side effects

  • Breaking backward compatibility

Related changes

  • Need to update the documentation

Does this PR introduce a user-facing change?:

NONE

@zhouqiang-cl
Copy link
Contributor

@yeya24 please fix ci

@YangKeao
Copy link
Member

YangKeao commented Feb 18, 2020 •
edited
Loading

I think reorder has to cooperate with delay, doesn't it?

to use reordering, a delay option must be specified. There are two
ways to use this option (assuming 'delay 10ms' in the options list).

Reorder will make some packets be sent immediately while others are delayed.

@yeya24
Copy link
Contributor Author

yeya24 commented Feb 18, 2020

@YangKeao Thanks for it! Sorry I miss that. What about make reorder an option field in delay?

@zhouqiang-cl
Copy link
Contributor

@YangKeao Thanks for it! Sorry I miss that. What about make reorder an option field in delay?

I think we can. @YangKeao PTAL

@YangKeao
Copy link
Member

YangKeao commented Feb 19, 2020 •
edited
Loading

@yeya24 Great. But notice that nullable is not supported in lower version of kubernetes, so we can only use optional + omitempty to add reorder related field.

@cwen0
Copy link
Member

cwen0 commented Feb 21, 2020

/test

@yeya24
Copy link
Contributor Author

yeya24 commented Feb 21, 2020

/test

I haven't updated this PR as @YangKeao said, let me make the state to WIP.

@yeya24 yeya24 changed the title Support reorder type network chaos WIP: Support reorder type network chaos Feb 21, 2020
@yeya24 yeya24 changed the title WIP: Support reorder type network chaos Support reorder type network chaos Feb 27, 2020
@yeya24
support reorder type network chaos
7f1ca28 
Signed-off-by: yeya24 <[email protected]>

regenerate crd files

Signed-off-by: yeya24 <[email protected]>
@yeya24
make reorder part of delay action
086dedd 
Signed-off-by: yeya24 <[email protected]>
@yeya24
Copy link
Contributor Author

yeya24 commented Feb 27, 2020

Ready for review now. But I found the netem reorder is not that easy to test.

@yeya24
regenerate crd file
94ffc92 
Signed-off-by: yeya24 <[email protected]>
@codecov-io
Copy link

codecov-io commented Feb 27, 2020 •
edited
Loading

Codecov Report

Merging #252 into master will increase coverage by 1.01%.
The diff coverage is 4.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #252      +/-   ##
==========================================
+ Coverage   59.61%   60.62%   +1.01%     
==========================================
  Files          37       40       +3     
  Lines        2028     2494     +466     
==========================================
+ Hits         1209     1512     +303     
- Misses        724      882     +158     
- Partials       95      100       +5
Impacted Files Coverage Δ
pkg/webhook/inject/inject.go 75.07% <ø> (ø)
api/v1alpha1/iochaos_types.go 28.57% <0%> (-3.99%) ⬇️
api/v1alpha1/timechaos_types.go 3.84% <0%> (-0.51%) ⬇️
api/v1alpha1/networkchaos_types.go 12.38% <0%> (-0.7%) ⬇️
api/v1alpha1/podchaos_types.go 21.87% <0%> (-10.69%) ⬇️
pkg/webhook/config/watcher/watcher.go 54.08% <100%> (ø)
pkg/webhook/config/watcher/config.go 100% <0%> (ø)
pkg/webhook/config/config.go 90% <0%> (+2.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94ffc92...62ea54c. Read the comment docs.

zhouqiang-cl
zhouqiang-cl reviewed Feb 27, 2020
View reviewed changes
Copy link
Contributor

@zhouqiang-cl zhouqiang-cl left a comment

Choose a reason for hiding this comment

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

LGTM

YangKeao
YangKeao approved these changes Feb 28, 2020
View reviewed changes
Copy link
Member

@YangKeao YangKeao left a comment

Choose a reason for hiding this comment

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

LGTM

cwen0
cwen0 approved these changes Mar 2, 2020
View reviewed changes
Copy link
Member

@cwen0 cwen0 left a comment

Choose a reason for hiding this comment

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

LGTM

@cwen0
Copy link
Member

cwen0 commented Mar 2, 2020

/merge

@yeya24 yeya24 changed the title Support reorder type network chaos Support reorder in network delay chaos Mar 2, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 2, 2020

/run-all-tests

@sre-bot sre-bot merged commit 65de8c3 into chaos-mesh:master Mar 2, 2020
sjwsl pushed a commit to sjwsl/chaos-mesh that referenced this pull request May 6, 2021
@yeya24
Support reorder in network delay chaos (chaos-mesh#252)
0797de5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhouqiang-cl zhouqiang-cl zhouqiang-cl left review comments

@cwen0 cwen0 cwen0 approved these changes

@YangKeao YangKeao YangKeao approved these changes

Assignees
No one assigned
Labels
status/can-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@yeya24 @zhouqiang-cl @YangKeao @cwen0 @codecov-io @sre-bot