Support ROS kinetic in addition to indigo

osrf / ariac-docker

Containerization of ARIAC competition server and competitors' code. See https://bitbucket.org/osrf/ariac/wiki/Home

10 stars 8 forks source link

Support ROS kinetic in addition to indigo #7

Closed dhood closed 7 years ago

dhood commented 7 years ago

addresses https://github.com/osrf/ariac-docker/issues/2

dhood commented 7 years ago

this doesn't build for indigo at the moment because of https://github.com/osrf/ariac-docker/pull/7/files#diff-266724451f3250ace79d67de111c02e3R14

dhood commented 7 years ago

Thanks for iterating on this @j-rivero: I was planning on taking another look after the other PRs (didn't mean to just dump it here with a complaint!)

I noticed the testing job you ran (thank you for creating that, by the way!), and saw it's failing because of the change I added to make the userid of the ariac user the same as the user running the script. To give context, that change was needed to support f011bac4ab6e05fc5ed4722f10a76347d35a22f0 to avoid the need for our script to be run with sudo.

There are a couple of different ways forward here (change the userid on the buildfarm job, support usage as root, revert the commits, etc): you probably have better insight into what's most appropriate, but I wanted to save you the time looking for the cause of the failure.

j-rivero commented 7 years ago

I've added a minor thing for users that run docker as root in the docker_version_root branch.

I've testing the whole build now in Jenkins:

Kinetic: https://build.osrfoundation.org/view/All/job/ariac-install-docker_kinetic-xenial-amd64/13/console

dhood commented 7 years ago

I think we'll also need to remove this line: https://github.com/osrf/ariac-docker/commit/f011bac4ab6e05fc5ed4722f10a76347d35a22f0#diff-44baf6f7c0d7d0beed7ccda1e6f7388bR26 in order to get it to run correctly. (I haven't checked, but when I was on a machine calling the script with uid 1002, and the ariac user had 1000, it was this line inside the container that failed, so I suspect that user 1000 will have issues copying files to the mounted directory if it's owned by root)

With https://github.com/osrf/ariac-docker/pull/8 I've changed the example team's system to end the trial after 10s, so we should be able to add automated runtime tests once it's merged. I've kicked off a test on trusty at https://build.osrfoundation.org/view/All/job/ariac-install-docker_indigo-trusty-amd64/9/. If that goes OK we can merge this, merge #8, add the runtime test to the buildfarm job, then iterate if it fails at runtime for root user.

dhood commented 7 years ago

the trusty job is having issues, but I understand it to be because of "docker in docker". Given that this works locally, I'll merge this in order to proceed, and we can work on the trusty buildfarm job in parallel.

j-rivero commented 7 years ago

the trusty job is having issues, but I understand it to be because of "docker in docker". Given that this works locally, I'll merge this in order to proceed, and we can work on the trusty buildfarm job in parallel.

Ummm yes, seems a problem related to the docker infra. It should be ok to merge.

With #8 I've changed the example team's system to end the trial after 10s, so we should be able to add automated runtime tests once it's merged.

Perfect! Let's work from #8.

dhood commented 7 years ago

Re-checking I noticed the kinetic job had issues building the competitor image; should be fixed in 67af1b0613d90d81585af68b172118882a83d00e (). Then, since the competitor dockerfiles are the same for trusty and xenial now, I merged them in b3e52dfc591b9fc9826a58b90366cb95dea60fc1. Now we should be good to merge :wink: thanks!