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): rename multi-stages #5170

Merged
merged 3 commits into from
Sep 5, 2024
Merged

Conversation

youtalk
Copy link
Member

@youtalk youtalk commented Sep 5, 2024

Description

As mentioned in this opinion #5164 (review) by @oguzkaganozt, the Dockerfile is becoming increasingly complex for the following reasons:

  • It's unclear from the stage names which stage is the development container and which stage is the runtime container.
  • Since the image name is "autoware," adding "autoware" to the stage names (tag names) is redundant.

Therefore, this PR introduces the following naming conventions to make it easier to maintain a one-to-one correspondence between development and runtime containers, even as multi-container setups become more prevalent:

  • Containers with the suffix -devel are development containers, while those without the suffix are runtime containers.
  • The prefix autoware- is no longer used.

The generated images will have names like the following.

Before After
autoware:latest-base autoware:latest-base
autoware:latest-autoware-core autoware:latest-core-devel
autoware:latest-autoware-universe autoware:latest-universe-devel
autoware:latest-devel autoware:latest-devel
autoware:latest-runtime autoware:latest-universe

Before:
Dockerfile
After:
Dockerfile

Tests performed

youtalk#102
https://github.com/youtalk/autoware/actions/runs/10712585980

Effects on system behavior

Documentation, shell scripts, and the GitHub Actions settings in the autoware.universe repository also need to be updated.

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>
@youtalk youtalk self-assigned this Sep 5, 2024
@youtalk youtalk added type:containers Docker containers, containerization of components, or container orchestration. component:openadkit Issues or Features related to Open AD Kit labels Sep 5, 2024
@youtalk youtalk changed the title Upstream rename multi stages only feat(docker): rename multi stages Sep 5, 2024
@youtalk youtalk added the tag:run-health-check Run health-check label Sep 5, 2024
@youtalk youtalk marked this pull request as ready for review September 5, 2024 05:21
@youtalk youtalk changed the title feat(docker): rename multi stages feat(docker): rename multi-stages Sep 5, 2024
@youtalk youtalk merged commit e3fa138 into main Sep 5, 2024
30 of 31 checks passed
@youtalk youtalk deleted the upstream-rename-multi-stages-only branch September 5, 2024 06:09
@xmfcx
Copy link
Contributor

xmfcx commented Sep 5, 2024

@xmfcx
Copy link
Contributor

xmfcx commented Sep 5, 2024

@youtalk -san, I think the latest keyword is also becoming redundant, what do you think of removing it?
Is there a benefit in keeping the latest keyword?

Before After
autoware:latest-base autoware:base
autoware:latest-core-devel autoware:core-devel
autoware:latest-universe-devel autoware:universe-devel
autoware:latest-devel autoware:devel
autoware:latest-universe autoware:universe

Referred in issue:

@kaspermeck-arm
Copy link

@youtalk -san, I think the latest keyword is also becoming redundant, what do you think of removing it? Is there a benefit in keeping the latest keyword?

Before After
autoware:latest-base autoware:base
autoware:latest-core-devel autoware:core-devel
autoware:latest-universe-devel autoware:universe-devel
autoware:latest-devel autoware:devel
autoware:latest-universe autoware:universe
Referred in issue:

If we remove the latest tag, how would we deal with timestamped tagging, e.g., autoware:devel-2024Q4?

@xmfcx
Copy link
Contributor

xmfcx commented Sep 5, 2024

If it has timestamp, it can have the timestamp in the tag. If it doesn't have a timestamp (it is latest) it will be without latest keyword.

@oguzkaganozt
Copy link
Contributor

Whenever user pulls an image, say autoware:universe it would always pull the relevant latest image anyways because we are overwriting it with the newest one on each push. In that manner we do not need to have latest tag really.

On the other hand latest tag should be unique and only be used for the one image not for all other alternative image types.
And for our case it would be the universe(monolithic runtime image) for the autoware, so like autoware:latest would be equal to latest autoware:universe always.

@oguzkaganozt
Copy link
Contributor

@youtalk I think we should remove the latest tag as we finalized monolithic images, we are already overwriting core and universe images with the same name anyways.

@xmfcx thanks for the heads up !

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.

5 participants