-
Notifications
You must be signed in to change notification settings - Fork 822
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
[add] Support namespace scoped chaos #872
[add] Support namespace scoped chaos #872
Conversation
… mode Signed-off-by: STRRL <str_ruiling@outlook.com>
…cope Signed-off-by: STRRL <str_ruiling@outlook.com>
21bf986
to
980cb96
Compare
@Yisaer PTAL |
I found that |
Hi @cwen0 @Gallardot mentors, one more thing, shall we consider to split out namespace which inject chaos and namespace which running controller? For example, running Or we should play in ONLY ONE namespace. |
Basically LGTM except the selector is too complex for now. Maybe we need to find a better way to refactor it. Adding an e2e test to verify the namespace-scoped would be better. @STRRL |
Hi @Yisaer , could you take a little while on this #872 (comment) ? Please give me your opinion about this question. Thanks! |
…ped mode or cluster scoped Signed-off-by: STRRL <str_ruiling@outlook.com>
…actions; Signed-off-by: STRRL <str_ruiling@outlook.com>
… and injecting chaos Signed-off-by: STRRL <str_ruiling@outlook.com>
I am working on this, it could improve isolation of our system. |
980cb96
to
ad37f8a
Compare
0cb5f2d
to
a446991
Compare
Thanks for your contribution. I didn't mean refactoring selector in this request either. This should be tracked by a new issue and still need to be discussed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest using namespaceSelector for webhook while namespace-scoped
is enabled.
As namespaceSelector
need apiserver >= 1.15(need to be verified), the if-else
template rendering could refer to here
For this question, my answer is no (do not split the manager and chaos in different namespaces). The origin demand for this feature is to limit the range for the chaos mesh (namespace scoped is common for the most controllers while it is more important for chaos-mesh comparing to other controllers as we do need to control the effected range by chaos-mesh) And this question also remind me that the manager's |
Hi @Yisaer mentor, I still do not figure out why. 😅 Deploying chaos-controller-manager and applications which are the target of chaos-testing into SAME namespace is DANGEROUS. For example, if user using a bad selector, it might inject chaos into controller itself. Other operators like prometheus-operator and openkruise have their own namespace which only running controller. I am very sure that your will NOT put your application into
It seems that we have opposite opinions, I am very pleasure to discuss this question on wechat or slack. :D |
In this way, deploying |
d4dc2dc
to
9d1349e
Compare
Signed-off-by: STRRL <str_ruiling@outlook.com>
73e2092
to
cccb5e9
Compare
Signed-off-by: STRRL <str_ruiling@outlook.com>
cccb5e9
to
3af5f80
Compare
HI, @Yisaer , PTAL |
@Gallardot @Yisaer PTAL |
Signed-off-by: STRRL <str_ruiling@outlook.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/merge |
/run-all-tests |
cherry pick to release-1.0 failed |
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
Signed-off-by: STRRL <str_ruiling@outlook.com>
What problem does this PR solve?
Make chaos-mesh controller could work without permission of cluster scope, like pod list from cluster scope.
Related issue: #487
What is changed and how does it work?
This PR makes three things:
Checklist
Todos:
- [ ] enhance webhook (like, using namespaceSelector)- [ ] e2e testsIt's hard to test as expected operation is NOOP.Tests
Manually test steps:
WIP
Side effects
Related changes
Does this PR introduce a user-facing change?
No