Skip to content
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

Wait after docker kill to prevent name conflicts #187

Merged

Conversation

RobertWilbrandt
Copy link
Collaborator

While working on ros2 driver unit tests, i noticed sporadically failing tests with the error message

2: [INFO] [start_ursim-15]: process started with pid [88538]
2: [start_ursim-15] ursim_net already exists
2: [start_ursim-15] docker: Error response from daemon: Conflict. The container name "/ursim" is already in use by container "f3ac67680b8da8c476966ac9b3ee2a4193b273379fa584647663bf5eb478c590". You have to remove (or rename) that container to be able to reuse that name.
2: [start_ursim-15] See 'docker run --help'.
2: [ERROR] [start_ursim-15]: process has died [pid 88538, exit code 125, cmd '/home/wilbrandt/robot_folders/checkout/ur_rolling/colcon_ws/install/ur_client_library/lib/ur_client_library/start_ursim.sh -m  ur5e'].

I also found examples for this in our CI, e.g. https://github.com/UniversalRobots/Universal_Robots_ROS2_Driver/actions/runs/6464858675/job/17550136726

Using docker wait after docker kill seems to fix this for me.

Without this, using start_ursim.sh directly after killing it (e.g. in a
test) could sporadically lead to container name conflicts.
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

All modified lines are covered by tests ✅

see 3 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

Copy link
Collaborator

@fmauch fmauch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fair, thank you!

@fmauch fmauch merged commit 111ddf2 into UniversalRobots:master Oct 12, 2023
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants