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

feat(docker,ci): split prebuilt stage into autoware-core and autoware-universe stages #4961

Merged
merged 11 commits into from
Jul 16, 2024

Conversation

youtalk
Copy link
Member

@youtalk youtalk commented Jul 10, 2024

Description

This PR is the first step in this discussion https://github.com/orgs/autowarefoundation/discussions/4661#discussioncomment-9995806.
The Dockerfile's prebuilt stage will be divided into two stages: autoware-core and autoware-universe.
Additionally, the src-imported stage, which was prepared for vcs import, has now been renamed to rosdep-depend because src is no longer imported via vcs import but simply called COPY.

The current Dockerfile

Screenshot from 2024-07-09 16-50-14

This PR's Dockerfile

Screenshot from 2024-07-10 14-18-54

Tests performed

https://github.com/youtalk/autoware/actions/runs/9868168693

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.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

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.

Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
@youtalk youtalk marked this pull request as ready for review July 10, 2024 05:24
@youtalk youtalk self-assigned this Jul 10, 2024
@mitsudome-r
Copy link
Member

It seems like some of the workflows in Autoware.Universe uses prebuilt image. If we are removing them, we should update Autoware.Universe CIs to use devel environment instead.
https://github.com/autowarefoundation/autoware.universe/blob/ea0276acd134a75bc0227aeeb96266c26db7100e/.github/workflows/build-and-test-differential.yaml#L36

@youtalk
Copy link
Member Author

youtalk commented Jul 10, 2024

@mitsudome-r That's good catch! Since latest-auware-universe tag will be replaced with the equivalent latest-prebuilt tag, I plan to submit a correction PR to https://github.com/autowarefoundation/autoware.universe as soon as this PR is merged and the image is pushed.

@youtalk youtalk added type:ci Continuous Integration (CI) processes and testing. component:openadkit Issues or Features related to Open AD Kit labels Jul 10, 2024
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
@youtalk youtalk changed the title feat(docker,ci): split prebuilt stage into autoware-core and autoware-universe feat(docker,ci): split prebuilt stage into autoware-core and autoware-universe stages Jul 10, 2024
@youtalk
Copy link
Member Author

youtalk commented Jul 10, 2024

I made a PR to resolve the autoware.universe's issue autowarefoundation/autoware.universe#7952.
Once this PR is merged and the image is pushed, I will change the PR to Ready for review.
@mitsudome-r Please review again.

@youtalk youtalk added the tag:run-health-check Run health-check label Jul 11, 2024
@youtalk
Copy link
Member Author

youtalk commented Jul 12, 2024

I'm sorry but I sent the message to the different PR.

#4975 (comment)

@xmfcx @oguzkaganozt @mitsudome-r Thank you for the discussion on the multi-containerization yesterday. Here is the PR for the first step. Please review it.

@xmfcx
Copy link
Contributor

xmfcx commented Jul 12, 2024

@youtalk thanks for the updates.

Could you add the following directly into the autoware/docker/README.md file:

Example mermaid link

## Images

Images diagram (in mermaid format please)

### base

- imports from X
- used for X
- contains X

### rosdep-depend

...

So we can understand what's happening easily.

@youtalk
Copy link
Member Author

youtalk commented Jul 16, 2024

@xmfcx It seems there is no de facto standard syntax for writing multi-stage build Dockerfile in Mermaid format. Do you want me to convert something equivalent to https://github.com/patrickhoefler/dockerfilegraph to Mermaid format on my own?

@youtalk
Copy link
Member Author

youtalk commented Jul 16, 2024

@xmfcx Is it okay to separate the README update and the Dockerfile modification into different PRs? I think the discussion about the format of the README and the discussion about the design of the Dockerfile's multi-stage build are separate issues.

@xmfcx
Copy link
Contributor

xmfcx commented Jul 16, 2024

@youtalk I see, I didn't know it was generated by dockerfilegraph, which uses graphviz, same as what colcon uses to visualize the package dependency graph!

https://github.com/patrickhoefler/dockerfilegraph?tab=readme-ov-file#more-options

Here it has -o svg option, which should output a svg to be version control friendly, we can use that.

Is it okay to separate the README update and the Dockerfile modification into different PRs? I think the discussion about the format of the README and the discussion about the design of the Dockerfile's multi-stage build are separate issues.

That's alright, as long as we have the documentation in sync with the dockerfile.
Right now it is a little bit messy, people need to read through the dockerfile to know what stands for what.

@xmfcx
Copy link
Contributor

xmfcx commented Jul 16, 2024

By the way, I need something like this:
https://github.com/osrf/docker_images/tree/master?tab=readme-ov-file#repo-info-3

@youtalk youtalk merged commit 45c3a78 into main Jul 16, 2024
22 checks passed
@youtalk youtalk deleted the upstream-autoware-core-stage branch July 16, 2024 12:06
@youtalk
Copy link
Member Author

youtalk commented Jul 16, 2024

@xmfcx I will make a PR to write the README. Thank you for you understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:openadkit Issues or Features related to Open AD Kit tag:run-health-check Run health-check type:ci Continuous Integration (CI) processes and testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants