Skip to content

Use chef configuration in windows dockerfile setup - #425

Merged
nuclearsandwich merged 14 commits into
masterfrom
brawner/ros2_windows_chef
Aug 6, 2020
Merged

nuclearsandwich merged 14 commits into
masterfrom
brawner/ros2_windows_chef

Conversation

@brawner

@brawner brawner commented Apr 9, 2020 •

Copy link
Copy Markdown
Contributor

This PR enables the windows dockerfile to install the ROS 2 dependencies from a chef cookbook. This will allow regular testing of the cookbook so that everyone can make use of it.

Currently this targets my personal repo, but I'll switch it over once the ros2 cookbook has a permanent home.

Depends on #424
New commit: ca34ec3

@brawner brawner self-assigned this Apr 9, 2020
@brawner
brawner force-pushed the brawner/ros2_windows_chef branch 11 times, most recently from 4a85d8c to ca34ec3 Compare April 9, 2020 20:29
@brawner

brawner commented Apr 9, 2020

Copy link
Copy Markdown
Contributor Author

This is now building and testing up to rclcpp
Build Status

@brawner brawner changed the title WIP: Use chef configuration in windows dockerfile setup Use chef configuration in windows dockerfile setup Apr 9, 2020
@brawner

brawner commented Apr 9, 2020

Copy link
Copy Markdown
Contributor Author

Full builds and tests:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@brawner
brawner force-pushed the brawner/ros2_windows_chef branch from ca34ec3 to a6649e9 Compare April 21, 2020 22:20
@brawner

brawner commented Apr 21, 2020

Copy link
Copy Markdown
Contributor Author

Updated tests:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@brawner

brawner commented Apr 24, 2020

Copy link
Copy Markdown
Contributor Author

Now external dependencies are installed through berks.

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@dirk-thomas

Copy link
Copy Markdown
Member

If this change is only touching Windows related files why does it need to run builds on the other platforms?

@brawner
brawner force-pushed the brawner/ros2_windows_chef branch from eb6fce2 to 9be96be Compare April 28, 2020 22:47
@brawner

brawner commented Apr 28, 2020 •

Copy link
Copy Markdown
Contributor Author

Updated cookbook, testing again (but only windows for Dirk's sake :P)

Build Status

@brawner

brawner commented Apr 29, 2020

Copy link
Copy Markdown
Contributor Author

Cookbook should now be good to build with connext.
Build Status

@brawner

brawner commented Apr 30, 2020 •

Copy link
Copy Markdown
Contributor Author

Building again Build Status

@brawner
brawner force-pushed the brawner/ros2_windows_chef branch 3 times, most recently from 41d4f69 to 62b070b Compare April 30, 2020 00:35
nuclearsandwich and others added 3 commits July 21, 2020 12:20
* Switching dockerfile to install from chef config.

* Updating cookbooks.

* Add environment json file for setting chef attributes.

* Add docker build argument for ROS_DISTRO.

Instead of using separate dockerfiles for different ROS distros this
uses separate chef environment files to set a ros_distro attribute which
can be used inside the chef cookbook.

If this is successful we can unify the separate Dockerfiles and modify
software in the chef cookbook.

* Use Docker variable syntax for build argument in COPY.

* Update chef environment usage.

I forgot that environment paths must be configured instead of specifying
an absolute path to an environment file.

* Expand environment file and add README describing directory.

* Use a single string for environment_path.

Unlike cookbook_path, environment_path must be a single string.

* Remove environment files.

We're going to instead add the attribute directly to the
install_ros2.json files.

* Copy install_ros.json to Dashing specific file.

* Add ros_distro attribute.

* Don't specify environment.

* Use ros_distro-specific node file.

* Re-organize dockerfiles and chef json files.

* Use ROS_DISTRO build arg instead of separate Dockerfiles.

With the setup being performed by chef the Dockerfiles aren't distinct
enough to warrant separate files and we can instead use a single
dockerfile with different chef attributes.

This also means we can stop fudging the ros_distro parameter since the
common behavior is also implemented in Chef.

Co-authored-by: Stephen Brawner <brawner@gmail.com>
Signed-off-by: Stephen Brawner <brawner@gmail.com>
Signed-off-by: Stephen Brawner <brawner@gmail.com>

@brawner brawner left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Merged in @nuclearsandwich's changes and addressed @j-rivero suggestions I believe.

Comment thread windows_docker_resources/Dockerfile
Comment thread windows_docker_resources/Dockerfile
@brawner

brawner commented Jul 21, 2020

Copy link
Copy Markdown
Contributor Author

Building rolling:

Build Status

@brawner

brawner commented Jul 21, 2020

Copy link
Copy Markdown
Contributor Author

Err, this looks like it needs the ci_job.xml.em updates to be applied.

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@nuclearsandwich

nuclearsandwich commented Jul 24, 2020 •

Copy link
Copy Markdown
Member

Err, this looks like it needs the ci_job.xml.em updates to be applied.

Yeah this PR makes changes to the job configuration as well as the scripts and dockerfiles. I've deployed this PR to the test_ci_windows job and launched the same job there

Rolling Build Status
Rolling Debug Build Status
Dashing Build Status
Dashing Debug Build Status

If CI calms down doing the full gamut and also testing Focal and Eloquent, which are currently identical to Rolling and Dashing respectively would be pleasantly paranoid.

@nuclearsandwich nuclearsandwich left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A couple of small questions but as long as the final round of testing is good I think this is ready to go.

Comment thread .gitmodules Outdated
Comment thread windows_docker_resources/solo.rb Outdated
brawner added 2 commits July 27, 2020 10:53
Signed-off-by: Stephen Brawner <brawner@gmail.com>
Signed-off-by: Stephen Brawner <brawner@gmail.com>
Comment thread windows_docker_resources/Dockerfile Outdated
Comment thread windows_docker_resources/README.md Outdated
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner

brawner commented Jul 27, 2020

Copy link
Copy Markdown
Contributor Author

Rebuilding/testing --packages-up-to rclcpp

Rolling Build Status
Rolling Debug Build Status
Dashing Build Status
Dashing Debug Build Status

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@nuclearsandwich

Copy link
Copy Markdown
Member

The latest builds have a plethora of errors due to the installation of cppcheck 2.x which we don't yet support.
ros-infrastructure/ros2-cookbooks#8 is updating the recipe to install 1.90 as we're currently doing in the direct Dockerfile. Once this merges I'll need to update #480 to target a cookbook branch that removes the pin.

@nuclearsandwich

Copy link
Copy Markdown
Member

I think this is ready. Running full builds of Rolling and Rolling Debug to compare with nightlies

Rolling Build Status
Rolling Debug Build Status

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner

brawner commented Aug 4, 2020

Copy link
Copy Markdown
Contributor Author

Updated branch to target ros-infrastructure/ros2-cookbooks#9

Rolling Build Status
Rolling Debug Build Status

@brawner

brawner commented Aug 5, 2020

Copy link
Copy Markdown
Contributor Author

Updated branch of cookbooks back to latest. Building up to and testing only image_tools to confirm update.

Rolling: Build Status
Rolling Debug: Build Status

@nuclearsandwich

Copy link
Copy Markdown
Member

Planning to merge and deploy before the next round of nightlies.

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@nuclearsandwich
nuclearsandwich merged commit f71c428 into master Aug 6, 2020
@nuclearsandwich
nuclearsandwich deleted the brawner/ros2_windows_chef branch August 6, 2020 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants