Add a Tasks manager in chaos daemon for time chaos #2812

Andrewmatilde · 2022-01-20T15:33:53Z

What problem does this PR solve?

I need a tasks manager to organize tasks on process in time chaos.

Two tasks can inject a same process.
When the last task on a process was recovered , I need to completely clear the chaos.

What's changed and how it works?

Nothing changes now , I just add a manager .
I will apply it on time chaos later.

Related changes

No related changes

Checklist

Tests

Unit test
Manual test (add steps below)

Side effects

Breaking backward compatibility

Release note

Please add a release note.

You can safely ignore this section if you don't think this PR needs a release note.

DCO

If you find the DCO check fails, please run commands like below (Depends on the actual situations. For example, if the failed commit isn't the most recent) to fix it:

git commit --amend --signoff
git push --force

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

ti-chi-bot · 2022-01-20T15:33:55Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

WangXiangUSTC
YangKeao

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

codecov · 2022-01-20T15:46:43Z

Codecov Report

Merging #2812 (8cfb897) into master (68e6ccb) will decrease coverage by 0.47%.
The diff coverage is 37.74%.

@@            Coverage Diff             @@
##           master    #2812      +/-   ##
==========================================
- Coverage   40.74%   40.27%   -0.48%     
==========================================
  Files         147      148       +1     
  Lines       12047    12230     +183     
==========================================
+ Hits         4909     4926      +17     
- Misses       6769     6912     +143     
- Partials      369      392      +23

Impacted Files	Coverage Δ
pkg/chaosdaemon/dns_server.go	`0.00% <0.00%> (ø)`
pkg/chaosdaemon/jvm_server.go	`0.00% <0.00%> (ø)`
pkg/chaosdaemon/stress_server_linux.go	`0.00% <0.00%> (ø)`
pkg/chaosdaemon/tasks/process_group_handle.go	`0.00% <0.00%> (ø)`
pkg/chaosdaemon/tc_server.go	`16.29% <0.00%> (ø)`
pkg/chaosdaemon/iptables_server.go	`65.92% <25.00%> (ø)`
pkg/chaosdaemon/tasks/task_manager.go	`43.79% <43.79%> (ø)`
pkg/chaosdaemon/tasks/task_config_manager.go	`52.32% <52.32%> (ø)`
pkg/chaosdaemon/ipset_server.go	`86.55% <80.00%> (ø)`
pkg/chaosdaemon/server.go	`37.50% <100.00%> (+1.22%)`	⬆️
... and 5 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 68e6ccb...8cfb897. Read the comment docs.

Andrewmatilde · 2022-01-20T16:14:13Z

PTAL, @WangXiangUSTC @STRRL

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

pkg/chaosdaemon/util/util.go

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

# Conflicts: # pkg/time/asset.go

YangKeao

Rest LGTM

pkg/chaoserr/common_errors.go

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

New Comments

pkg/chaosdaemon/tasks/task_manager.go

pkg/chaosdaemon/tasks/tasks_test.go

pkg/chaosdaemon/tasks/task_manager.go

…TaskConfig Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

…ge data inside the config manager. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

pkg/chaosdaemon/util/util.go

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

YangKeao

LGTM

WangXiangUSTC · 2022-02-18T08:16:04Z

pkg/chaosdaemon/tasks/process_group_handle.go

+	childPids, err := util.GetChildProcesses(uint32(pid), gp.logger)
+	if err != nil {
+		gp.logger.Error(err, "failed to get child process")
+	}


how about moving these codes after L58, and returning the error when getting child PID failed?

I agree with you.

WangXiangUSTC · 2022-02-18T08:22:34Z

pkg/chaosdaemon/tasks/process_group_handle.go

+	childPids, err := util.GetChildProcesses(uint32(pid), gp.logger)
+	if err != nil {
+		gp.logger.Error(err, "failed to get child process")
+	}


I agree with you.

WangXiangUSTC · 2022-02-18T08:34:09Z

pkg/chaosdaemon/tasks/task_config_manager.go

+}
+
+// Addable introduces the data gathering ability.
+type Addable interface {


how about moving this interface after interface Object? It looks strange to put it in the TaskConfigManager

WangXiangUSTC · 2022-02-18T08:35:20Z

pkg/chaosdaemon/tasks/task_config_manager.go

+	}
+	_, ok := m.TaskConfigMap[id]
+	if !ok {
+		return errors.Wrapf(ErrTaskConfigNotFound, "UID : %v", id)


L84 use %s for uid, and here use %v, how about unifying them?

I agree with you.

WangXiangUSTC · 2022-02-18T08:49:58Z

pkg/chaosdaemon/tasks/task_manager.go

+	if !ok {
+		return errors.New("task.Data here must implement TaskExecutor")
+	}
+	_ = tasker.Assign(process)


Suggested change

_ = tasker.Assign(process)

err = tasker.Assign(process)

WangXiangUSTC

rest LGTM

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

WangXiangUSTC

LGTM

Andrewmatilde · 2022-02-21T06:00:42Z

/run-e2e-test

Andrewmatilde · 2022-02-21T06:34:54Z

/merge

ti-chi-bot · 2022-02-21T06:34:56Z

This pull request has been accepted and is ready to merge.

Commit hash: 8cfb897

* Merge tasks manager Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Rollback time changes Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Format Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * some fix Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fix license Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fix fmt Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Add comments Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Add comments Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix Merge Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix logger Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix merge Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix merge fmt Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * minor fix Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fix comment Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fix data -> Data Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fix Data -> data Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Change some name & add more comments Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * make name in common error exported Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * make NotFound.Name unexported & Add some global variable&XXXTask->XXXTaskConfig Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Add more test. & Fix a bug that user can give a config pointer & change data inside the config manager. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Add more test. & Fix a bug that user can give a config pointer & change data inside the config manager. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * make error not found just a func Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix comment. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix comment. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix merge. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fmt Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

* Merge tasks manager Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Rollback time changes Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Format Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * some fix Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fix license Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fix fmt Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Add comments Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Add comments Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix Merge Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix logger Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix merge Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix merge fmt Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * minor fix Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fix comment Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fix data -> Data Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fix Data -> data Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Change some name & add more comments Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * make name in common error exported Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * make NotFound.Name unexported & Add some global variable&XXXTask->XXXTaskConfig Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Add more test. & Fix a bug that user can give a config pointer & change data inside the config manager. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Add more test. & Fix a bug that user can give a config pointer & change data inside the config manager. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * make error not found just a func Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix comment. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix comment. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * Fix merge. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> * fmt Signed-off-by: andrewmatilde <davis6813585853062@outlook.com> Signed-off-by: zhqqqy <zhqqqy@163.com>

Andrewmatilde added 4 commits January 20, 2022 17:03

Merge tasks manager

f54dfeb

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Rollback time changes

ea301f6

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Format

e087367

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

some fix

1b29c9a

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

ti-chi-bot requested review from YangKeao and yeya24 January 20, 2022 15:34

ti-chi-bot added the size/XXL label Jan 20, 2022

Andrewmatilde removed request for YangKeao and yeya24 January 20, 2022 15:35

Andrewmatilde added 2 commits January 20, 2022 23:43

fix license

1b22fea

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

fix fmt

393f8b0

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Andrewmatilde changed the title ~~Tasks manager~~ Add a Tasks manager for time chaos Jan 20, 2022

Andrewmatilde changed the title ~~Add a Tasks manager for time chaos~~ Add a Tasks manager in chaos daemon for time chaos Jan 20, 2022

Add comments

c87a17e

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Andrewmatilde force-pushed the tasks-manager branch from d1bd446 to c87a17e Compare January 21, 2022 07:54

Andrewmatilde added 2 commits January 21, 2022 16:49

Add comments

6f9bdc8

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Merge branch 'master' into tasks-manager

b4b8337

STRRL requested review from STRRL, YangKeao, Hexilee and WangXiangUSTC January 24, 2022 05:08

Fix Merge

5c96ffa

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

iguoyr reviewed Jan 25, 2022

View reviewed changes

pkg/chaosdaemon/util/util.go Outdated Show resolved Hide resolved

ti-chi-bot added the needs-rebase label Jan 25, 2022

Andrewmatilde added 2 commits January 26, 2022 14:14

Fix logger

35d4bdd

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Merge remote-tracking branch 'm/master' into tasks-manager

be5daa4

# Conflicts: # pkg/time/asset.go

ti-chi-bot removed the needs-rebase label Jan 26, 2022

YangKeao previously approved these changes Feb 10, 2022

View reviewed changes

pkg/chaoserr/common_errors.go Outdated Show resolved Hide resolved

pkg/chaoserr/common_errors.go Show resolved Hide resolved

ti-chi-bot added the status/LGT1 label Feb 10, 2022

STRRL mentioned this pull request Feb 11, 2022

UPDATE: appending logger name convention chaos-mesh/rfcs#37

Merged

make name in common error exported

f109deb

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

YangKeao reviewed Feb 15, 2022

View reviewed changes

Andrewmatilde added 3 commits February 15, 2022 22:05

make NotFound.Name unexported & Add some global variable&XXXTask->XXX…

60bcc07

…TaskConfig Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Add more test. & Fix a bug that user can give a config pointer & chan…

720b3cb

…ge data inside the config manager. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Add more test. & Fix a bug that user can give a config pointer & chan…

26dff2c

…ge data inside the config manager. Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

sonatype-lift bot reviewed Feb 15, 2022

View reviewed changes

pkg/chaosdaemon/util/util.go Show resolved Hide resolved

Andrewmatilde added 2 commits February 15, 2022 23:39

make error not found just a func

efab693

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Merge branch 'master' into tasks-manager

180a0f0

YangKeao approved these changes Feb 18, 2022

View reviewed changes

WangXiangUSTC reviewed Feb 19, 2022

View reviewed changes

WangXiangUSTC reviewed Feb 21, 2022

View reviewed changes

Andrewmatilde added 5 commits February 21, 2022 12:39

Fix comment.

8b36210

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Fix comment.

9d12aa4

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

Merge branch 'master' into tasks-manager

d5fc956

Fix merge.

c758707

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

fmt

8cfb897

Signed-off-by: andrewmatilde <davis6813585853062@outlook.com>

WangXiangUSTC approved these changes Feb 21, 2022

View reviewed changes

ti-chi-bot added status/LGT2 and removed status/LGT1 labels Feb 21, 2022

ti-chi-bot added the status/can-merge label Feb 21, 2022

ti-chi-bot merged commit bef9049 into chaos-mesh:master Feb 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a Tasks manager in chaos daemon for time chaos #2812

Add a Tasks manager in chaos daemon for time chaos #2812

Andrewmatilde commented Jan 20, 2022 •

edited

ti-chi-bot commented Jan 20, 2022 •

edited

codecov bot commented Jan 20, 2022 •

edited

Andrewmatilde commented Jan 20, 2022

YangKeao left a comment

YangKeao left a comment

WangXiangUSTC Feb 18, 2022

Andrewmatilde Feb 21, 2022

Andrewmatilde Feb 21, 2022

WangXiangUSTC Feb 18, 2022

Andrewmatilde Feb 21, 2022

Andrewmatilde Feb 21, 2022

WangXiangUSTC Feb 18, 2022

Andrewmatilde Feb 21, 2022

WangXiangUSTC Feb 18, 2022

Andrewmatilde Feb 21, 2022

Andrewmatilde Feb 21, 2022

WangXiangUSTC Feb 18, 2022

Andrewmatilde Feb 21, 2022

Andrewmatilde Feb 21, 2022

WangXiangUSTC left a comment

WangXiangUSTC left a comment

Andrewmatilde commented Feb 21, 2022

Andrewmatilde commented Feb 21, 2022

ti-chi-bot commented Feb 21, 2022

Add a Tasks manager in chaos daemon for time chaos #2812

Add a Tasks manager in chaos daemon for time chaos #2812

Conversation

Andrewmatilde commented Jan 20, 2022 • edited

What problem does this PR solve?

What's changed and how it works?

Related changes

Checklist

Release note

DCO

ti-chi-bot commented Jan 20, 2022 • edited

codecov bot commented Jan 20, 2022 • edited

Codecov Report

Andrewmatilde commented Jan 20, 2022

YangKeao left a comment

Choose a reason for hiding this comment

YangKeao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WangXiangUSTC left a comment

Choose a reason for hiding this comment

WangXiangUSTC left a comment

Choose a reason for hiding this comment

Andrewmatilde commented Feb 21, 2022

Andrewmatilde commented Feb 21, 2022

ti-chi-bot commented Feb 21, 2022

Andrewmatilde commented Jan 20, 2022 •

edited

ti-chi-bot commented Jan 20, 2022 •

edited

codecov bot commented Jan 20, 2022 •

edited