Add event recorder to show chaos status. #245

AngleNet · 2020-02-16T07:15:32Z

What problem does this PR solve?

#162 Add event recorder to show chaos status after we have applied a resource.

What is changed and how does it work?

Support event recorder for chaos starting and completion

Check List

Tests

Manual test (add detailed scripts or steps below)

Code changes

Has Go code change

Side effects

No

Related changes

Changes of signature of schedule reconciler.

Does this PR introduce a user-facing change?:

Add event recorder for chaos start and completion

codecov-io · 2020-02-16T07:17:59Z

Codecov Report

Merging #245 into master will increase coverage by 7.77%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
+ Coverage    52.9%   60.67%   +7.77%     
==========================================
  Files          34       36       +2     
  Lines        1603     2014     +411     
==========================================
+ Hits          848     1222     +374     
- Misses        668      698      +30     
- Partials       87       94       +7

Impacted Files	Coverage Δ
controllers/timechaos_controller.go	`0% <0%> (ø)`	⬆️
pkg/chaosfs/server.go	`82.25% <0%> (ø)`
pkg/chaosfs/hook.go	`96.51% <0%> (ø)`
controllers/iochaos_controller.go	`92.85% <0%> (+2.38%)`	⬆️
controllers/podchaos_controller.go	`93.1% <0%> (+2.62%)`	⬆️
controllers/networkchaos_controller.go	`93.1% <0%> (+2.62%)`	⬆️

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 bc904ad...db5c56c. Read the comment docs.

AngleNet · 2020-02-16T07:24:31Z

HI. Currently I only add event recorder for the controller-manager. Its events are still missing. I wonder whether we should gather all of events around the chaos resource or spread the events to the owned resources. I prefer the previous one.

AngleNet · 2020-02-16T07:27:45Z

Another problem is that shall we keep track of normal execution of a chaos action? Should the event report the chaos progress?

zhouqiang-cl · 2020-02-16T09:20:22Z

@AngleNet thank you, @cwen0 @Yisaer PTAL

zhouqiang-cl · 2020-02-16T12:05:14Z

Another problem is that shall we keep track of normal execution of a chaos action?

I think we should

Should the event report the chaos progress?

I think the scheduler chaos can not show the progress, we did not know when to end

I wonder whether we should gather all of events around the chaos resource or spread the events to the owned resources. I prefer the previous one.

@Yisaer @cwen0 @YangKeao what's your oponion?

cwen0 · 2020-02-17T02:34:26Z

Another problem is that shall we keep track of normal execution of a chaos action? Should the event report the chaos progress?

In my option, I think we can record the event when the chaos starts and ends.

cwen0 · 2020-02-17T02:53:58Z

HI. Currently I only add event recorder for the controller-manager. Its events are still missing. I wonder whether we should gather all of events around the chaos resource or spread the events to the owned resources. I prefer the previous one.

About this question, If we gather all of the events around the chaos resource, the number of events will be very large and confusing. In my option, I think we can spread the events to the owned resources.

AngleNet · 2020-02-17T03:30:37Z

HI. Currently I only add event recorder for the controller-manager. Its events are still missing. I wonder whether we should gather all of events around the chaos resource or spread the events to the owned resources. I prefer the previous one.

HI. Currently I only add event recorder for the controller-manager. Its events are still missing. I wonder whether we should gather all of events around the chaos resource or spread the events to the owned resources. I prefer the previous one.

About this question, If we gather all of the events around the chaos resource, the number of events will be very large and confusing. In my option, I think we can spread the events to the owned resources.

Make sense ! How about we gather events about chaos actions for begining and ending around the chaos resource? For the events from resources operated by chaos actions, we should let it untouched and K8s will generate events for them. Users can check them if they want.

cwen0 · 2020-02-17T04:32:02Z

HI. Currently I only add event recorder for the controller-manager. Its events are still missing. I wonder whether we should gather all of events around the chaos resource or spread the events to the owned resources. I prefer the previous one.

HI. Currently I only add event recorder for the controller-manager. Its events are still missing. I wonder whether we should gather all of events around the chaos resource or spread the events to the owned resources. I prefer the previous one.

About this question, If we gather all of the events around the chaos resource, the number of events will be very large and confusing. In my option, I think we can spread the events to the owned resources.

Make sense ! How about we gather events about chaos actions for begining and ending around the chaos resource? For the events from resources operated by chaos actions, we should let it untouched and K8s will generate events for them. Users can check them if they want.

We can record an event when to apply chaos action and record the other one when to cancel the action

…eature/event

controllers/iochaos_controller.go

Yisaer · 2020-02-20T15:20:54Z

As ChaosMesh users needs to know the every timestamp for performing and recovering the chaos action, I suggest that to add event in the certain chaos action controller like following logic:
In podfailure/types.go

// Apply implements the reconciler.InnerReconciler.Apply
func (r *Reconciler) Apply(ctx context.Context, req ctrl.Request, obj reconciler.InnerObject) error {

// Perform chaos action logic ...

r.Recorder.Event(chaos, v1.EventTypeNormal, utils.EventChaosPerformed, "podfaulre chaos perfromed,affect 3 pods")
}

// Recover implements the reconciler.InnerReconciler.Recover
func (r *Reconciler) Recover(ctx context.Context, req ctrl.Request, obj reconciler.InnerObject) error {

// Recovery chaos action logic

r.Recorder.Event(chaos, v1.EventTypeNormal, utils.EventChaosRecovered, "podfaulre chaos recovered, recover 3 pods")
}

In this way, the event would be sent for the podfailure chaos object each time it get performed and recovered without caring where the calling from (schedule chaos or common chaos) I think this would help you. @AngleNet

AngleNet · 2020-02-20T15:52:24Z

Yes. It’s a good place for handling chaos starting and recovering event. However, I think we could still record failing events in the entry reconciler.

Yisaer · 2020-02-21T03:58:04Z

@AngleNet Remaining failure event in the entry reconciler is ok to me.

controllers/iochaos/types.go

…eature/event

zhouqiang-cl · 2020-02-22T09:45:58Z

@AngleNet thank you very much, we will take a look as soon as possible

Yisaer

The rest lgtm.

controllers/iochaos/fs/types.go

Yisaer · 2020-02-23T04:34:19Z

controllers/iochaos_controller.go

+	if err != nil {
+		if !chaos.IsDeleted() {
+			r.Event(chaos, v1.EventTypeWarning, utils.EventChaosInjectFailed, err.Error())
+		} else {
+			r.Event(chaos, v1.EventTypeWarning, utils.EventChaosRecoverFailed, err.Error())
+		}
+	}
+	return result, nil


The logic seems weird here. If the reconciliation failed, I suggest send event with reconciliation failed event type and add the error message into event message. It is not necessary to judge which kind of chaos action (inject or recover)

controllers/networkchaos/netem/types.go

Yisaer · 2020-02-23T04:34:40Z

controllers/networkchaos/netem/types.go

@@ -88,6 +94,7 @@ func (r *Reconciler) Apply(ctx context.Context, req ctrl.Request, chaos reconcil
 		return err
 	}

+	r.Event(networkchaos, v1.EventTypeNormal, utils.EventChaosToInject, "")


controllers/podchaos_controller.go

Yisaer · 2020-02-23T04:36:53Z

controllers/timechaos/types.go

@@ -131,6 +127,7 @@ func (r *Reconciler) Recover(ctx context.Context, req ctrl.Request, chaos reconc
 		return err
 	}

+	r.Event(timechaos, v1.EventTypeNormal, utils.EventChaosToRecover, "")


controllers/timechaos/types.go

Yisaer · 2020-02-23T04:37:09Z

controllers/timechaos_controller.go

+	if err != nil {
+		if !chaos.IsDeleted() {
+			r.Event(chaos, v1.EventTypeWarning, utils.EventChaosInjectFailed, err.Error())
+		} else {
+			r.Event(chaos, v1.EventTypeWarning, utils.EventChaosRecoverFailed, err.Error())
+		}


pkg/utils/events.go

controllers/common/types.go

cwen0

LGTM

zhouqiang-cl · 2020-02-24T14:24:58Z

@Yisaer Please take a final look 😝

Add event recorder to show chaos status

Update

f210607

AngleNet changed the title ~~[WIP] Add event recorder to show chaos status.~~ [DRAFT][WIP] Add event recorder to show chaos status. Feb 16, 2020

AngleNet requested review from Yisaer and zhouqiang-cl February 16, 2020 07:31

zhouqiang-cl added contribution labels Feb 16, 2020

zhouqiang-cl and others added 3 commits February 17, 2020 21:01

Merge branch 'master' into feature/event

875aa9a

Update

1f772d5

Merge branch 'feature/event' of github.com:AngleNet/chaos-mesh into f…

061b0ae

…eature/event

cwen0 removed the status/PTAL label Feb 18, 2020

Update

b70204c

hound bot deleted a comment from zhouqiang-cl Feb 18, 2020

hound bot deleted a comment from AngleNet Feb 18, 2020

hound bot deleted a comment from zhouqiang-cl Feb 18, 2020

hound bot deleted a comment from cwen0 Feb 18, 2020

hound bot deleted a comment from AngleNet Feb 18, 2020

AngleNet added 3 commits February 18, 2020 22:11

Update

b2063ce

Update

479f12e

Update

38d46c6

Yisaer reviewed Feb 19, 2020

View reviewed changes

controllers/iochaos_controller.go Show resolved Hide resolved

zhouqiang-cl added status/ReqChange and removed status/PTAL labels Feb 20, 2020

zhouqiang-cl reviewed Feb 22, 2020

View reviewed changes

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

AngleNet and others added 4 commits February 22, 2020 15:25

Merge branch 'master' into feature/event

9f956f2

Update

0f8b154

Merge branch 'feature/event' of github.com:AngleNet/chaos-mesh into f…

5c764b7

…eature/event

Update

c648a95

AngleNet requested review from Yisaer and zhouqiang-cl February 22, 2020 08:50

zhouqiang-cl added status/PTAL and removed status/ReqChange labels Feb 22, 2020

zhouqiang-cl requested review from YangKeao and cwen0 February 22, 2020 09:38

Merge branch 'master' into feature/event

7e6aa6f

Yisaer requested changes Feb 23, 2020

View reviewed changes

zhouqiang-cl added status/ReqChange and removed status/PTAL labels Feb 23, 2020

Update

bc904ad

cwen0 reviewed Feb 24, 2020

View reviewed changes

controllers/common/types.go Show resolved Hide resolved

cwen0 approved these changes Feb 24, 2020

View reviewed changes

Merge branch 'master' into feature/event

db5c56c

Yisaer approved these changes Feb 25, 2020

View reviewed changes

zhouqiang-cl merged commit fe22eae into chaos-mesh:master Feb 25, 2020

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

Add event recorder to show chaos status. (chaos-mesh#245)

8cf8b64

Add event recorder to show chaos status

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add event recorder to show chaos status. #245

Add event recorder to show chaos status. #245

AngleNet commented Feb 16, 2020 •

edited

Loading

codecov-io commented Feb 16, 2020 •

edited

Loading

AngleNet commented Feb 16, 2020

AngleNet commented Feb 16, 2020

zhouqiang-cl commented Feb 16, 2020

zhouqiang-cl commented Feb 16, 2020

cwen0 commented Feb 17, 2020

cwen0 commented Feb 17, 2020

AngleNet commented Feb 17, 2020

cwen0 commented Feb 17, 2020

Yisaer commented Feb 20, 2020 •

edited

Loading

AngleNet commented Feb 20, 2020

Yisaer commented Feb 21, 2020

zhouqiang-cl commented Feb 22, 2020

Yisaer left a comment

Yisaer Feb 23, 2020

Yisaer Feb 23, 2020

Yisaer Feb 23, 2020

Yisaer Feb 23, 2020

cwen0 left a comment

zhouqiang-cl commented Feb 24, 2020

Add event recorder to show chaos status. #245

Add event recorder to show chaos status. #245

Conversation

AngleNet commented Feb 16, 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 16, 2020 • edited Loading

Codecov Report

AngleNet commented Feb 16, 2020

AngleNet commented Feb 16, 2020

zhouqiang-cl commented Feb 16, 2020

zhouqiang-cl commented Feb 16, 2020

cwen0 commented Feb 17, 2020

cwen0 commented Feb 17, 2020

AngleNet commented Feb 17, 2020

cwen0 commented Feb 17, 2020

Yisaer commented Feb 20, 2020 • edited Loading

AngleNet commented Feb 20, 2020

Yisaer commented Feb 21, 2020

zhouqiang-cl commented Feb 22, 2020

Yisaer left a comment

Choose a reason for hiding this comment

Yisaer Feb 23, 2020

Choose a reason for hiding this comment

Yisaer Feb 23, 2020

Choose a reason for hiding this comment

Yisaer Feb 23, 2020

Choose a reason for hiding this comment

Yisaer Feb 23, 2020

Choose a reason for hiding this comment

cwen0 left a comment

Choose a reason for hiding this comment

zhouqiang-cl commented Feb 24, 2020

AngleNet commented Feb 16, 2020 •

edited

Loading

codecov-io commented Feb 16, 2020 •

edited

Loading

Yisaer commented Feb 20, 2020 •

edited

Loading