-
Notifications
You must be signed in to change notification settings - Fork 402
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
Fix duplicated volume issue #690
Fix duplicated volume issue #690
Conversation
confirmed by Yi Cheng that this issue is no longer observed. cc @iycheng |
4969e91
to
e0de9f7
Compare
e0de9f7
to
fb1f485
Compare
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.
Unit test appreciated but not required.
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.
Thank @wilsonwang371 for the update! Can you rebase the branch? I have added some config tests in the latest commit. In addition, unit tests appreciated. Thank you!
fb1f485
to
80b5290
Compare
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.
Mm, actually, I do want to request a unit test -- testing is pretty important to the stability of this codebase.
@wilsonwang371 this will be good to merge after a basic unit test is added |
7155d0c
to
266b9b3
Compare
266b9b3
to
152b4a4
Compare
@wilsonwang371 is it ready to merge? The PR title is "WIP". Thank you! |
Deleted the WIP :) . Merging. |
this is really fast..... i was thinking about a few more tests... although it is fine to merge now... |
My bad! Sometimes I'm a bit trigger happy. |
Avoid duplicating volumeMount paths.
Why are these changes needed?
some corner cases can be triggered and the error below is returned
Related issue number
#688
Checks