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.

Sign up for GitHub

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

Jump to bottom

redisChaos: support sentinel stop/restart #152

Merged
merged 13 commits into from
May 5, 2022
Merged

redisChaos: support sentinel stop/restart #152

merged 13 commits into from
May 5, 2022

Conversation

FingerLeader
Copy link
Member

No description provided.

@FingerLeader
sentinel stop attack draft
eacf903 
Signed-off-by: FingerLeader <wanxfinger@gmail.com>
@FingerLeader
sentinel stop and restart attack draft
9931ca5 
Signed-off-by: FingerLeader <wanxfinger@gmail.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Apr 7, 2022 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • WangXiangUSTC
  • 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.

@FingerLeader
remove flag DB
813abb3 
Signed-off-by: FingerLeader <wanxfinger@gmail.com>
@FingerLeader FingerLeader marked this pull request as ready for review April 8, 2022 05:38
@FingerLeader
add flag flush-config
47cd87b 
Signed-off-by: FingerLeader <wanxfinger@gmail.com>
@cwen0
Copy link
Member

cwen0 commented Apr 15, 2022

@FingerLeader Can you fix the CI first?

FingerLeader and others added 4 commits April 18, 2022 22:42
@FingerLeader
make boilerplate
4594948 
Signed-off-by: FingerLeader <wanxfinger@gmail.com>
@FingerLeader
Merge branch 'main' into add-redis
a6f5e33
@FingerLeader
make check
7f02b6f 
Signed-off-by: FingerLeader <wanxfinger@gmail.com>
@FingerLeader
Merge branch 'add-redis' of https://github.com/FingerLeader/chaosd in…
9a97b4c 
…to add-redis
@FingerLeader
make check
eeae948 
Signed-off-by: FingerLeader <wanxfinger@gmail.com>
WangXiangUSTC
WangXiangUSTC reviewed Apr 24, 2022
View reviewed changes
pkg/server/chaosd/redis.go Show resolved Hide resolved
pkg/server/chaosd/redis.go Outdated
if attack.Conf == "" {
return errors.WithStack(errors.Errorf("redis config is not present"))
}
recoverCmd := exec.Command("redis-server", attack.Conf, "--sentinel")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to make sure the redis-server is in PATH. I think we can provide a recover-cmd just like the process attack.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a flag to set the path of redis-server

pkg/server/chaosd/redis.go Show resolved Hide resolved
@FingerLeader
add flag redis-path
d878d71 
Signed-off-by: FingerLeader <wanxfinger@gmail.com>
@WangXiangUSTC WangXiangUSTC added this to the v1.3 milestone May 5, 2022
@FingerLeader
Copy link
Member Author

PTAL /cc @cwen0 @WangXiangUSTC

WangXiangUSTC
WangXiangUSTC approved these changes May 5, 2022
View reviewed changes
Copy link
Collaborator

@WangXiangUSTC WangXiangUSTC left a comment

Choose a reason for hiding this comment

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

LGTM

WangXiangUSTC
WangXiangUSTC reviewed May 5, 2022
View reviewed changes
cmd/attack/redis.go Outdated Show resolved Hide resolved
@WangXiangUSTC
Copy link
Collaborator

It's better to add some description for this pr, and link the issue #133

@FingerLeader
edit some details
f7a9325 
Signed-off-by: FingerLeader <wanxfinger@gmail.com>
@FingerLeader FingerLeader mentioned this pull request May 5, 2022
Open
@FingerLeader
Copy link
Member Author

It's better to add some description for this pr, and link the issue #133

Done it

cwen0
cwen0 approved these changes May 5, 2022
View reviewed changes
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 May 5, 2022

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 98cd90d

@ti-chi-bot ti-chi-bot merged commit bf14130 into chaos-mesh:main May 5, 2022
@FingerLeader FingerLeader deleted the add-redis branch May 6, 2022 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WangXiangUSTC WangXiangUSTC WangXiangUSTC approved these changes

@cwen0 cwen0 cwen0 approved these changes

@Andrewmatilde Andrewmatilde Awaiting requested review from Andrewmatilde

Assignees
No one assigned
Labels
size/XL status/can-merge status/LGT2
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.
4 participants
@FingerLeader @ti-chi-bot @cwen0 @WangXiangUSTC