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.

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

Support multiple containers and multiple processes for time chaos #325

Merged
merged 20 commits into from Mar 12, 2020

Conversation

YangKeao
Copy link
Member

@YangKeao YangKeao commented Mar 9, 2020

What problem does this PR solve?

Support multiple containers and multiple processes for time chaos.

What is changed and how does it work?

Check List

Tests

  • Manual test

Code changes

  • Has Go code change

Signed-off-by: Yang Keao <keao.yang@yahoo.com>
Signed-off-by: Yang Keao <keao.yang@yahoo.com>
@YangKeao YangKeao requested review from fewdan and cwen0 March 9, 2020 10:56
@codecov-io
Copy link

codecov-io commented Mar 9, 2020

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #325   +/-   ##
=======================================
  Coverage   58.75%   58.75%           
=======================================
  Files          51       51           
  Lines        2849     2849           
=======================================
  Hits         1674     1674           
  Misses       1057     1057           
  Partials      118      118

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 88f3091...6ed444b. Read the comment docs.

@@ -128,15 +128,30 @@ tidy:
GO111MODULE=on go mod tidy
git diff --quiet go.mod go.sum

image:
image: image-chaos-daemon image-chaos-mesh image-chaos-fs image-chaos-scripts image-chaos-grafana image-chaos-dashboard image-chaos-kernel
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change because building chaos-kernel image is too slow (on my minikube virtual machine).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to change CI scripts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I only split image into multiple subtasks, so there is no need to change CI scripts.

Signed-off-by: Yang Keao <keao.yang@yahoo.com>
Signed-off-by: Yang Keao <keao.yang@yahoo.com>
@YangKeao YangKeao changed the title Support multiple containers and multiple processes for time chaos [WIP] Support multiple containers and multiple processes for time chaos Mar 10, 2020
Signed-off-by: Yang Keao <keao.yang@yahoo.com>
@YangKeao YangKeao changed the title [WIP] Support multiple containers and multiple processes for time chaos Support multiple containers and multiple processes for time chaos Mar 10, 2020
Signed-off-by: Yang Keao <keao.yang@yahoo.com>
@YangKeao YangKeao requested a review from cwen0 March 10, 2020 10:44
Signed-off-by: Yang Keao <keao.yang@yahoo.com>
Signed-off-by: Yang Keao <keao.yang@yahoo.com>
Signed-off-by: Yang Keao <keao.yang@yahoo.com>

reader, err := os.Open(statusPath)
if err != nil {
log.Info("read status file error", "path", statusPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log.Error?

allPids := append(childPids, pid)
log.Info("all related processes found", "pids", allPids)

for _, pid := range allPids {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the child process disappeared at this time?

Signed-off-by: Yang Keao <keao.yang@yahoo.com>
Copy link
Member

@cwen0 cwen0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@ethercflow ethercflow 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
Copy link
Member Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Mar 12, 2020

/run-all-tests

@sre-bot sre-bot merged commit 0724ec9 into chaos-mesh:master Mar 12, 2020
@YangKeao YangKeao deleted the time-child-processes branch August 3, 2020 03:43
sjwsl pushed a commit to sjwsl/chaos-mesh that referenced this pull request May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants