RFC: Changing how we handle Webpack Building

If plugin (or core) depend on a package include in foreman-vendor it will ignore the local package and will use the package included in foreman-vendor.
If a package not included in foreman-vendor it will compile this package into the core/plugin bundle file.

We might be able to reduce it, you need different packages for different use-cases.

Build the foreman-vendor dist files
Needs all the packages, once done, you have dist files in npm.

Run foreman dev-server, lint, storybook, testing
Needs all the nonDev dependencies in foreman-vendor package.json

Build foreman/plugins production/dev bundles
Needs only one package from the foreman-vendor package.json, copy-webpack-plugin.

Can we reduce build sizes around it?

To be perfectly clear: currently we have all sources (babel-x.y.z.tgz, react-x.y.z.tgz) listed in the spec file. We also have a tarball that contains the cached responses from npm (PACKAGE-registry-x.y.z.tgz) which we check into git directly. It turns out that also contains the sources (babel-x.y.z.tgz, …). This duplication doesn’t serve any purpose and only blows up our git repository size. I think a patch that clears the duplication and reuses the sources should be easy to come up with.

A quick test removing the actual sources reduced the cache size from 34M to 14M. Will proceed with that further. Looks like we’ve been duplicating that for quite some time but this ridiculous size made it obvious.

I do think it’d be good to have a package that you can add to your devRequires and one to your requires. From the packaging side that would greatly improve our life.

2 Likes

Agree, I will work on a separation between @theforeman/vendor and @theforeman/vendor-dev :+1:

1 Like

Updates

Created a new monorepo foreman-js using lerna with the following packages:

Created a new draft PR that integrates with @theforeman/vendor and @theforeman/vendor-dev:

2 Likes

I may have asked this before. I had a general question regarding the overall architecture of UI components and how plugins are handled.

  1. Could the navigation be split into’s own .js file with it’s own bound dependencies (e.g. nav-vendor.js) that is loaded on all pages?
  2. Could the JS that is loaded per page be reduced to a set of JS for the navigation, and then any JS the plugin (or core) page being loaded requires to function?
  3. Could plugins be built against and include their own vendor-.js (or included into their plugin.js) such that the JS for a given plugin is independently built and loaded thus isolating it from the current dependency chain?

This in my estimation would give plugins back the flexibility they need and reduce the burden on the build system that we encounter to date. This may in part require previously proposed solutions but it feels like there has got to be a way to arrive at a more independent architecture for plugins to prevent deadlock.

Status update, see:

I gave it a try and it seems to be working fine, here is a breakdown of the assets foreman compiles

Before this Patch

After this patch

if you want to give a try you can use Quay to try it out.

Update from a 03.07.19 meeting between @tbrisker @ekohl @evgeni @amirfefer and I about this topic

  1. Working on fixing the issue that blocks the packaging https://github.com/theforeman/foreman-packaging/pull/3882
    The issue is the vendor uses vendor-core in the plugin it delivers to foreman.
    The approach will be to update the build process of the vendor so it will have the file it uses from vendor-core internally.
    @sharvit

  2. Continue the packaging work on @theforeman/vendor.
    Blocked by #1
    @ekohl @evgeni

  3. Fix the ruby integration test errors: https://ci.theforeman.org/job/test_develop_pr_core/1282/database=postgresql-integrations,ruby=2.5,slave=fast/testReport/junit/(root)/AboutIntegrationTest/test_0001_about_page/
    All the integration test fails atm but works well when using the chrome driver.

    1. Push forward the work on https://github.com/theforeman/foreman/pull/6748
      @ezr-ondrej @tbrisker
    2. Rebase the migration PR in core to use https://github.com/theforeman/foreman/pull/6748 so we can verify it is actually solving the test errors.
      @sharvit

Ping me if you want to join the next meeting.

Thanks!

Update from a 10.07.19 meeting between @tbrisker @ekohl @evgeni and I about this topic

Done last week

  1. @theforeman/vendor version 0.1.0-alpha.9 released with https://github.com/theforeman/foreman-js/pull/13

  2. The ruby test errors got fixed by merging and rebasing @ezr-ondrej PR that switches to headless chrome based testing https://github.com/theforeman/foreman/pull/6748
    Thanks @ezr-ondrej, @tbrisker

Goals for the next week

  1. The packaging team will continue the effort packaging @theforeman/vendor version 0.1.0-alpha.9 and produce a koji build. @tbrisker @ekohl @evgeni

  2. Finish the work over https://github.com/theforeman/foreman/pull/6605
    The goal is to merge until the end of the next week (18.07).
    @sharvit @tbrisker

Cheers!

1 Like

This has been merged:

https://github.com/theforeman/foreman/commit/0220eaac7cbfc419aa0570a53308d9b2d25b72e5

RPM packaging has been updated, but plugins need to be rebuilt. Expect some breakages in nightlies over the next few days.

This also means all PRs that touch package.json need to be rebased. I’ll leave it to @sharvit to instruct developers on this :slight_smile:

1 Like

Thanks to everyone who contributed to the vendor effort!

atm we will not accept PRs changing the package.json until 1.23 is branched?

Later today I will update about the next goals of the vendor project.

  • It will Include creating a tutorial and a deep-dive about working with 3rd-party packages (add/update/delete/consume).

After the merges for core and packaging yesterday, it’s time to look at plugins (foreman-tasks, foreman_ansible, foreman_remote_execution, katello):

  • foreman_remote_execution got a regular update yesterday (1.8.0 to 1.8.2) which got built with the new Foreman bundle already and is working fine on my nightly VM
  • foreman-tasks also got a regular update, which scratch built fine before the big change, but then failed to release build with the change, see below for detail
  • foreman_ansible started building public/webpack/foreman-vendor.<version>.<hash>.js and .css files, which need to be excluded. I’ve opened foreman-packaging#3930 for that, but we might want to make this a macro or something. @ekohl, what do you think?
  • katello I didn’t touch at all yet

Now to the (slightly longer) foreman-tasks story :slight_smile:

I’ve used foreman-tasks 0.15.5 yesterday for my initial “we just need to rebuild the plugin to make it work again” tests and it worked just fine. However, in parallel to this, there also was a foreman-tasks 0.16.0 release that built fine during scratch building (as that happened before we merged @sharvit’s work), but failed during the actual release build.

Avi and I think that commit 0eacaf8587bf88986a675f4b5acecb93cc9b58f6 is what broke tasks, as it introduced a dependency on bootstrap-sass without adding it to package.json or the RPM spec file. This was fine before, as Foreman itself required bootstrap-sass and thus it was also available to the plugins. With the change to @theforeman/vendor, this is not the canse anymore and the foreman-tasks fails to build. As a workaround, I’ve opened foreman-packaging#3931 which explicitly adds bootstrap-sass to the BuildRequires and that builds fine again.

I think this is a good enough workaround until we migrate foreman-tasks to use @theforeman/vendor directly.

CC @aruzicka @iNecas for their f-tasks hats :slight_smile:

1 Like

Quick update on Katello, it needs the same massaging as foreman_ansible for the newly generated files, but the resulting RPM does not have a working UI :frowning:

PR: https://github.com/theforeman/foreman-packaging/pull/3932
Scratch build: http://koji.katello.org/koji/taskinfo?taskID=212894

the fix for public/webpack/foreman-vendor.<version>.<hash>.js and .css went in as foreman-packaging#3933 and foreman_ansible is currently building with it. when the build has finished we will have tasks, ansible and rex as working rpms.

just katello left :slight_smile:

2 Likes

The next goals for the vendor

plugin work

  1. ATM we are only having issues with katello and I am working on migrating katello to use the new vendor, hopefully it will fix katello or give us some clue about the issue.
  2. Update foreman-tasks
  3. Update rex
  4. Update foreman-ansible

Vendor Work

  1. Release none alpha version v0.1.0
  2. Announce the new vendor
  3. Write a tutorial about adding/updating/removing packages
  4. Do a deep dive about managing npm deps with the new vendor
  5. Update the how to create a plugin
  6. Update the plugin template to use webpack and the new vendor
1 Like

Migration PR in katello: https://github.com/Katello/katello/pull/8221

The katello issue resolved :slight_smile:

2 Likes

foreman-tasks migration: https://github.com/theforeman/foreman-tasks/pull/440

1 Like