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

chaos-daemon: remove host ipc #2869

Merged
merged 4 commits into from
Mar 10, 2022
Merged

Conversation

YangKeao
Copy link
Member

What problem does this PR solve?

Fix #2845 . With hostIPC, the /dev/shm on the host will be mounted to the container. As I know, the hostIPC was brought with TimeChaos, and I'm not sure whether it's really needed, we can see in the test.

A more surprising thing is that even with priviledged, the chaos-daemon can successfully start. As I have tested, the following chaos works well:

  • PodChaos
  • NetworkChaos
  • TimeChaos

The following chaos doesn't work:

  • DNSChaos
  • IOChaos
  • StressChaos
  • HTTPChaos

Need further discussion about why they didn't work. I noticed that the sidecar injection test also fails.

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>
Signed-off-by: YangKeao <yangkeao@chunibyo.icu>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Feb 10, 2022

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • STRRL
  • cwen0

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.

@YangKeao YangKeao changed the title Remove host ipc chaos-daemon: remove host ipc Feb 10, 2022
@YangKeao
Copy link
Member Author

For HTTPChaos, the e2e-test fails to dial the helper.

STEP: waiting on e2e helper ready
Feb 10 08:54:41.974: INFO: Err : Get "http://10.41.0.100:31428/ping": context deadline exceeded (Client.Timeout exceeded while awaiting headers) , IP : 10.41.0.100
Feb 10 08:54:53.977: INFO: Err : Get "http://10.41.0.100:31428/ping": context deadline exceeded (Client.Timeout exceeded while awaiting headers) , IP : 10.41.0.100

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>
@YangKeao
Copy link
Member Author

/run-e2e-tests

@YangKeao YangKeao requested a review from a team February 10, 2022 10:15
@STRRL
Copy link
Member

STRRL commented Feb 21, 2022

The following chaos doesn't work:

DNSChaos
IOChaos
StressChaos
HTTPChaos

What does this mean? I saw the e2e tests succeed. Only not work in rootless containers?

@YangKeao
Copy link
Member Author

YangKeao commented Feb 23, 2022

What does this mean? I saw the e2e tests succeed. Not work only in rootless containers?

Yes. (But I don't know why actually. It needs more investigation, but it's fine to merge this PR)

Copy link
Member

@STRRL STRRL 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 YangKeao requested a review from a team February 28, 2022 06:18
@cwen0
Copy link
Member

cwen0 commented Mar 1, 2022

The following chaos doesn't work:

  • DNSChaos
  • IOChaos
  • StressChaos
  • HTTPChaos

Need further discussion about why they didn't work. I noticed that the sidecar injection test also fails.

Do these chaos types work well if set hostIPS to true?

@YangKeao
Copy link
Member Author

YangKeao commented Mar 2, 2022

The following chaos doesn't work:

  • DNSChaos
  • IOChaos
  • StressChaos
  • HTTPChaos

Need further discussion about why they didn't work. I noticed that the sidecar injection test also fails.

Do these chaos types work well if set hostIPS to true?

No. chaos-daemon cannot start up with hostIPS. See #2845

@YangKeao
Copy link
Member Author

@cwen0 PTAL

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

@cwen0
Copy link
Member

cwen0 commented Mar 10, 2022

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 220fd90

@ti-chi-bot ti-chi-bot merged commit d73ff71 into chaos-mesh:master Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants