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): copy src by --mount=type=bind instead of COPY #5027

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

youtalk
Copy link
Member

@youtalk youtalk commented Jul 24, 2024

Description

Following the discussion https://github.com/orgs/autowarefoundation/discussions/5007#discussioncomment-10126509, this PR changes not to include src in the autoware-universe image, and only include it in the devel image.
However, due to the effects of previous PR #5009, the amount of src copied has already been minimized, resulting in a reduction of only 18.69MB in the image size.

@xmfcx As a result, I think it would be convenient to keep src copied in the autoware-universe image and not merge this PR, as it would make using the autoware-universe as a development container easier.

Tests performed

Before this PR merged:

After this PR merged:

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.

@youtalk youtalk self-assigned this Jul 24, 2024
@youtalk youtalk marked this pull request as ready for review July 24, 2024 02:28
@youtalk youtalk requested a review from oguzkaganozt as a code owner July 24, 2024 02:28
@youtalk youtalk requested review from xmfcx and mitsudome-r July 24, 2024 02:28
@youtalk youtalk added type:containers Docker containers, containerization of components, or container orchestration. component:openadkit Issues or Features related to Open AD Kit tag:run-health-check Run health-check labels Jul 24, 2024
@xmfcx
Copy link
Contributor

xmfcx commented Jul 24, 2024

That size might be compressed.

For me, in the fresh autoware folder:

# build, install, log files are removed, src is empty.
$ vcs import src < autoware.repos
$ du -ch --exclude='*/.git*' . | grep 'total$'
343M	total

$vcs import src < tools.repos
$ du -ch --exclude='*/.git*' . | grep 'total$'
390M	total

To see if it was the compression, tested:

$ tar --exclude='*/.git' -czf X.tar.gz src
$ du -h X.tar.gz
199M	X.tar.gz

I'm not sure how this translated to 18.69MB change in image size.

Also rather than the image size, the actual disk space usage when the container is running is more important for the CI use case.

In general, I don't know how to feel about the autoware source files in the container, especially without the version control. Only developers would need the autoware source folder and they would like to use it with version control.


Edit:
Trying with:

du -ch --exclude='*/.git*' --exclude='*.png' --exclude='*.svg' --exclude='*.pcd'  --exclude='*.csv'  --exclude='*.gif'  --exclude='*.md'  --exclude='*.asc'  --exclude='*/.*' src

still I see 104MB usage.

Edit 2:

$ tar --exclude='*.git*' --exclude='*.png' --exclude='*.svg' --exclude='*.pcd' --exclude='*.csv' --exclude='*.gif' --exclude='*.md' --exclude='*.asc' --exclude='*/.*' -czvf archive.tar.gz -C src .

$ du -h archive.tar.gz 
19M	archive.tar.gz

Ok this is probably it then.

@youtalk
Copy link
Member Author

youtalk commented Jul 25, 2024

@xmfcx If you think so, please approve it.

Copy link
Member

@mitsudome-r mitsudome-r left a comment

Choose a reason for hiding this comment

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

At least the change looks reasonable to me. This should reduce about 100MB from the CI image.

@youtalk youtalk merged commit ede026d into main Jul 26, 2024
33 checks passed
@youtalk youtalk deleted the upstream-src-bind-mount branch July 26, 2024 06:39
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:containers Docker containers, containerization of components, or container orchestration.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants