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

chaosdaemon: make more TC filters work #1309

Merged
merged 4 commits into from
Dec 21, 2020
Merged

chaosdaemon: make more TC filters work #1309

merged 4 commits into from
Dec 21, 2020

Conversation

cwen0
Copy link
Member

@cwen0 cwen0 commented Dec 19, 2020

Signed-off-by: cwen0 cwenyin0@gmail.com

What problem does this PR solve?

Support more TC filters.

If the TC request doesn't include ipset field, the other filters won't work. Because we use ipset to distinguish if the tc rules need filters.

https://github.com/chaos-mesh/chaos-mesh/blob/master/pkg/chaosdaemon/tc_server.go#L122-L127

	for _, tc := range in.Tcs {
		if tc.Ipset == "" {
			globalTc = append(globalTc, tc)
		} else {
			// TODO: support multiple tc with one ipset
			filterTc[tc.Ipset] = append(filterTc[tc.Ipset], tc)
		}
	}

What is changed and how does it work?

Check all TC filters(ipset, source port, egress port, Protocol) to distinguish if we need to set TC rules with filters.

Checklist

Tests

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

Side effects

  • Breaking backward compatibility

Related changes

  • Need to update the documentation

Does this PR introduce a user-facing change?

NONE

@cwen0
support more filters
deb5e7a 
Signed-off-by: cwen0 <cwenyin0@gmail.com>
@cwen0 cwen0 added the DNM label Dec 19, 2020
@cwen0
Copy link
Member Author

cwen0 commented Dec 19, 2020

/run-e2e-tests

@cwen0 cwen0 added status/WIP and removed DNM labels Dec 19, 2020
@cwen0 cwen0 changed the title chaosdaemon: support more TC filters [WIP] chaosdaemon: support more TC filters Dec 19, 2020
@codecov-io
Copy link

codecov-io commented Dec 19, 2020 •
edited
Loading

Codecov Report

Merging #1309 (1b39e88) into master (7e9ff3f) will decrease coverage by 2.56%.
The diff coverage is 52.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1309      +/-   ##
==========================================
- Coverage   55.78%   53.21%   -2.57%     
==========================================
  Files          68       85      +17     
  Lines        4383     5346     +963     
==========================================
+ Hits         2445     2845     +400     
- Misses       1768     2231     +463     
- Partials      170      270     +100
Impacted Files Coverage Δ
api/v1alpha1/common_types.go 0.00% <0.00%> (ø)
api/v1alpha1/common_webhook.go 100.00% <ø> (ø)
api/v1alpha1/dnschaos_type.go 0.00% <0.00%> (ø)
api/v1alpha1/dnschaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/httpchaos_types.go 0.00% <0.00%> (ø)
api/v1alpha1/iochaos_types.go 0.00% <ø> (-40.00%) ⬇️
api/v1alpha1/jvmchaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/kernelchaos_types.go 0.00% <ø> (-20.00%) ⬇️
api/v1alpha1/kernelchaos_webhook.go 100.00% <ø> (+14.81%) ⬆️
api/v1alpha1/kinds.go 27.27% <ø> (+0.60%) ⬆️
... and 124 more

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 e97ff52...1b39e88. Read the comment docs.

@cwen0 cwen0 changed the title [WIP] chaosdaemon: support more TC filters [WIP] chaosdaemon: make more TC filters work Dec 19, 2020
@cwen0
Copy link
Member Author

cwen0 commented Dec 19, 2020

/run-e2e-tests

@cwen0
support multiple tc rules with same filters
1b39e88 
Signed-off-by: cwen0 <cwenyin0@gmail.com>
@cwen0
Copy link
Member Author

cwen0 commented Dec 19, 2020

/run-e2e-tests

@cwen0 cwen0 marked this pull request as ready for review December 20, 2020 13:53
@cwen0 cwen0 changed the title [WIP] chaosdaemon: make more TC filters work chaosdaemon: make more TC filters work Dec 20, 2020
@cwen0 cwen0 added type/enhancement New feature or request status/PTAL and removed status/WIP labels Dec 21, 2020
STRRL
STRRL approved these changes Dec 21, 2020
View reviewed changes
Copy link
Member

@STRRL STRRL 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 Dec 21, 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
Copy link
Member Author

cwen0 commented Dec 21, 2020

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit b74b0e1 into chaos-mesh:master Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YangKeao YangKeao YangKeao approved these changes

@STRRL STRRL STRRL approved these changes

Assignees
No one assigned
Labels
status/can-merge type/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@cwen0 @codecov-io @ti-srebot @YangKeao @STRRL