-
-
Notifications
You must be signed in to change notification settings - Fork 68
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 for re-usable containers #693
Conversation
Related with #691 |
@orlangure do you agree with the approach I followed? If yes, I'll add the comments and tests and put it to review. |
Very cool! I like the idea to keep the existing behaviour unless the other option is specified, and I really like the extra bit you added with the image match: if the image changed, no need to reuse the container, it needs to be created again. I looked at the code briefly, and all looks good. I think there might be a problem with "running" filter you used to list the containers: if there is a container with the same name that is not "running", we will try to create a new one with the same name and fail (I guess that's what would happen). Please look a bit more into that potential issue, or confirm that the behaviour will be as expected with the existing code. |
@orlangure tried to reproduce the problem but was unable to. For two reasons:
|
@orlangure also added tests. The only test I did not add was for the "replacement scenario" (the one where the image mismatches) and that was because I didn't know which image to use other than |
Codecov ReportBase: 85.88% // Head: 85.80% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #693 +/- ##
==========================================
- Coverage 85.88% 85.80% -0.09%
==========================================
Files 49 49
Lines 2281 2310 +29
==========================================
+ Hits 1959 1982 +23
- Misses 167 170 +3
- Partials 155 158 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@orlangure fixed lint issues, should be ok now |
@devuo, thank you so much for working on it, and for the last sidecar touch as well 😼 |
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.
Perfect 😼
Thank you.
Add support for developers to pass
gnomock.WithContainerReuse()
option that makes Gnomock attempt to re-use an already running container with the name specified viagnomock.WithContainerName()
.If the container with the given name does not exist, or if it uses a different image than the one configured, the normal flow of Gnomock is used, namely a new container is created in case it doesn't exist, or replaced if using a different image.
Pending: