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): push autoware image on "push to main branch" and "push tag" events #4993

Merged
merged 10 commits into from
Jul 18, 2024

Conversation

youtalk
Copy link
Member

@youtalk youtalk commented Jul 16, 2024

Description

Under the current GitHub Actions' workflow configuration, the autoware images are only pushed at the following times:

  1. On the openadkit-v*.*.* tag pushed
  2. On the 1st and 15th of each month
  3. On a workflow_dispatch with registory option dispatched

Since 1 and 3 have nerver been executed so far, images have actually only been pushed regularly on the 1st and 15th of each month. However, there is no particular significance to the 1st and 15th of each month.
In order to advance the version control and proceed with container development and operation, it is necessary to always be able to pull the latest container images.

Therefore, this PR revises the image push timings as follows:

  1. On the main branch pushed (i.e., on a PR merged)
  2. On a tag pushed
  3. On a workflow_dispatch dispatched

I would like to propose a revision of the naming conventions for the Docker images and tags in a discussions and a separate PR later.

Tests performed

Not applicable.

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 <[email protected]>
Signed-off-by: Yutaka Kondo <[email protected]>
Signed-off-by: Yutaka Kondo <[email protected]>
Signed-off-by: Yutaka Kondo <[email protected]>
This reverts commit 5f3b139.
Signed-off-by: Yutaka Kondo <[email protected]>
@youtalk youtalk self-assigned this Jul 16, 2024
@youtalk youtalk added type:ci Continuous Integration (CI) processes and testing. component:openadkit Issues or Features related to Open AD Kit labels Jul 16, 2024
@youtalk youtalk marked this pull request as ready for review July 16, 2024 02:55
@youtalk youtalk added the tag:run-health-check Run health-check label Jul 16, 2024
@youtalk
Copy link
Member Author

youtalk commented Jul 16, 2024

I would like to propose a revision of the naming conventions for the Docker images and tags in a discussions and a separate PR later.

I've submitted a Discussions post about the tag naming convention. https://github.com/orgs/autowarefoundation/discussions/4995

xmfcx
xmfcx previously approved these changes Jul 16, 2024
@xmfcx xmfcx dismissed their stale review July 16, 2024 10:34

approved wrong pr sorry

@xmfcx
Copy link
Contributor

xmfcx commented Jul 16, 2024

When do we need new images?

  • When ansible script changes
  • When the dockerfile changes
  • When rosdep dependencies of Autoware change
  • When dependent (ROS or not) apt packages are updated
  • When the content of autoware.repos or tools.repos? (I dont know if this is part of the docker image) changes

On the 1st and 15th of each month.

This was to loosely handle these updates, with a twice a month schedule.
Unless we detect the changes from autoware packages, or the dockerfile/ansible updates, it might be better to do this on a schedule. Maybe more often, even daily.

ROS 2 has nightly images: https://hub.docker.com/layers/osrf/ros2/nightly/images/sha256-1f29100323ee91dbc90ed248862c408d67ceb9580757bc9c94005de0c2144fef?context=explore

It seems they generate these dockerfilesi with this create_dockerfiles.py script explained here.

On the main branch pushed (i.e., on a PR merged)

If we don't check for changes made in:

  • ansible setup procedure
  • dockerfiles
  • x.repos
  • x.env files,
    this is not good. Any random documentation update or ci push will trigger a new image build, completely detached from any meaningful trigger.

On a tag pushed

I'm ok with keeping this.

workflow_dispatch

This is for testing, why remove it?

@xmfcx
Copy link
Contributor

xmfcx commented Jul 16, 2024

Directly related issue and PR regarding to the following being removed:

 on:
  push:
    branches:
      - main

@xmfcx
Copy link
Contributor

xmfcx commented Jul 16, 2024

https://github.com/marketplace/actions/changed-files

This can be used to detect changes to certain files within this repository.

@youtalk youtalk changed the title feat(docker,ci): push autoware image on push to main branch and push tag events feat(docker,ci): push autoware image on push to main branch" and "push tag" events Jul 16, 2024
@youtalk youtalk changed the title feat(docker,ci): push autoware image on push to main branch" and "push tag" events feat(docker,ci): push autoware image on "push to main branch" and "push tag" events Jul 16, 2024
@youtalk
Copy link
Member Author

youtalk commented Jul 16, 2024

@xmfcx

workflow_dispatch

This is for testing, why remove it?

I'm sorry but I just forgot the listing and have fixed the PR description #4993 (comment).

I'm trying to modify the workflow based on your suggestion. Please wait a while.

@youtalk youtalk marked this pull request as draft July 16, 2024 12:16
@youtalk youtalk marked this pull request as ready for review July 17, 2024 07:33
@youtalk
Copy link
Member Author

youtalk commented Jul 17, 2024

I have modified this PR so that the build is only executed when there are changes to the corresponding files using changed-files action.

I've checked that the Build 'Autoware' was skipped in my forked workflow when there was no change https://github.com/youtalk/autoware/actions/runs/9969947469/job/27547864413.

@xmfcx Please review again.

@youtalk youtalk requested a review from xmfcx July 17, 2024 07:36
Signed-off-by: Yutaka Kondo <[email protected]>
@xmfcx
Copy link
Contributor

xmfcx commented Jul 17, 2024

@youtalk -san, what do you think about nightly builds?

Rest of the changes look good 👍 Thank you.

@oguzkaganozt
Copy link
Contributor

I think we need to re-enable the artifact saving option as tar files as soon as possible on the workflow_dispatch otherwise LGTM. Thanks !

@youtalk
Copy link
Member Author

youtalk commented Jul 18, 2024

@xmfcx nightly is seems to be better than latest but we need to discuss first on https://github.com/orgs/autowarefoundation/discussions/4995.

@youtalk youtalk merged commit ed46a7a into main Jul 18, 2024
17 checks passed
@youtalk youtalk deleted the push-image-on-push branch July 18, 2024 12:58
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