feat(docker): fix CUDA compile on devel image and improve run.sh #4849

oguzkaganozt · 2024-06-10T14:04:55Z

Description

This PR includes the first phase changes of this bulk PR

Revert this to download artifacts into container images to make them self-sufficient for direct running again
Add base image for modular containers
Fix of bug. Not removing CUDA and Tensorrt static libraries to enable building Autoware from scratch with CUDA libraries
Make run.sh more intuitive

Tests performed

https://github.com/autowarefoundation/autoware/actions/runs/9450305663

Effects on system behavior

Not applicable.

Interface changes

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

I've confirmed the contribution guidelines.
The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

The PR follows the pull request guidelines.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

oguzkaganozt · 2024-06-10T14:10:51Z

@youtalk @mitsudome-r This is the first phase of this PR

youtalk

It's even better than before! We can review it easier and faster.

Do you have an actual reason to re-add --download-artifacts at build time?
ref. #4771

oguzkaganozt · 2024-06-11T12:08:11Z

It's even better than before! We can review it easier and faster.

Do you have an actual reason to re-add --download-artifacts at build time? ref. #4771

I think we should keep our images as complete as possible. Since we don't have any disk usage problem anymore, we can re-add those artifacts to simplify container usage. Because the original intention was to save space.

And also think about a scenario in which user just downloads the image and go on a field test where there is no sufficient internet connection to download all artifacts. IMHO, devel and runtime images should be self-sufficient. Moreover these artifacts are mandatory to run essential parts of the Autoware(perception, localization) that means user will always need to download them so why not just keep them inside the image ?

oguzkaganozt · 2024-06-12T13:38:03Z

It's even better than before! We can review it easier and faster.
Do you have an actual reason to re-add --download-artifacts at build time? ref. #4771

I think we should keep our images as complete as possible. Since we don't have any disk usage problem anymore, we can re-add those artifacts to simplify container usage. Because the original intention was to save space.

And also think about a scenario in which user just downloads the image and go on a field test where there is no sufficient internet connection to download all artifacts. IMHO, devel and runtime images should be self-sufficient. Moreover these artifacts are mandatory to run essential parts of the Autoware(perception, localization) that means user will always need to download them so why not just keep them inside the image ?

@youtalk ?

youtalk · 2024-06-13T02:44:50Z

I'm sorry to reply late.
I understood. Then we need to first revert #4771. I made the PR #4864.
After merging #4864 and rebasing this PR, I will approve it.

youtalk · 2024-06-13T02:49:16Z

@oguzkaganozt In addition, I think we also need to change the PR title. The current title alone does not describe what it is trying to achieve.

docker/build.sh

youtalk

LGTM
9665082 is strange though.

Signed-off-by: Oguz <oguzkaganozt@gmail.com>

Co-authored-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>

Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com>

This reverts commit c66a1dd.

Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com>

docker/Dockerfile

ansible/playbooks/openadkit.yaml

docker/build.sh

isouf · 2024-06-16T08:19:35Z

docker/build.sh

 # Main script execution
 parse_arguments "$@"
 set_cuda_options
 set_build_options
 set_platform
 set_arch_lib_dir
 load_env
+clone_repositories


@oguzkaganozt may I please check with you what is the purpose of this step?
In the Dockerfile we are still copying the src that is under autoware not the src that is under autoware/docker.

Thank you for your review. I think it is a mistake and I fix it by #4894.

xmfcx · 2024-06-16T12:54:41Z

docker-build-and-push-main CI started failing after this was merged:

https://github.com/autowarefoundation/autoware/actions/runs/9516216183

ERROR: tag is needed when pushing to registry
Error: buildx bake failed with: ERROR: tag is needed when pushing to registry

https://github.com/autowarefoundation/autoware/actions/runs/9516216183/job/26232045461#step:8:1288

@oguzkaganozt

Probabable cause

It has passed the test on the initial PR description:

https://github.com/autowarefoundation/autoware/actions/runs/9450305663

Then, some tag related things were changed within the PR review process:
#4849 (comment)

To prevent such cases from happening, perform the tests again before merging.

Referenced in the issue:

docker-build-and-push-main is failing #4879

Let's track it there.

…owarefoundation#4849) * modular containers PR phase1 Signed-off-by: Oguz <oguzkaganozt@gmail.com> * style(pre-commit): autofix * add download artifacts to devel and runtime image Signed-off-by: Oguz <oguzkaganozt@gmail.com> * Update docker/build.sh Co-authored-by: Yutaka Kondo <yutaka.kondo@youtalk.jp> * style(pre-commit): autofix * build from temp source directory and remove dangling images Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com> * Revert "build from temp source directory and remove dangling images" This reverts commit c66a1dd. * build containers from temp source and remove dangling images Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com> * style(pre-commit): autofix * add vcs pull to update Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com> * style(pre-commit): autofix --------- Signed-off-by: Oguz <oguzkaganozt@gmail.com> Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>

oguzkaganozt requested a review from mitsudome-r June 10, 2024 14:04

oguzkaganozt self-assigned this Jun 10, 2024

oguzkaganozt requested a review from youtalk as a code owner June 10, 2024 14:04

oguzkaganozt force-pushed the modular-containers-phase1 branch from 72da2e2 to d382308 Compare June 10, 2024 14:11

youtalk reviewed Jun 10, 2024

View reviewed changes

oguzkaganozt requested a review from youtalk June 12, 2024 13:37

youtalk mentioned this pull request Jun 13, 2024

feat(docker,ansible): revert "feat(docker,ansible): download artifacts at runtime (#4771)" #4864

Merged

oguzkaganozt changed the title ~~feat(docker): first phase of the modular containers PR~~ feat(docker): fix CUDA compile on devel image and improve run.sh Jun 13, 2024

youtalk reviewed Jun 13, 2024

View reviewed changes

docker/build.sh Outdated Show resolved Hide resolved

docker/build.sh Show resolved Hide resolved

youtalk approved these changes Jun 14, 2024

View reviewed changes

oguzkaganozt force-pushed the modular-containers-phase1 branch from f4d0bd0 to 873fe8a Compare June 14, 2024 10:03

oguzkaganozt requested a review from xmfcx as a code owner June 14, 2024 10:06

oguzkaganozt and others added 9 commits June 14, 2024 14:37

modular containers PR phase1

1b0fb74

Signed-off-by: Oguz <oguzkaganozt@gmail.com>

style(pre-commit): autofix

1c5ad2d

add download artifacts to devel and runtime image

acf3559

Signed-off-by: Oguz <oguzkaganozt@gmail.com>

Update docker/build.sh

0689973

Co-authored-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>

style(pre-commit): autofix

50554b5

build from temp source directory and remove dangling images

de669e2

Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com>

Revert "build from temp source directory and remove dangling images"

ad365ee

This reverts commit c66a1dd.

build containers from temp source and remove dangling images

a8d3b6c

Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com>

style(pre-commit): autofix

cf8405e

oguzkaganozt force-pushed the modular-containers-phase1 branch from 3920e0a to cf8405e Compare June 14, 2024 11:39

oguzkaganozt and others added 2 commits June 14, 2024 14:43

add vcs pull to update

895bd5c

Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com>

style(pre-commit): autofix

7696467

oguzkaganozt commented Jun 14, 2024

View reviewed changes

docker/Dockerfile Show resolved Hide resolved

docker/Dockerfile Outdated Show resolved Hide resolved

ansible/playbooks/openadkit.yaml Show resolved Hide resolved

docker/build.sh Show resolved Hide resolved

oguzkaganozt merged commit 4e7f539 into main Jun 14, 2024
12 of 13 checks passed

oguzkaganozt deleted the modular-containers-phase1 branch June 14, 2024 12:35

tier4-autoware-public-bot bot mentioned this pull request Jun 15, 2024

chore: sync upstream tier4/autoware#5

Open

isouf reviewed Jun 16, 2024

View reviewed changes

This was referenced Jun 17, 2024

docker-build-and-push-main is failing #4879

Closed

ci(docker-build-and-push): add base tag to fix failure #4886

Merged

Docker Image OpenADK with CUDA support has an issue with CUDA installation #4765

Closed

youtalk mentioned this pull request Jun 20, 2024

fix(docker): fix the src directory in docker/build.sh #4894

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(docker): fix CUDA compile on devel image and improve run.sh #4849

feat(docker): fix CUDA compile on devel image and improve run.sh #4849

oguzkaganozt commented Jun 10, 2024 •

edited

Loading

oguzkaganozt commented Jun 10, 2024

youtalk left a comment

oguzkaganozt commented Jun 11, 2024

oguzkaganozt commented Jun 12, 2024

youtalk commented Jun 13, 2024

youtalk commented Jun 13, 2024

youtalk left a comment

isouf Jun 16, 2024

youtalk Jun 20, 2024

xmfcx commented Jun 16, 2024 •

edited

Loading

feat(docker): fix CUDA compile on devel image and improve run.sh #4849

feat(docker): fix CUDA compile on devel image and improve run.sh #4849

Conversation

oguzkaganozt commented Jun 10, 2024 • edited Loading

Description

Tests performed

Effects on system behavior

Interface changes

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

oguzkaganozt commented Jun 10, 2024

youtalk left a comment

Choose a reason for hiding this comment

oguzkaganozt commented Jun 11, 2024

oguzkaganozt commented Jun 12, 2024

youtalk commented Jun 13, 2024

youtalk commented Jun 13, 2024

youtalk left a comment

Choose a reason for hiding this comment

isouf Jun 16, 2024

Choose a reason for hiding this comment

youtalk Jun 20, 2024

Choose a reason for hiding this comment

xmfcx commented Jun 16, 2024 • edited Loading

Probabable cause

oguzkaganozt commented Jun 10, 2024 •

edited

Loading

xmfcx commented Jun 16, 2024 •

edited

Loading