-
Notifications
You must be signed in to change notification settings - Fork 5k
add BUILD_INSTALLER parameter for optionally build prepare and log container #22148
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
add BUILD_INSTALLER parameter for optionally build prepare and log container #22148
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #22148 +/- ##
===========================================
+ Coverage 45.36% 65.87% +20.50%
===========================================
Files 244 1072 +828
Lines 13333 115713 +102380
Branches 2719 2925 +206
===========================================
+ Hits 6049 76226 +70177
- Misses 6983 35262 +28279
- Partials 301 4225 +3924
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
6e97fd9 to
a2c783a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Add a BUILD_INSTALLER flag to control whether the prepare and log containers are built and included in the offline installer.
- Introduce a conditional
buildcompttarget inmake/photon/Makefileto skip_build_prepareand_build_logwhenBUILD_INSTALLER=false - Update
BUILDBASETARGETandDOCKERSAVE_PARAin the top-levelMakefileto respect the new flag and pass it into build containers - Add a
check_buildinstallerguard for thepackage_offlinetarget to enforceBUILD_INSTALLER=true
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| make/photon/Makefile | Replace hardcoded build: dependencies with a conditional buildcompt based on BUILD_INSTALLER |
| Makefile | Default BUILD_INSTALLER=true, adjust BUILDBASETARGET, DOCKERSAVE_PARA, expose flag in build container, and guard the package_offline target |
Comments suppressed due to low confidence (2)
make/photon/Makefile:237
- [nitpick] The target name 'buildcompt' is ambiguous; consider renaming it to a more descriptive name like 'build_components' for clarity.
buildcompt: _build_prepare _build_db _build_portal _build_core _build_jobservice _build_log _build_nginx _build_registry _build_registryctl _build_trivy_adapter _build_redis _compile_and_build_exporter
Makefile:458
- [nitpick] Consider rephrasing this error message to 'BUILD_INSTALLER must be set to "true" to use package_offline target' for better readability.
echo "Must set BUILD_INSTALLER as true while triggering package_offline build" ; \
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
…tainer only when we need to build offline_installer Signed-off-by: my036811 <[email protected]>
a2c783a to
8eb9ff5
Compare
…ntainer (goharbor#22148) add BUILD_INSTALLER parameter to optionally build prepare and log container only when we need to build offline_installer Signed-off-by: my036811 <[email protected]> Signed-off-by: AYDEV-FR <[email protected]>
…ntainer (goharbor#22148) add BUILD_INSTALLER parameter to optionally build prepare and log container only when we need to build offline_installer Signed-off-by: my036811 <[email protected]>
Thank you for contributing to Harbor!
Comprehensive Summary of your change
add BUILD_INSTALLER parameter to optional build prepare and log container when only need build offline_installer
Issue being fixed
Fixes #(issue)
Please indicate you've done the following: