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

docker: update Dockerfiles to use focal images #388

Merged
merged 1 commit into from
Oct 6, 2020
Merged

docker: update Dockerfiles to use focal images #388

merged 1 commit into from
Oct 6, 2020

Conversation

russkel
Copy link
Contributor

@russkel russkel commented Sep 29, 2020

To close #382

  • use focal base images instead of bionic, including using newer version of libglvnd
  • runtime=nvidia is deprecated, gpus=all has replaced it in newer
    versions of docker

This has not been built or tested by me.

@russkel russkel requested a review from chapulina as a code owner September 29, 2020 11:11
Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

I tried the changes using the new Dome release. Everything seems fine except that the ignition-dome package is not yet released, should be ready tomorrow. Let's wait 48 hours to give this a final run. Thanks @russkel for the changes!

@@ -154,7 +154,7 @@ Docker has two available versions: Community Edition (CE) and Enterprise Edition

1. Verify the installation:

docker run --runtime=nvidia --rm nvidia/cuda nvidia-smi
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand it correctly, this change is not strictly related to the update to focal but more about the version of docker in the user system. I think we can update it as you did in the PR since modern docker uses the native nvidia integration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is correct. I decided to update to reflect the docs found at https://docs.nvidia.com/datacenter/cloud-native/container-toolkit/install-guide.html#docker to hopefully reduce confusion.

I'm not too familiar with what is actually going on in docker/nvidia-runtime internals so maybe this change in CLI arguments is required.

@chapulina chapulina added the 🔮 dome Ignition Dome label Sep 29, 2020
@chapulina chapulina changed the base branch from master to main October 3, 2020 01:29
@chapulina chapulina removed the 🔮 dome Ignition Dome label Oct 3, 2020
@chapulina chapulina changed the base branch from main to ign-gazebo4 October 3, 2020 01:31
@chapulina chapulina added the 🔮 dome Ignition Dome label Oct 3, 2020
@j-rivero
Copy link
Contributor

j-rivero commented Oct 5, 2020

Everything seems fine except that the ignition-dome package is not yet released, should be ready tomorrow

@russkel now that dome in released, we are fine to merge these changes. I would need a couple of things from you: could you please rebase/merge the changes on ign-gazebo4 with your branch? While doing that we need to address the DCO bot failure in the CI, there are instrcutions in the "Details" link https://github.com/ignitionrobotics/ign-gazebo/pull/388/checks?check_run_id=1181732463?

* runtime=nvidia is deprecated, gpus=all has replaced it in newer
  versions of docker

Signed-off-by: Russ Webber <[email protected]>
@russkel
Copy link
Contributor Author

russkel commented Oct 5, 2020

Hi @j-rivero, done as you have asked.

Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

thanks you very much! ready to go.

@j-rivero j-rivero merged commit 94560a2 into gazebosim:ign-gazebo4 Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker images still on bionic, old versions of glvnd don't seem to work with latest versions of nvidia-docker2
3 participants