ray-operator: disallow pod creation in namespaces outside of RayCluster namespace #1951
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewsykim
force-pushed
the
fix-pod-namespace
branch
2 times, most recently
from
ca43795
to
d06aafe
Compare
andrewsykim
changed the title
|
Do any release notes need to be update? |
| podTemplate.ObjectMeta.Namespace = instance.Namespace | ||
| log.Info("Setting pod namespaces", "namespace", instance.Namespace) | ||
| } | ||
| podTemplate.ObjectMeta.Namespace = instance.Namespace |
There was a problem hiding this comment.
Maybe its better to return an error in this case overriding user values seems wrong?
| podTemplate.ObjectMeta.Namespace = instance.Namespace | ||
| log.Info("Setting pod namespaces", "namespace", instance.Namespace) | ||
| } | ||
| podTemplate.ObjectMeta.Namespace = instance.Namespace |
There was a problem hiding this comment.
could you add a comment here explaining why we are overriding the namespace.
|
The testing section of the PR description needs to be updated. How was this tested? |
I was waiting for feedback from @kevin85421 before writing tests. I've added tests now. Thanks |
andrewsykim
force-pushed
the
fix-pod-namespace
branch
from
d06aafe
to
52e6aba
Compare
…r namespace Signed-off-by: Andrew Sy Kim <andrewsy@google.com>
andrewsykim
force-pushed
the
fix-pod-namespace
branch
from
52e6aba
to
20db4e9
Compare
kevin85421
approved these changes
Feb 29, 2024
There was a problem hiding this comment.
LGTM. @vinayakankugoyal Do you have any other comments? I will merge this PR after your approval.
vinayakankugoyal
approved these changes
Feb 29, 2024
ravishtiwari
pushed a commit
to ravishtiwari/kuberay
that referenced
this pull request
Mar 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
Currently it is possible to specify a namespace in the pod template that is different from the namespace of the RayCluster. I haven't found a use-case where this is needed. This PR disallows namespaces for Pods outside the namespace of the RayCluster.
Related issue number
Checks