fix(binder): sanitize configmap volume names for dotted pod names#1728
fix(binder): sanitize configmap volume names for dotted pod names#1728julsemaan wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
9885f57 to
a2e2596
Compare
|
@julsemaan Thsnk you or the fix can you please add to the PR description of the solution? |
|
@coderabbitai review |
✅ Action performedReview finished.
|
25681df to
aeb9ef2
Compare
@enoodle, thanks for triggering the review. I fixed:
|
|
Total coverage: 51.2% -> 51.0% (delta -0.20%) Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
|
@julsemaan Can you update the changelog file please? |
Signed-off-by: Julien Semaan <jsemaan@kubex.ai>
aeb9ef2 to
536313d
Compare
|
@enoodle, I forgot to force push 🙃, should be fine now |
|
@enoodle, it looks like FOSSA failed due to some network error, could you get it to run again 🙏 |
|
@enoodle, looks like only approvals are missing and we are good to merge? |
Description
When pods contain a dot in their name, the configmap created for gpu sharing also contains a dot.
Although the configmap name can contain a dot, the volumes in the pods cannot which generates invalid pods when KAI mutates them on admission to add the configmap as a volume to the pod
Example error this is fixing:
The solution is to replace all chars that are not valid DNS chars by
-which will include dots and all other potential invalid charsRelated Issues
Fixes #1729
Checklist
Breaking Changes
I am not sure yet if this should be considered a breaking change, please advise
Additional Notes