feat(docker): provide modular images for openadkit planning simulator visualizer #4673

oguzkaganozt · 2024-04-30T14:10:44Z

Description

Provide modular planning, simulator and visualizer containers to enable easier development and deployment of Autoware functionalities with distinct needs.

Move monilithic autoware images into autoware package and, openadkit images into autoware-openadk package
Provide planning, simulator, visualizer container images
Provide docker-compose file for running autoware with multiple images easily

Detailed information can be found at the below related issue.

Tests performed

docker build action: https://github.com/autowarefoundation/autoware/actions/runs/9254434490
New docker images are thoroughly tested in a fresh environment.

Notes for reviewers

Interface changes

N/A as it will only introduce new modular container images.

Effects on system behavior

N/A to bare-metal setups, but it will provide new modular deployment scheme.

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.
The PR has been properly tested.
The PR has been reviewed by the code owners.

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.
The PR is ready for merge.

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

youtalk · 2024-05-07T08:34:29Z

@oguzkaganozt It is good PR but quite a big changes to review it.

To streamline the review process, please minimize the amount of revisions per PR and break it up into multiple PRs.
In addition, for both maintenance and build efficiency, a single Dockerfile should be used with multi-stage build instead of creating multiple Dockerfiles.
ref https://github.com/orgs/autowarefoundation/discussions/4661

oguzkaganozt · 2024-05-08T12:25:45Z

@oguzkaganozt It is good PR but quite a big changes to review it.

To streamline the review process, please minimize the amount of revisions per PR and break it up into multiple PRs. In addition, for both maintenance and build efficiency, a single Dockerfile should be used with multi-stage build instead of creating multiple Dockerfiles. ref https://github.com/orgs/autowarefoundation/discussions/4661

Thank you for the review @youtalk. You're right on big PR changes but since this PR includes many tightly-related changes I though that it will be more easier to review in all-in-one PR.

For the multi-stage build we are already utilizing it in the monolithic images of Autoware devel and runtime. And these new modules also utilizing the base image from the monolithic images (which includes ros, rmw and some basic dependencies) but we still need to keep module dockerfiles separate because ideally in the near future we will only copy related nodes into specific dockerfile, which means reducing the size of the modules even better. (At this time, this PR copying all of the install folder into all modules which is not ideal).

mitsudome-r · 2024-05-09T14:30:16Z

@oguzkaganozt Could you make modification to the documentation accordingly as well? I think the path for run scripts and build scripts must be changed on this page.

youtalk · 2024-05-20T03:56:53Z

I think the renaming refactoring and multi-stage build PRs should be done separately. If we do the renaming first, the diff would be smaller.

oguzkaganozt · 2024-05-22T13:50:50Z

@oguzkaganozt Could you make modification to the documentation accordingly as well? I think the path for run scripts and build scripts must be changed on this page.

@mitsudome-r we don't need to update any documentation for this PR anymore, as right now it is only adding openadkit directory which only contains modular images.

oguzkaganozt · 2024-05-22T14:21:06Z

I think the renaming refactoring and multi-stage build PRs should be done separately. If we do the renaming first, the diff would be smaller.

I know the changes look like a lot, but as I said this work is tightly coupled and includes the all work that previously demonstrated at the last CES. Creating separate PRs would be ideal but the time timeline is tight for these features to make users to replicate the last demo.

.github/actions/docker-build-and-push-openadkit/action.yaml

.github/workflows/docker-build-and-push-main.yaml

ansible/playbooks/openadkit.yaml

youtalk

I tried to review it, but the diff is too large to write review comments easily. Although it is divided into multiple images, it looks very wasteful because the same vcs import and rosdep install are repeated in various Dockerfiles. Also, the Dockerfiles other than the simulator are almost the same in content.

.github/workflows/docker-build-and-push-main-self-hosted.yaml

.github/workflows/docker-build-and-push-main.yaml

.github/actions/docker-build-and-push/action.yaml

docker/docker-bake.hcl

.github/workflows/docker-build-and-push-main-self-hosted.yaml

docker/build.sh

docker/openadkit/etc/ros_entrypoint.sh

docker/openadkit/modules/planning-control/Dockerfile

docker/run.sh

oguzkaganozt · 2024-05-24T09:40:30Z

I tried to review it, but the diff is too large to write review comments easily. Although it is divided into multiple images, it looks very wasteful because the same vcs import and rosdep install are repeated in various Dockerfiles. Also, the Dockerfiles other than the simulator are almost the same in content.

@youtalk at this stage of development my main focus was to just introduce these 3 different modules with demonstration of planning scenario. You're right that there are not much of diff between these dockerfiles but in the near future after defining the perception-localization container there will be.

Refining of each module's dockerfiles will be done on top of that, also we can leverage src-imported layer in the next PR of modular images. But at the time being intention was to upstream planning-control, simulator and visualizer containers and enable users to run planning scenarios with modular container setting with the help of this docker-compose file.

oguzkaganozt · 2024-05-24T15:21:47Z

Removed the upload artifacts section from the workflow because of the disk space after resolving the issue we can re-enable them. @youtalk @mitsudome-r

youtalk · 2024-05-25T02:21:39Z

#4771 reduces the image size. Please merge the latest main branch and check the CI again.

youtalk · 2024-05-26T00:58:54Z

Removed the upload artifacts section from the workflow because of the disk space after resolving the issue we can re-enable them. @youtalk @mitsudome-r

@oguzkaganozt You were saying the opposite #4738 (comment). So I think we can't remove the Upload Artifact.

Uploading of images is necessary because you can test out the produced images without pushing them into the registry.

oguzkaganozt · 2024-05-26T12:49:10Z

Removed the upload artifacts section from the workflow because of the disk space after resolving the issue we can re-enable them. @youtalk @mitsudome-r

@oguzkaganozt You were saying the opposite #4738 (comment). So I think we can't remove the Upload Artifact.

Uploading of images is necessary because you can test out the produced images without pushing them into the registry.

@youtalk Since it's stepping on our feet with various features I think we can re-enable them after we solve the disk space issue for now. I just don't want to slow down our development process for now, but eventually, we will need them soon.

Signed-off-by: Oguz Ozturk <[email protected]>

…anning-simulator-visualizer

oguzkaganozt · 2024-05-28T14:25:25Z

This PR includes too many changes and will take a lot of time to merge, so I have split out a subset PR for only renaming. #4785

Thanks @youtalk i really didn't have the time to do this as i am busy with demo prep. Now that I have merged main into this PR and the diff should be more human-readable :) @mitsudome-r

Signed-off-by: Oguz Ozturk <[email protected]>

youtalk · 2024-05-29T00:55:06Z

docker/Dockerfile

@@ -72,11 +72,9 @@ ENV CXX="/usr/lib/ccache/g++"
 # cspell: ignore libcu libnv
 # Set up development environment
 RUN --mount=type=ssh \
- ./setup-dev-env.sh -y --module all ${SETUP_ARGS} --no-cuda-drivers openadkit \
+ ./setup-dev-env.sh -y --module all ${SETUP_ARGS} --download-artifacts --no-cuda-drivers openadkit \


I think the change of #4771 was reverted. Do you have some reason?

We need to decrease the image size to re-enable the Upload Artifacts step.

youtalk · 2024-05-29T07:44:58Z

I have split out a subset PR for only disabling Upload Artifacts.
#4789

…anning-simulator-visualizer

youtalk · 2024-05-31T12:56:53Z

.github/actions/docker-build-and-push-openadkit/action.yaml

+ echo 'No artifacts uploaded because of disk space issue'
+ shell: bash
+
+ # TODO:Enable after solving the issue with the disk space


We are using the version control tool. We can revert the commit. That's why removing them is simple.

youtalk · 2024-05-31T12:58:27Z

.github/actions/docker-build-and-push/action.yaml

 set: |
 ${{ inputs.build-args }}

- - name: Build only
+ - name: Build Only


Please focus on the minimum change. If you want to refactor please make another PR.

Suggested change

- name: Build Only

- name: Build only

youtalk · 2024-05-31T12:59:35Z

.yamllint.yaml

@@ -10,6 +10,7 @@ rules:
 comments:
 level: error
 min-spaces-from-content: 1 # To be compatible with C++ and Python
+ comments-indentation: disable # Disable as it's sometimes fails to detect vscode auto-commenting https://github.com/adrienverge/yamllint/issues/384


Suggested change

comments-indentation: disable # Disable as it's sometimes fails to detect vscode auto-commenting https://github.com/adrienverge/yamllint/issues/384

youtalk · 2024-05-31T13:01:56Z

docker/Dockerfile

@@ -72,11 +72,9 @@ ENV CXX="/usr/lib/ccache/g++"
 # cspell: ignore libcu libnv
 # Set up development environment
 RUN --mount=type=ssh \
- ./setup-dev-env.sh -y --module all ${SETUP_ARGS} --no-cuda-drivers openadkit \
+ ./setup-dev-env.sh -y --module all ${SETUP_ARGS} --download-artifacts --no-cuda-drivers openadkit \


We need to decrease the image size to re-enable the Upload Artifacts step.

youtalk · 2024-05-31T13:03:23Z

docker/Dockerfile

- && find / -name 'libcu*.a' -delete \
- && find / -name 'libnv*.a' -delete


Why did you remove them?

youtalk · 2024-05-31T13:05:47Z

docker/build.sh

+ targets=()
 if [ "$option_devel_only" = "true" ]; then
 targets=("devel")
- else
- targets=()


Please focus on the minimum change. If you want to refactor please make another PR.

youtalk · 2024-05-31T13:06:14Z

docker/etc/ros_entrypoint.sh

@@ -22,7 +22,7 @@ else
 # Add sudo privileges to the user
 echo "$USER_NAME ALL=(ALL) NOPASSWD:ALL" >>/etc/sudoers

- # Source ROS2
+ # Source ROS 2


Please focus on the minimum change. If you want to refactor please make another PR.

Suggested change

# Source ROS 2

# Source ROS2

youtalk · 2024-05-31T13:12:06Z

@oguzkaganozt I understand that you need to use this for a demonstration, but as I've pointed out several times, this PR includes too many changes that should be separated.
Therefore, the PR can never be made merge-ready.

oguzkaganozt added component:system System design and integration. component:planning Route planning, decision-making, and navigation. DevOps Dojo: Build & Run type:ci Continuous Integration (CI) processes and testing. labels Apr 30, 2024

oguzkaganozt self-assigned this Apr 30, 2024

oguzkaganozt requested review from mitsudome-r and xmfcx as code owners April 30, 2024 14:10

oguzkaganozt mentioned this pull request May 17, 2024

fix: update docker installation autowarefoundation/autoware-documentation#550

Closed

4 tasks

oguzkaganozt force-pushed the 4538-provide-modular-images-for-openadkit-planning-simulator-visualizer branch from 9f002d5 to 6199c9c Compare May 20, 2024 11:35

oguzkaganozt force-pushed the 4538-provide-modular-images-for-openadkit-planning-simulator-visualizer branch from d998572 to 69b7859 Compare May 22, 2024 14:12

oguzkaganozt requested review from youtalk and TakaHoribe May 23, 2024 12:02

oguzkaganozt force-pushed the 4538-provide-modular-images-for-openadkit-planning-simulator-visualizer branch from 363229f to cbc9263 Compare May 23, 2024 13:15

mitsudome-r reviewed May 23, 2024

View reviewed changes

.github/actions/docker-build-and-push-openadkit/action.yaml Outdated Show resolved Hide resolved

.github/workflows/docker-build-and-push-main.yaml Show resolved Hide resolved

ansible/playbooks/openadkit.yaml Show resolved Hide resolved

youtalk reviewed May 24, 2024

View reviewed changes

.github/workflows/docker-build-and-push-main-self-hosted.yaml Outdated Show resolved Hide resolved

.github/workflows/docker-build-and-push-main.yaml Outdated Show resolved Hide resolved

mitsudome-r reviewed May 24, 2024

View reviewed changes

oguzkaganozt requested review from mitsudome-r and youtalk May 24, 2024 14:21

oguzkaganozt force-pushed the 4538-provide-modular-images-for-openadkit-planning-simulator-visualizer branch from 6c10f71 to 582a2c9 Compare May 27, 2024 11:04

pre-commit-ci bot and others added 15 commits May 28, 2024 12:04

style(pre-commit): autofix

fc237f8

fix #4765

6608bfe

Signed-off-by: Oguz Ozturk <[email protected]>

style(pre-commit): autofix

eb8b55a

Update from reviewers

e0d0a31

Signed-off-by: Oguz Ozturk <[email protected]>

remove upload artifacts because of the disk space issue

24d84f1

Signed-off-by: Oguz Ozturk <[email protected]>

style(pre-commit): autofix

43824c5

.

cf43ae9

Signed-off-by: Oguz Ozturk <[email protected]>

update yamllint settings

1705246

Signed-off-by: Oguz Ozturk <[email protected]>

Remove data_path as it is included in the image again

c32b00f

Signed-off-by: Oguz Ozturk <[email protected]>

download artifacts inside the dockerfile again

29d1a47

Signed-off-by: Oguz Ozturk <[email protected]>

.

83dad77

Signed-off-by: Oguz Ozturk <[email protected]>

fix playbook naming

6293ea2

Signed-off-by: Oguz Ozturk <[email protected]>

add caching

9ddc399

Signed-off-by: Oguz Ozturk <[email protected]>

remove base from build.sh

e6d49b5

Signed-off-by: Oguz Ozturk <[email protected]>

remove extra dash'-' from docker image tagging

d0a6d20

Signed-off-by: Oguz Ozturk <[email protected]>

oguzkaganozt force-pushed the 4538-provide-modular-images-for-openadkit-planning-simulator-visualizer branch from f4652bb to d0a6d20 Compare May 28, 2024 09:04

Merge branch 'main' into 4538-provide-modular-images-for-openadkit-pl…

573fbde

…anning-simulator-visualizer

oguzkaganozt marked this pull request as ready for review May 28, 2024 14:24

oguzkaganozt and others added 3 commits May 29, 2024 00:26

add clone repositories into build.sh

95f1bb5

Signed-off-by: Oguz Ozturk <[email protected]>

style(pre-commit): autofix

0b28c0e

.

ef2683a

Signed-off-by: Oguz Ozturk <[email protected]>

youtalk reviewed May 29, 2024

View reviewed changes

youtalk mentioned this pull request May 29, 2024

feat(ci): disable Upload Artifacts on docker-build-and-push action #4789

Merged

4 tasks

oguzkaganozt and others added 2 commits May 29, 2024 12:27

Merge branch 'main' into 4538-provide-modular-images-for-openadkit-pl…

deb0d40

…anning-simulator-visualizer

style(pre-commit): autofix

04477cf

youtalk reviewed May 31, 2024

View reviewed changes

oguzkaganozt mentioned this pull request Jun 10, 2024

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

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(docker): provide modular images for openadkit planning simulator visualizer #4673

feat(docker): provide modular images for openadkit planning simulator visualizer #4673

oguzkaganozt commented Apr 30, 2024 •

edited

youtalk commented May 7, 2024 •

edited

oguzkaganozt commented May 8, 2024

mitsudome-r commented May 9, 2024

youtalk commented May 20, 2024

oguzkaganozt commented May 22, 2024

oguzkaganozt commented May 22, 2024

youtalk left a comment

oguzkaganozt commented May 24, 2024

oguzkaganozt commented May 24, 2024

youtalk commented May 25, 2024

youtalk commented May 26, 2024

oguzkaganozt commented May 26, 2024

oguzkaganozt commented May 28, 2024

youtalk May 29, 2024

youtalk May 31, 2024

youtalk commented May 29, 2024 •

edited

youtalk May 31, 2024

youtalk May 31, 2024

youtalk May 31, 2024

youtalk May 31, 2024

youtalk May 31, 2024

youtalk May 31, 2024

youtalk May 31, 2024

youtalk commented May 31, 2024

		&& find / -name 'libcu*.a' -delete \
		&& find / -name 'libnv*.a' -delete

feat(docker): provide modular images for openadkit planning simulator visualizer #4673

Are you sure you want to change the base?

feat(docker): provide modular images for openadkit planning simulator visualizer #4673

Conversation

oguzkaganozt commented Apr 30, 2024 • edited

Description

Related links

Tests performed

Notes for reviewers

Interface changes

Effects on system behavior

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

youtalk commented May 7, 2024 • edited

oguzkaganozt commented May 8, 2024

mitsudome-r commented May 9, 2024

youtalk commented May 20, 2024

oguzkaganozt commented May 22, 2024

oguzkaganozt commented May 22, 2024

youtalk left a comment

Choose a reason for hiding this comment

oguzkaganozt commented May 24, 2024

oguzkaganozt commented May 24, 2024

youtalk commented May 25, 2024

youtalk commented May 26, 2024

oguzkaganozt commented May 26, 2024

oguzkaganozt commented May 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youtalk commented May 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youtalk commented May 31, 2024

oguzkaganozt commented Apr 30, 2024 •

edited

youtalk commented May 7, 2024 •

edited

youtalk commented May 29, 2024 •

edited