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

Separate focal and jammy carma-base Dockerfiles #191

Merged
merged 28 commits into from
Oct 9, 2024

Conversation

MishkaMN
Copy link
Contributor

@MishkaMN MishkaMN commented Oct 4, 2024

PR Details

Description

After Humble migration, carma will be running noetic/humble hybrid system.
However, humble requires ubuntu 22.04, which is different than noetic/foxy, so a separate Dockerfile is needed from here on out.
Please see the docker deployment design here

Therefore, this PR creates separates the two OS by their names "focal" and "jammy", which lets the build-image.sh script to build two different images. New usage:
build-image.sh --jammy OR
build-image.sh --focal OR
build-image.sh no option to build both.

Resulting images will have an extra trailing name in their tag distinguising their names such as:
usdotfhwastol/carma-base:latest-jammy

Github Action is modifed to also check docker build and dockerhub push wiht distinctive trailing suffix for each OS at the same time.

NOTE: because humble is not officialy merged to develop yet, focal is still the main docker images. So we will not use the trailing tag for focal until everything is regression tested and merged: https://usdot-carma.atlassian.net/browse/ARC-227. So currently build-image.sh --focal will still build following:
usdotfhwastol/carma-base:latest
same for the dockerhub image tag as well where focal is not tagged at the moment.

In terms of contents in the respective Dockerfile, focal stayed the same to continue supporting foxy/noetic until humble migration is fully done.
Jammy Dockerfile contains the mirror upgrades of foxy and several universal dependencies generated by Generative AI.

If any dependency is missing, we can add them as we go, but this sould be enough for now.

Related GitHub Issue

NA

Related Jira Key

https://usdot-carma.atlassian.net/browse/ARC-155

Motivation and Context

From Foxy to Humble upgrade

How Has This Been Tested?

Types of changes

  • Defect fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that cause existing functionality to change)

Checklist:

  • I have added any new packages to the sonar-scanner.properties file
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@MishkaMN MishkaMN changed the title Separate focal and jammy [WIP] Separate focal and jammy Oct 4, 2024
  docker:
    uses: usdot-fhwa-stol/actions/.github/workflows/docker.yml@main
    file: ./jammy/Dockerfile
@MishkaMN MishkaMN marked this pull request as ready for review October 4, 2024 22:22
@MishkaMN MishkaMN changed the title [WIP] Separate focal and jammy Separate focal and jammy carma-base Dockerfiles Oct 5, 2024
@MishkaMN MishkaMN self-assigned this Oct 5, 2024
@MishkaMN MishkaMN added the enhancement New feature or request label Oct 5, 2024
TAGS+=("$USERNAME/$IMAGE:latest")
cd ..

# If neither --focal nor --jammy is specified, build both
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are currently just starting to update the systems to include jammy, it would make more sense to me for this to default to just building focal. Open to discussion on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. I will turn it off by default.

adev4a
adev4a previously approved these changes Oct 8, 2024
# Jammy 22.04 (ROS2 Humble)
docker-build-jammy:
uses: usdot-fhwa-stol/actions/.github/workflows/docker.yml@main
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the two jobs triggered when changes are pushed ? May be can we use this path filters (https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#onpushpull_requestpaths) to only run specific job that is required when its associated files changed instead of running multiple jobs every time ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes great point I forgot. I tried adding those. I had to separate the yml files to write it cleanly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested the change by creating a dummy PR here that points to this branch. The dummy PR only has jammy changes, which successfully only triggered jammy docker: https://github.com/usdot-fhwa-stol/carma-base/actions/runs/11247485262/job/31271050955?pr=192

# TODO, distinguish with suffix when Humble is fully integrated
# until then focal will have no suffix and be the main image
# https://usdot-carma.atlassian.net/browse/ARC-227
if [ "$BUILD_FOCAL" = true ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add echo "Building Focal Image..." and echo "Building Jammy Image..." some thing like that which is easy to debug during CI for each focal and jammy ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

COMMIT_MSG=$(git log -1 --pretty=%B)

# Remove newlines and special characters, replace spaces with underscores
VERSION=$(echo "$COMMIT_MSG" | tr -d '\n' | tr -dc '[:alnum:][:space:]' | tr '[:space:]' '_')
Copy link
Contributor

Choose a reason for hiding this comment

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

so the version is generated from the commit message, which works well for general versioning should we consider appending a suffix like -focal or -jammy to the VERSION variable depending on which image is being built (focal or jammy)?
some thing like VERSION="${VERSION}-focal" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The suffix is appended outside this function through variable called $tag_suffix. So it should be good.

@MishkaMN MishkaMN merged commit 12d4547 into develop Oct 9, 2024
2 checks passed
@MishkaMN MishkaMN deleted the arc-155-support-jammy branch October 9, 2024 13:16
@MishkaMN MishkaMN mentioned this pull request Oct 9, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants