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

New image for single images #11954

Merged
merged 82 commits into from
Oct 17, 2023
Merged

New image for single images #11954

merged 82 commits into from
Oct 17, 2023

Conversation

adrinr
Copy link
Collaborator

@adrinr adrinr commented Oct 2, 2023

Description

Updating single image docker in order to be able to be cachable. These changes are for now not applied in the actual single image release pipeline, but they are used in a test pipeline and the same Dockerfile is used for the feature branch environments.
This new image will not have dependencies with the npm registry, so we will be able to test the feature branches as a whole.
It will require some changes on the release side that can be found in this PR: https://github.com/Budibase/budibase-deploys/pull/130

This PR also contains multiple package updates in order to fix some security and performance issues and in order to remove the dependency of pulling down the Budibase dependencies from the registry

Feature branch URL: http://fb-budi-7573-use-existing-image-cache.fb.qa.budibase.net/builder/auth/login

@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2023

Codecov Report

Merging #11954 (55698d9) into master (762581f) will increase coverage by 13.15%.
Report is 36 commits behind head on master.
The diff coverage is n/a.

❗ Current head 55698d9 differs from pull request most recent head 54daf2f. Consider uploading reports for the commit 54daf2f to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff             @@
##           master   #11954       +/-   ##
===========================================
+ Coverage   67.87%   81.02%   +13.15%     
===========================================
  Files         585       95      -490     
  Lines       21858     2161    -19697     
  Branches     4406      319     -4087     
===========================================
- Hits        14836     1751    -13085     
+ Misses       6495      385     -6110     
+ Partials      527       25      -502     

see 492 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@adrinr adrinr changed the title WIP New image for single images Oct 13, 2023
root_package_json=$(cat "package.json")

process_package() {
local pkg="$1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does this work with loading the dependencies of the packages themselves? Like if we remove @budibase/backend-core from the package.json how do we fetch its dependencies?

I may be mis-understanding this as well - just trying to understand the script!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This removes all the @budibase/* packages from the package.json.
With the current setup, all the packages are bundled using esbuild, and we only need to fetch the packages from worker and server. Without this, when running yarn install, it will try to pull the @budibase packages from the npm repo, and it would fail.
Note that we could not bundle string-templates, for this the image is pulling it and linking it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok cool - good to understand!

@adrinr adrinr closed this Oct 16, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2023
@adrinr adrinr reopened this Oct 16, 2023
@adrinr adrinr force-pushed the BUDI-7573/use_existing_image_cache branch from bf77741 to 7c924f4 Compare October 16, 2023 13:40
@adrinr adrinr marked this pull request as ready for review October 16, 2023 14:19
Copy link
Member

@shogunpurple shogunpurple left a comment

Choose a reason for hiding this comment

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

LGTM Adri - I think we should write some documentation in confluence on how this build works though at a high level just for reference.

Copy link
Collaborator

@mike12345567 mike12345567 left a comment

Choose a reason for hiding this comment

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

Looks good to me! If adding documentation to Confluence should probably also include the note about Node 18 and IPv6 vs v4 (issues with it) as the single image is the only thing that will be affected by this (nothing else needs to access local resources).

@adrinr adrinr merged commit a440fbc into master Oct 17, 2023
9 of 10 checks passed
@adrinr adrinr deleted the BUDI-7573/use_existing_image_cache branch October 17, 2023 09:20
@adrinr adrinr restored the BUDI-7573/use_existing_image_cache branch October 17, 2023 09:23
@adrinr adrinr deleted the BUDI-7573/use_existing_image_cache branch October 17, 2023 09:25
@adrinr
Copy link
Collaborator Author

adrinr commented Oct 17, 2023

Wrong push merged an unexpected node_modules. Removing it in here: #12081

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants