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

CI: Update containers and migrate to newer tooling #23142

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

TSC21
Copy link
Member

@TSC21 TSC21 commented May 16, 2024

Solved Problem

Update the containers to use more recent OS versions and tooling. This also allows to solve some flash problems by bumping the version of GCC to 11.

Solution

  • Add updated container images for GitHub Actions workflows.
  • Refactor CI configurations to migrate from ROS Melodic to Noetic.
  • Remove Python 2 shebang in the mavros tests, to allow Python 3 to be used instead.
  • Others as per reported errors and build failures.

Changelog Entry

For release notes:

Feature: Updated GitHub Actions container images to newer versions (jammy, 2024-05-17).
Migration: Moved MAVROS tests from ROS Melodic to ROS Noetic.
Documentation: Clarify CI configurations on docs.px4.io.

Alternatives

Well, we can always keep using legacy container versions and continue trying to optimize the flash space.

Test coverage

  • Check the CI runs

Context

N.A.

@TSC21 TSC21 force-pushed the update_ga_ci branch 8 times, most recently from 734b927 to 5d1f0ed Compare May 18, 2024 18:12
@dagar dagar self-requested a review May 18, 2024 18:18
@TSC21 TSC21 marked this pull request as ready for review May 18, 2024 19:54
@TSC21
Copy link
Member Author

TSC21 commented May 18, 2024

@dagar would appreciate your review here. I can see some sim tests consistently failing but they are also seen failing without the changes on the containers (in other PR runs). Might be reasonable to explore ways of solving those failures, but potentially in a separate PR (I am mostly interested on getting the plane tests up, running and passing ASAP). Other than that, I believe this PR should be ready to go.

@TSC21 TSC21 changed the title ci: github actions: update container images CI Update containers and migrate to newer tooling May 18, 2024
@TSC21 TSC21 changed the title CI Update containers and migrate to newer tooling CI: Update containers and migrate to newer tooling May 18, 2024
@TSC21 TSC21 self-assigned this May 18, 2024
@TSC21 TSC21 added Sim: gazebo classic Gazebo classic simulator Tools Sub-tools used within PX4 ecosystem (scripts, etc) CI (Continuous Integration) 🤖 Development Environment 🖥️ For setting up environment for developers. VSCode Extensions, WSL configuration, etc labels May 18, 2024
@AlexKlimaj
Copy link
Member

Do you want to update the tools/setup/ubuntu.sh installed GCC toolchain to match as well? We save some flash by upgrading. Also makes compatibility with cortex-debug work again since it requires a newer GDB version.

@TSC21
Copy link
Member Author

TSC21 commented May 20, 2024

Do you want to update the tools/setup/ubuntu.sh installed GCC toolchain to match as well? We save some flash by upgrading. Also makes compatibility with cortex-debug work again since it requires a newer GDB version.

Yes. If you already have a suggestion on what to change there, please feel free to push a commit directly to this branch.

@AlexKlimaj
Copy link
Member

I have this PR open, but there was the issue of updating the CI tools to match. #22568

@TSC21
Copy link
Member Author

TSC21 commented May 20, 2024

I have this PR open, but there was the issue of updating the CI tools to match. #22568

I would say we don't need an exact match - as long as we use GCC 10 or above we should be ok either using GCC 11 (default on Ubuntu 22.04) or GCC 13.

@dagar
Copy link
Member

dagar commented May 20, 2024

I have this PR open, but there was the issue of updating the CI tools to match. #22568

I would say we don't need an exact match - as long as we use GCC 10 or above we should be ok either using GCC 11 (default on Ubuntu 22.04) or GCC 13.

We need to have one "official" version that we test in CI, use for the QGC builds, and most importantly can point to if they hit some weird edge case, slight flash overflow, etc. Once we bump everything to Ubuntu 24.04 I'd like this to be the packaged arm-none-eabi-gcc version so we can eliminate the custom installation.

That being said, we should absolutely try to keep the compiler support as wide as possible, always keeping up with latest GCC/Clang on the side. There's no reason the default GCC/Clang on most recent-ish distros, MacOS, etc shouldn't "just work", but it's only best effort, if there's ever an annoying problem we can always point to the exact version that's tested across everything.

@TSC21
Copy link
Member Author

TSC21 commented May 20, 2024

I have this PR open, but there was the issue of updating the CI tools to match. #22568

I would say we don't need an exact match - as long as we use GCC 10 or above we should be ok either using GCC 11 (default on Ubuntu 22.04) or GCC 13.

We need to have one "official" version that we test in CI, use for the QGC builds, and most importantly can point to if they hit some weird edge case, slight flash overflow, etc. Once we bump everything to Ubuntu 24.04 I'd like this to be the packaged arm-none-eabi-gcc version so we can eliminate the custom installation.

That being said, we should absolutely try to keep the compiler support as wide as possible, always keeping up with latest GCC/Clang on the side. There's no reason the default GCC/Clang on most recent-ish distros, MacOS, etc shouldn't "just work", but it's only best effort, if there's ever an annoying problem we can always point to the exact version that's tested across everything.

@dagar ok so what do you suggest we do right now? Install GCC 11 on the build scripts, or install GCC 13 or later in the containers? Or can we merge this and take that decision in another PR soon?

@dagar
Copy link
Member

dagar commented May 20, 2024

Do you want to start with updating/fixing just the MAVROS pieces here?

Screenshot 2024-05-20 at 3 53 25 PM

For actually bumping the compiler version for the firmware builds I'd like to do that in a unified pass where we update Docker, Tools/setup, documentation all together. We also need a sanity check flying those firmware builds across the main architectures. I know it seems paranoid, but there have been subtle problems in the past.

@TSC21
Copy link
Member Author

TSC21 commented May 28, 2024

Do you want to start with updating/fixing just the MAVROS pieces here?

Screenshot 2024-05-20 at 3 53 25 PM

@dagar I can give it a try but this doesn't seem to be a problem inherent to the version of the tooling but rather something in the tests or even in the controller side. Can this be addressed in a separate PR?

For actually bumping the compiler version for the firmware builds I'd like to do that in a unified pass where we update Docker, Tools/setup, documentation all together. We also need a sanity check flying those firmware builds across the main architectures. I know it seems paranoid, but there have been subtle problems in the past.

OK fair enough - I can help with getting a unified update here. @mrpollo @AlexKlimaj maybe you guys can or know someone that can help with the flight testing, specially with MCs?

@AlexKlimaj
Copy link
Member

We can test fly multicopter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI (Continuous Integration) 🤖 Development Environment 🖥️ For setting up environment for developers. VSCode Extensions, WSL configuration, etc Sim: gazebo classic Gazebo classic simulator Tools Sub-tools used within PX4 ecosystem (scripts, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants