Modify CI to track ros testing repo #256

CihatAltiparmak · 2024-08-03T00:37:12Z

To prevent the errors stated in this PR ( #255 ) , i have refactored the build CI so that it can be tracked the modifications, API changes etc. for rolling, jazzy and iron continuously. I don't know what type of approach this team has, however i wanted to come up with some option. It's ready to your service. It doesn't matter if you accepted or not. Btw to just modify container except this PR's idea can be another option. I can do it too.

…ng repos

MichaelOrlov · 2024-08-15T19:53:38Z

@Yadunund @clalancette Could you please review this PR?

Yadunund

Thanks for kicking off this initiative!

I feel that the using binaries from the testing repo may not highlight issues soon enough. It relies on someone blooming changes from the core repos and then waiting for the rosdistro PR to be merged to kick-off the build jobs.

On the other hand we expect core packages for released distros like jazzy, iron etc to be fairly stable so we don't need to test with the latest source code or binaries from the test repo.

What do you think of the following:

Update the build.yaml workflow to rely on setup-ros.
- If we're testing for rolling, the job will build --packages-up-to rmw_zenoh_cpp from source.
- Else, we configure setup-ros to use regular ROS 2 core binaries and build upto rmw_zenoh_cpp
Configure the build job to run on a nightly schedule using a cron config. This way the maintainers will be notified if any breaking changes get merged in core upstream repos.

clalancette · 2024-08-21T18:04:50Z

Agreed with what @Yadunund laid out above.

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

ahcorde · 2024-08-22T10:56:28Z

I made the changes @Yadunund

source build for rolling build / build_and_test_source_rolling
binary build for jazzy build / build_and_test_binaries_jazzy
binary build for iron build / build_and_test_binaries_iron
Added cron too

ahcorde · 2024-08-22T11:01:31Z

rolling is failing because of the localhost_only issue #255

…ld_ci_with_industrial_ci

Yadunund

Thanks @ahcorde for the changes. Left a comment to reuse the same job across distros/os.

Yadunund · 2024-08-22T20:05:42Z

.github/workflows/build.yaml

 defaults:
  run:
    shell: bash
 jobs:
-  test:
+  build_and_test_source_rolling:


Instead of having three separate jobs like this, can we retain the distro matrix from earlier and setup one job only to run for each distro in the matrix? Within this job, we can have a special if condition to check if distro == rolling and if so, build from source. If you need access to both the ROS distro version and ubuntu version, the matrix can be defined like so.

Also it appears that your changes now pull a rostooling/setup-ros-docker:ubuntu-x image. How often is this image updated? And why not use ros-tooling/[email protected] to setup from latest ubuntu base image like so.

Here is an example of using an if condition within the action: https://github.com/open-rmf/rmf_ci_templates/blob/1ae7b4f64644ab2f8724e9a1e1b3d14803caa61e/.github/workflows/reusable_build.yaml#L53

Signed-off-by: CihatAltiparmak <[email protected]>

Yadunund · 2024-08-22T21:05:22Z

.github/workflows/build.yaml

+          ROS_DISTRO: [rolling, jazzy, iron]
+      runs-on: ubuntu-latest
+      container:
+        image: rostooling/setup-ros-docker:ubuntu-noble-latest


We can't alwayse use noble since iron is distributed on jammy. Please see my previous comment on how the matrix can be updated to include distro and os. #256 (comment)

Not addressed form the same comment:

Using If condition to include vcs-repo-file-url

Using ros-tooling/[email protected] instead of the rostooling/setup-ros-docker:ubuntu image

Firstly sorry for some commits, but would using separate jobs for source and binaries be good instead of using if? I am yet to complete my work. I will ping you.

I'd prefer to have everything in the same job. The CI here is a temporary measure until we start running the usual CI jobs using the ROS buildfarm.

I've applied, @Yadunund and i've readded the fail-fast param which @ahcorde has overlooked by deleting this. Would you review in your available time? (Inspired by https://github.com/ros-tooling/setup-ros/blob/a6ce30ecca1e5dcc10ae5e6a44fe2169115bf852/README.md?plain=1#L249-L252 and https://github.com/ros2/ros2_tracing/blob/a866b9c701311bc8200a00c949b4a0fff9803777/.github/workflows/test.yml#L68) Btw if you want, we can merge rolling branch to this branch so that all CI can pass.

thank you for the changes @CihatAltiparmak

Signed-off-by: CihatAltiparmak <[email protected]>

…ld_ci_with_industrial_ci

ahcorde

I will wait for a final review from @Yadunund

Yadunund

It looks like CI time for binary jobs has increased by little over 2 mins due to the fact that we're installing ROS binaries as part of the workflow. Left a suggestion for how we could tackle this.

Yadunund · 2024-08-25T04:25:28Z

.github/workflows/build.yaml

+          # Jazzy (binary)
+          - ROS_DISTRO: jazzy
+            BUILD_TYPE: binary
+            DOCKER_IMAGE: ubuntu:noble


I think we should remove this DOCKER_IMAGE flag as it's one extra item to configure.
See commend below for relevant suggestion.

Yadunund · 2024-08-25T04:27:09Z

.github/workflows/build.yaml

    container:
-      image: ros:${{ matrix.distro }}-ros-base
-    timeout-minutes: 30
+      image: ${{ matrix.DOCKER_IMAGE }}


Our CI times have increased by 2mins by installed ROS binaries for cases where BUILD_TYPE is binary. Can we use an if condition here to use ros:${{ matrix. ROS_DISTRO }}-ros-base here if matrix.BUILD_TYPE == 'binary'; else ubuntu:noble. We won't require the DOCKER_IMAGE variable anymore.

I've modified based on your suggestion. But i've also had to install coverage tools in case of using binary because of crashing CI. I hope i've decreased CI check time. Finally, my approach is okay with binary builds, but It can create some problems in case that you want to build from iron source because iron fails in ubuntu-noble
While modifying, i've utilized https://github.com/open-rmf/rmf_ci_templates/blob/1ae7b4f64644ab2f8724e9a1e1b3d14803caa61e/.github/workflows/reusable_build.yaml#L56-L59

Signed-off-by: CihatAltiparmak <[email protected]>

Yadunund

LGTM! Thanks for iterating @CihatAltiparmak and for addressing the build error on Rolling!

* Modified CI in a way that rmw_zenoh can also be tested with ROS testing repos * Fixed indentation error in workflow yaml * Build rolling from source, Iron and Jazzy from binaries and cron job too Signed-off-by: Alejandro Hernández Cordero <[email protected]> * Fixed syntaz Signed-off-by: Alejandro Hernández Cordero <[email protected]> * Use Ubuntu jammy with Iron Signed-off-by: Alejandro Hernández Cordero <[email protected]> * Applied the suggestions in ( ros2#256 (comment) ) Signed-off-by: CihatAltiparmak <[email protected]> * Fixed vcs-url * Added matrix, changed setup-ros and reverted deleted fail-fast-param Signed-off-by: CihatAltiparmak <[email protected]> * Removed DOCKER_IMAGE argument Signed-off-by: CihatAltiparmak <[email protected]> * Added lcov package Signed-off-by: CihatAltiparmak <[email protected]> --------- Signed-off-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: CihatAltiparmak <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]>

CihatAltiparmak added 2 commits August 3, 2024 02:31

Modified CI in a way that rmw_zenoh can also be tested with ROS testi…

fdc23e1

…ng repos

Fixed indentation error in workflow yaml

08856d9

Yadunund requested changes Aug 15, 2024

View reviewed changes

ahcorde added 3 commits August 22, 2024 12:39

Build rolling from source, Iron and Jazzy from binaries and cron job too

7b4435b

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

Fixed syntaz

d6f9cdc

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

Use Ubuntu jammy with Iron

3938e8b

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

ahcorde requested a review from Yadunund August 22, 2024 10:55

Merge remote-tracking branch 'origin/rolling' into feature/modify_bui…

325a9cf

…ld_ci_with_industrial_ci

CihatAltiparmak changed the title ~~Use industrial ci to track ros testing repo and simplify the build job~~ Modify CI to track ros testing repo Aug 22, 2024

Yadunund requested changes Aug 22, 2024

View reviewed changes

CihatAltiparmak added a commit to CihatAltiparmak/rmw_zenoh that referenced this pull request Aug 22, 2024

Applied review ( See ros2#256 (comment) )

e80d46e

Signed-off-by: CihatAltiparmak <[email protected]>

CihatAltiparmak added 2 commits August 22, 2024 23:31

Applied the suggestions in ( ros2#256 (comment) )

f7e16ba

Signed-off-by: CihatAltiparmak <[email protected]>

Fixed vcs-url

c7c45fd

Yadunund requested changes Aug 22, 2024

View reviewed changes

CihatAltiparmak and others added 2 commits August 23, 2024 02:05

Added matrix, changed setup-ros and reverted deleted fail-fast-param

1cfa9be

Signed-off-by: CihatAltiparmak <[email protected]>

Merge remote-tracking branch 'origin/rolling' into feature/modify_bui…

ccf3cab

…ld_ci_with_industrial_ci

ahcorde requested a review from Yadunund August 23, 2024 08:38

ahcorde approved these changes Aug 23, 2024

View reviewed changes

Yadunund requested changes Aug 25, 2024

View reviewed changes

CihatAltiparmak added 2 commits August 25, 2024 19:01

Removed DOCKER_IMAGE argument

01bcb09

Signed-off-by: CihatAltiparmak <[email protected]>

Added lcov package

57abcc3

Signed-off-by: CihatAltiparmak <[email protected]>

Yadunund approved these changes Aug 26, 2024

View reviewed changes

Yadunund merged commit a9b95a9 into ros2:rolling Aug 26, 2024
8 checks passed

Yadunund mentioned this pull request Aug 26, 2024

test_rclcpp in ci #155

Closed

CihatAltiparmak mentioned this pull request Oct 1, 2024

Add system tests into CI #271

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify CI to track ros testing repo #256

Modify CI to track ros testing repo #256

CihatAltiparmak commented Aug 3, 2024 •

edited

Loading

MichaelOrlov commented Aug 15, 2024

Yadunund left a comment •

edited

Loading

clalancette commented Aug 21, 2024

ahcorde commented Aug 22, 2024

ahcorde commented Aug 22, 2024

Yadunund left a comment

Yadunund Aug 22, 2024

Yadunund Aug 22, 2024

Yadunund Aug 22, 2024

CihatAltiparmak Aug 22, 2024 •

edited

Loading

Yadunund Aug 22, 2024

CihatAltiparmak Aug 22, 2024 •

edited

Loading

ahcorde Aug 23, 2024

ahcorde left a comment

Yadunund left a comment

Yadunund Aug 25, 2024

Yadunund Aug 25, 2024

CihatAltiparmak Aug 25, 2024 •

edited

Loading

Yadunund left a comment

Modify CI to track ros testing repo #256

Modify CI to track ros testing repo #256

Conversation

CihatAltiparmak commented Aug 3, 2024 • edited Loading

MichaelOrlov commented Aug 15, 2024

Yadunund left a comment • edited Loading

Choose a reason for hiding this comment

clalancette commented Aug 21, 2024

ahcorde commented Aug 22, 2024

ahcorde commented Aug 22, 2024

Yadunund left a comment

Choose a reason for hiding this comment

Yadunund Aug 22, 2024

Choose a reason for hiding this comment

Yadunund Aug 22, 2024

Choose a reason for hiding this comment

Yadunund Aug 22, 2024

Choose a reason for hiding this comment

CihatAltiparmak Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

Yadunund Aug 22, 2024

Choose a reason for hiding this comment

CihatAltiparmak Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

ahcorde Aug 23, 2024

Choose a reason for hiding this comment

ahcorde left a comment

Choose a reason for hiding this comment

Yadunund left a comment

Choose a reason for hiding this comment

Yadunund Aug 25, 2024

Choose a reason for hiding this comment

Yadunund Aug 25, 2024

Choose a reason for hiding this comment

CihatAltiparmak Aug 25, 2024 • edited Loading

Choose a reason for hiding this comment

Yadunund left a comment

Choose a reason for hiding this comment

CihatAltiparmak commented Aug 3, 2024 •

edited

Loading

Yadunund left a comment •

edited

Loading

CihatAltiparmak Aug 22, 2024 •

edited

Loading

CihatAltiparmak Aug 22, 2024 •

edited

Loading

CihatAltiparmak Aug 25, 2024 •

edited

Loading