fix(verify): use container name from configuration in verify tests#9753
fix(verify): use container name from configuration in verify tests#9753mattsanta merged 2 commits intoGoogleContainerTools:mainfrom
Conversation
|
Thanks for the PR, I tested it locally and it seems to be working. It looks like an integration test is failing however. |
Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
katiexzhang
left a comment
There was a problem hiding this comment.
It looks like container name is required for verify actually, so this would solve the problem completely.
Something else to consider - wdyt about extracting the name checking logic in getContainerName to run in the s.Go()? If we check the container name existence every time before createAndRunContainer is called, then it should be creating the new ones with diff UUIDs.
I think it's also a good solution, it'll reduce a bit the execution speed, because you have to call the docker client in sync mode multiple times, but I think it should be okay, because it shouldn't take a lot of time and it'll make sense if a user has tons of containers. And, I think it'll be better to extract the |
|
I'm going to merge this PR for now + create another issue to unify the verify + custom actions logic. I'm still not sure exactly why everything is working for custom actions (likely has to do with how it gets called + the threads start running), so refactoring might be more complicated than anticipated. |
Fixes: #9747
Description
Add using container name from configuration in verify tests