Support force clean finalizer in various chaos types #478

yujunz · 2020-05-09T01:06:11Z

What problem does this PR solve?

#227

What is changed and how does it work?

Force clean finalizer

Check List

Tests

Manual test (see examples/clean-finalizer)

Code changes

Has Go code change

Does this PR introduce a user-facing change?:

Add support to force clean finalizer with annotation in various chaos types

yujunz · 2020-05-09T01:23:42Z

@cwen0 It is almost a copy & paste from #415 .

Would it be good to refactor networkchaos types to reduce code duplicate?

yujunz · 2020-05-09T01:34:49Z

It seems not just networkchaos, some other types of chaos also have similar boilerplate code.

yujunz · 2020-05-09T01:42:04Z

The test failure seems irrelevant to this PR.

• Failure [0.012 seconds]
PTrace
/home/runner/work/chaos-mesh/chaos-mesh/go/src/github.com/pingcap/chaos-mesh/pkg/ptrace/ptrace_linux_test.go:63
  should mmap slice successfully [It]
  /home/runner/work/chaos-mesh/chaos-mesh/go/src/github.com/pingcap/chaos-mesh/pkg/ptrace/ptrace_linux_test.go:88
  error: no such process

Is it just flake? How to re-trigger test?

yeya24 · 2020-05-09T01:43:49Z

The test failure seems irrelevant to this PR.

• Failure [0.012 seconds]
PTrace
/home/runner/work/chaos-mesh/chaos-mesh/go/src/github.com/pingcap/chaos-mesh/pkg/ptrace/ptrace_linux_test.go:63
  should mmap slice successfully [It]
  /home/runner/work/chaos-mesh/chaos-mesh/go/src/github.com/pingcap/chaos-mesh/pkg/ptrace/ptrace_linux_test.go:88
  error: no such process

Is it just flake? How to re-trigger test?

Yes, it is flaky and I saw this several times. I will re-trigger the CI for you.

cwen0 · 2020-05-11T02:32:02Z

/run-e2e-test

cwen0 · 2020-05-11T06:19:59Z

It seems not just networkchaos, some other types of chaos also have similar boilerplate code.

@yujunz Yes, at present, we have some repeated code in deferent chaos, we also want to refine these code, do you have any good idea about this issue?

yujunz · 2020-05-11T06:42:33Z

@yujunz Yes, at present, we have some repeated code in deferent chaos, we also want to refine these code, do you have any good idea about this issue?

We may need an abstract layer for it. There are already something like InnerReconciler, TwoPhaseReconciler but I haven't look into the details. Generally speaking, we may move some common logic to the abstract layer or have some factory method to create those common logic when initializing the concrete chaos object.

My two cents.

cwen0 · 2020-05-11T09:50:48Z

@yujunz Yes, at present, we have some repeated code in deferent chaos, we also want to refine these code, do you have any good idea about this issue?

We may need an abstract layer for it. There are already something like InnerReconciler, TwoPhaseReconciler but I haven't look into the details. Generally speaking, we may move some common logic to the abstract layer or have some factory method to create those common logic when initializing the concrete chaos object.

My two cents.

I agree with your idea. But we won't include it in the next release and implement it in the later versions. Because this modification needs to involve a lot of places and work to do. If you are interested in it, you can push a proposal and help us to improve this. 😁

controllers/kernelchaos/types.go

controllers/networkchaos/partition/types.go

yujunz · 2020-05-11T11:14:58Z

Re-grouped imports. @cwen0

yujunz · 2020-05-11T11:57:16Z

The same flake...

yeya24 · 2020-05-11T14:37:31Z

/run-e2e-test

controllers/networkchaos/tbf/types.go

controllers/kernelchaos/types.go

controllers/networkchaos/partition/types.go

controllers/networkchaos/tbf/types.go

controllers/podchaos/podfailure/types.go

controllers/stresschaos/types.go

controllers/timechaos/types.go

yeya24

Awesome! LGTM if E2E passed

yeya24 · 2020-05-12T13:34:22Z

/run-e2e-test

cwen0 · 2020-05-13T02:45:06Z

/run-e2e-test

cwen0

LGTM

cwen0 · 2020-05-13T04:06:19Z

/merge

sre-bot · 2020-05-13T04:06:21Z

Your auto merge job has been accepted, waiting for:

481

sre-bot · 2020-05-13T04:10:47Z

/run-all-tests

codecov-io · 2020-05-13T04:12:59Z

Codecov Report

Merging #478 into master will increase coverage by 0.13%.
The diff coverage is 52.14%.

@@            Coverage Diff             @@
##           master     #478      +/-   ##
==========================================
+ Coverage   58.57%   58.71%   +0.13%     
==========================================
  Files          60       68       +8     
  Lines        3626     4374     +748     
==========================================
+ Hits         2124     2568     +444     
- Misses       1333     1616     +283     
- Partials      169      190      +21

Impacted Files	Coverage Δ
api/v1alpha1/common_webhook.go	`100.00% <ø> (ø)`
api/v1alpha1/iochaos_types.go	`31.11% <0.00%> (-1.45%)`	⬇️
api/v1alpha1/kernelchaos_types.go	`4.44% <0.00%> (-0.21%)`	⬇️
api/v1alpha1/networkchaos_types.go	`18.65% <0.00%> (-0.29%)`	⬇️
api/v1alpha1/podchaos_types.go	`31.11% <0.00%> (-1.45%)`	⬇️
controllers/podchaos/types.go	`65.95% <0.00%> (ø)`
controllers/stresschaos_controller.go	`0.00% <0.00%> (ø)`
controllers/timechaos_controller.go	`0.00% <0.00%> (ø)`
controllers/twophase/types.go	`55.78% <0.00%> (-7.31%)`	⬇️
pkg/chaosdaemon/netem_linux.go	`26.66% <0.00%> (-17.78%)`	⬇️
... and 38 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 2dbab6c...2d78dba. Read the comment docs.

sre-bot · 2020-05-13T04:14:54Z

/run-all-tests

sre-bot · 2020-05-13T04:16:56Z

@yujunz merge failed.

cwen0 · 2020-05-13T04:18:51Z

/merge

sre-bot · 2020-05-13T04:18:53Z

Your auto merge job has been accepted, waiting for:

466

sre-bot · 2020-05-13T04:21:09Z

/run-all-tests

sre-bot · 2020-05-13T04:25:17Z

/run-all-tests

sre-bot · 2020-05-13T04:27:19Z

@yujunz merge failed.

yujunz · 2020-05-13T06:26:54Z

What is the cause of "merge failed" ? @cwen0

you06 · 2020-05-13T07:58:10Z

@yujunz it's caused by a bug of sre-bot 🤖. FYI pingcap-incubator/cherry-bot/22.

sre-bot added the contribution label May 9, 2020

yujunz changed the title ~~Support force clean finalizer in networkchaos/tbf~~ Support force clean finalizer in networkchaos May 9, 2020

yujunz changed the title ~~Support force clean finalizer in networkchaos~~ Support force clean finalizer in various chaos types May 9, 2020

yujunz marked this pull request as ready for review May 9, 2020 01:35

cwen0 reviewed May 11, 2020

View reviewed changes

controllers/kernelchaos/types.go Outdated Show resolved Hide resolved

controllers/networkchaos/partition/types.go Outdated Show resolved Hide resolved

yujunz force-pushed the clean-finalizer branch from 03b4592 to 2fd32a3 Compare May 11, 2020 11:11

yujunz added 2 commits May 11, 2020 19:12

Support force clean finalizer in networkchaos/tbf

d5f3d16

Support force clean finalizer in networkchaos/partition

316a060

yujunz force-pushed the clean-finalizer branch from 2fd32a3 to 29069d0 Compare May 11, 2020 11:13

yeya24 requested changes May 11, 2020

View reviewed changes

Support force clean finalizer in various chaos

a86bd3a

yujunz force-pushed the clean-finalizer branch from 29069d0 to a86bd3a Compare May 12, 2020 10:07

yujunz requested a review from yeya24 May 12, 2020 10:34

yeya24 approved these changes May 12, 2020

View reviewed changes

cwen0 approved these changes May 13, 2020

View reviewed changes

sre-bot added the status/can-merge label May 13, 2020

Merge branch 'master' into clean-finalizer

ae217f7

Merge branch 'master' into clean-finalizer

2d78dba

Merge branch 'master' into clean-finalizer

7a3628e

cwen0 merged commit 18c13f3 into chaos-mesh:master May 13, 2020

yujunz deleted the clean-finalizer branch May 13, 2020 06:26

sjwsl pushed a commit to sjwsl/chaos-mesh that referenced this pull request May 6, 2021

Support force clean finalizer in various chaos types (chaos-mesh#478)

c48aab7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support force clean finalizer in various chaos types #478

Support force clean finalizer in various chaos types #478

yujunz commented May 9, 2020 •

edited

Loading

yujunz commented May 9, 2020

yujunz commented May 9, 2020 •

edited

Loading

yujunz commented May 9, 2020 •

edited

Loading

yeya24 commented May 9, 2020

cwen0 commented May 11, 2020

cwen0 commented May 11, 2020

yujunz commented May 11, 2020 •

edited

Loading

cwen0 commented May 11, 2020

yujunz commented May 11, 2020

yujunz commented May 11, 2020

yeya24 commented May 11, 2020

yeya24 left a comment •

edited

Loading

yeya24 commented May 12, 2020

cwen0 commented May 13, 2020

cwen0 left a comment

cwen0 commented May 13, 2020

sre-bot commented May 13, 2020

sre-bot commented May 13, 2020

codecov-io commented May 13, 2020 •

edited

Loading

sre-bot commented May 13, 2020

sre-bot commented May 13, 2020

cwen0 commented May 13, 2020

sre-bot commented May 13, 2020

sre-bot commented May 13, 2020

sre-bot commented May 13, 2020

sre-bot commented May 13, 2020

yujunz commented May 13, 2020

you06 commented May 13, 2020

Support force clean finalizer in various chaos types #478

Support force clean finalizer in various chaos types #478

Conversation

yujunz commented May 9, 2020 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Does this PR introduce a user-facing change?:

yujunz commented May 9, 2020

yujunz commented May 9, 2020 • edited Loading

yujunz commented May 9, 2020 • edited Loading

yeya24 commented May 9, 2020

cwen0 commented May 11, 2020

cwen0 commented May 11, 2020

yujunz commented May 11, 2020 • edited Loading

cwen0 commented May 11, 2020

yujunz commented May 11, 2020

yujunz commented May 11, 2020

yeya24 commented May 11, 2020

yeya24 left a comment • edited Loading

Choose a reason for hiding this comment

yeya24 commented May 12, 2020

cwen0 commented May 13, 2020

cwen0 left a comment

Choose a reason for hiding this comment

cwen0 commented May 13, 2020

sre-bot commented May 13, 2020

sre-bot commented May 13, 2020

codecov-io commented May 13, 2020 • edited Loading

Codecov Report

sre-bot commented May 13, 2020

sre-bot commented May 13, 2020

cwen0 commented May 13, 2020

sre-bot commented May 13, 2020

sre-bot commented May 13, 2020

sre-bot commented May 13, 2020

sre-bot commented May 13, 2020

yujunz commented May 13, 2020

you06 commented May 13, 2020

yujunz commented May 9, 2020 •

edited

Loading

yujunz commented May 9, 2020 •

edited

Loading

yujunz commented May 9, 2020 •

edited

Loading

yujunz commented May 11, 2020 •

edited

Loading

yeya24 left a comment •

edited

Loading

codecov-io commented May 13, 2020 •

edited

Loading