Support duration chaos for pod failure #175

Yisaer · 2020-02-01T10:58:48Z

What problem does this PR solve?

#171
Make Podfailure chaos support duration chaos feature.

What is changed and how does it work?

Adding a new controller interface as duration controller like twophase controller to control the logic of duration chaos. Each kind of chaos would implement both interfaces.

Check List

Tests

Manual test (add detailed scripts or steps below)

Code changes

Has Go code change

Side effects

None

Related changes

Need to update the documentation

Does this PR introduce a user-facing change?:

Support duration chaos for pod failure by omitting the scheduler and duration.

codecov-io · 2020-02-01T11:05:41Z

Codecov Report

❗ No coverage uploaded for pull request base (master@97015f9). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #175   +/-   ##
=========================================
  Coverage          ?   39.47%           
=========================================
  Files             ?       17           
  Lines             ?      608           
  Branches          ?        0           
=========================================
  Hits              ?      240           
  Misses            ?      337           
  Partials          ?       31

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 97015f9...9ad11f5. Read the comment docs.

controllers/duration/podfailure/types.go

zhouqiang-cl · 2020-02-02T03:25:55Z

@mahjonp @cwen0 @YangKeao PTAL

zhouqiang-cl · 2020-02-02T03:31:52Z

controllers/podchaos/podfailure/types.go

@@ -15,7 +15,6 @@ package podfailure

 import (


should we make a dir named controllers/scheduler/podchaos

zhouqiang-cl · 2020-02-02T03:32:57Z

controllers/duration/types.go

+			return r.Update(ctx, chaos)
+		})
+		if updateError != nil {
+			r.Log.Error(updateError, "unable to update chaos finalizers")


Did we need return updateError here

I think we don't need to, this reconciliation would be re queued in the end.

mahjonp · 2020-02-02T08:26:53Z

controllers/podchaos/types.go

+			Client:                 r.Client,
+			Log:                    r.Log,
+		}
+		return reconciler.Reconcile(req)


I think this is a little redundant, how about merge these two parts into one constructor method?

controllers/podchaos/podfailure/types.go

cwen0 · 2020-02-03T02:03:56Z

@Yisaer I think the name duration chaos is not very accurate to describe this kind of chaos, because we don't have the option of duration to control this kind of chaos, which is easy to cause misunderstanding.

cwen0 · 2020-02-03T02:11:52Z

@Yisaer I think we can rename twophase controller to cheduler controller and rename duration chaos to common chaos. What do you think?

Yisaer · 2020-02-03T02:14:10Z

@cwen0 It's ok for me to rename both twophase controller and duration controller. In this request, I would rename duration chaos to common chaos. We could create new request for renaming twophase controller.

cwen0 · 2020-02-03T02:34:46Z

@cwen0 It's ok for me to rename both twophase controller and duration controller. In this request, I would rename duration chaos to common chaos. We could create new request for renaming twophase controller.

Great！

mahjonp

rest lgtm

controllers/podchaos/podfailure/types.go

controllers/podchaos/types.go

you06 · 2020-02-03T05:25:43Z

I find both twophase controller and duration controller wrap podfailure in very similar ways and they do only expose some functions from podfailure, what's the purpose for it?

Co-Authored-By: mahjonp <[email protected]>

…ub.com/Yisaer/chaos-mesh into support_duration_chaos_for_pod_failure

Yisaer · 2020-02-03T05:42:15Z

I find both twophase controller and duration controller wrap podfailure in very similar ways and they do only expose some functions from podfailure, what's the purpose for it?

The wrap here is to implement the interface like InnerReconciler and InnerCommonReconcile which were used to handle the logic of common chaos and scheduler chaos.

zhouqiang-cl

LGTM

controllers/podchaos/types.go

Co-Authored-By: mahjonp <[email protected]>

zhouqiang-cl · 2020-02-03T06:02:26Z

@cwen0 PTAL again

controllers/common/podfailure/types.go

controllers/common/types.go

controllers/podchaos/podfailure/types.go

cwen0 · 2020-02-03T06:52:00Z

controllers/podchaos/types.go

+		r.Log.Error(err, fmt.Sprintf("unable to get podchaos[%s/%s]'s duration", podchaos.Namespace, podchaos.Name))
+		return ctrl.Result{}, nil
+	}
+	if scheduler == nil && duration == nil {


How about pod-kill action? Duration is always empty for pod-kill

good catch and updated.

controllers/twophase/podfailure/types.go

…ub.com/Yisaer/chaos-mesh into support_duration_chaos_for_pod_failure

zhouqiang-cl · 2020-02-03T07:35:46Z

@cwen0 PTAL

cwen0

LGTM

support duration chaos for podfailure

b85730b

Yisaer added status/WIP component/operator labels Feb 1, 2020

Song Gao added 2 commits February 1, 2020 19:15

fix failinject annotation logic

752f0e3

add comment by lint

be05ec5

Yisaer changed the title ~~[WIP] Support duration chaos for pod failure~~ Support duration chaos for pod failure Feb 1, 2020

Yisaer added status/PTAL and removed status/WIP labels Feb 1, 2020

Yisaer requested review from cwen0, zhouqiang-cl and mahjonp February 1, 2020 11:27

zhouqiang-cl reviewed Feb 1, 2020

View reviewed changes

controllers/duration/podfailure/types.go Outdated Show resolved Hide resolved

zhouqiang-cl requested a review from YangKeao February 1, 2020 13:14

Merge branch 'master' into support_duration_chaos_for_pod_failure

81234c8

zhouqiang-cl reviewed Feb 2, 2020

View reviewed changes

mahjonp reviewed Feb 2, 2020

View reviewed changes

controllers/podchaos/podfailure/types.go Outdated Show resolved Hide resolved

Merge branch 'master' into support_duration_chaos_for_pod_failure

ae8159c

mahjonp requested review from you06 and mapleFU February 2, 2020 11:57

Song Gao added 3 commits February 3, 2020 10:43

fix by goimport

b88b097

rename duiration chaos into common chaos

4de14d2

rename duration chaos to common chaos

f7e4f1c

mahjonp reviewed Feb 3, 2020

View reviewed changes

controllers/podchaos/podfailure/types.go Outdated Show resolved Hide resolved

controllers/podchaos/podfailure/types.go Outdated Show resolved Hide resolved

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

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

Yisaer and others added 4 commits February 3, 2020 13:33

Update controllers/podchaos/podfailure/types.go

a0cdada

Co-Authored-By: mahjonp <[email protected]>

Update controllers/podchaos/podfailure/types.go

e06d8f5

Co-Authored-By: mahjonp <[email protected]>

fix by comment

8c9d709

Merge branch 'support_duration_chaos_for_pod_failure' of https://gith…

34f50c0

…ub.com/Yisaer/chaos-mesh into support_duration_chaos_for_pod_failure

Yisaer requested a review from mahjonp February 3, 2020 05:38

zhouqiang-cl reviewed Feb 3, 2020

View reviewed changes

mahjonp reviewed Feb 3, 2020

View reviewed changes

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

Update controllers/podchaos/types.go

8b442e0

Co-Authored-By: mahjonp <[email protected]>

mahjonp previously approved these changes Feb 3, 2020

View reviewed changes

cwen0 reviewed Feb 3, 2020

View reviewed changes

Song Gao and others added 5 commits February 3, 2020 15:05

fix controller logic

97015f9

Merge branch 'master' into support_duration_chaos_for_pod_failure

560c3fa

format

40e4d41

Merge branch 'support_duration_chaos_for_pod_failure' of https://gith…

861a626

…ub.com/Yisaer/chaos-mesh into support_duration_chaos_for_pod_failure

format

d2c1b52

Yisaer dismissed mahjonp’s stale review via d2c1b52 February 3, 2020 07:17

format goimports

9ad11f5

mahjonp approved these changes Feb 3, 2020

View reviewed changes

Yisaer requested review from cwen0 and zhouqiang-cl February 3, 2020 07:31

cwen0 approved these changes Feb 3, 2020

View reviewed changes

cwen0 merged commit 8b02bd3 into chaos-mesh:master Feb 3, 2020

Yisaer mentioned this pull request Feb 20, 2020

[Proposal] A New Design for Control Layer Controller #269

Closed

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

Support duration chaos for pod failure (chaos-mesh#175)

6c29cfe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support duration chaos for pod failure #175

Support duration chaos for pod failure #175

Yisaer commented Feb 1, 2020 •

edited

Loading

codecov-io commented Feb 1, 2020 •

edited

Loading

zhouqiang-cl commented Feb 2, 2020

zhouqiang-cl Feb 2, 2020

zhouqiang-cl Feb 2, 2020

Yisaer Feb 3, 2020

mahjonp Feb 2, 2020

cwen0 commented Feb 3, 2020

cwen0 commented Feb 3, 2020

Yisaer commented Feb 3, 2020

cwen0 commented Feb 3, 2020

mahjonp left a comment

you06 commented Feb 3, 2020

Yisaer commented Feb 3, 2020

zhouqiang-cl left a comment

zhouqiang-cl commented Feb 3, 2020

cwen0 Feb 3, 2020

Yisaer Feb 3, 2020

zhouqiang-cl commented Feb 3, 2020

cwen0 left a comment

Support duration chaos for pod failure #175

Support duration chaos for pod failure #175

Conversation

Yisaer commented Feb 1, 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?:

codecov-io commented Feb 1, 2020 • edited Loading

Codecov Report

zhouqiang-cl commented Feb 2, 2020

zhouqiang-cl Feb 2, 2020

Choose a reason for hiding this comment

zhouqiang-cl Feb 2, 2020

Choose a reason for hiding this comment

Yisaer Feb 3, 2020

Choose a reason for hiding this comment

mahjonp Feb 2, 2020

Choose a reason for hiding this comment

cwen0 commented Feb 3, 2020

cwen0 commented Feb 3, 2020

Yisaer commented Feb 3, 2020

cwen0 commented Feb 3, 2020

mahjonp left a comment

Choose a reason for hiding this comment

you06 commented Feb 3, 2020

Yisaer commented Feb 3, 2020

zhouqiang-cl left a comment

Choose a reason for hiding this comment

zhouqiang-cl commented Feb 3, 2020

cwen0 Feb 3, 2020

Choose a reason for hiding this comment

Yisaer Feb 3, 2020

Choose a reason for hiding this comment

zhouqiang-cl commented Feb 3, 2020

cwen0 left a comment

Choose a reason for hiding this comment

Yisaer commented Feb 1, 2020 •

edited

Loading

codecov-io commented Feb 1, 2020 •

edited

Loading