20548 MISSING CONDITION FOR RETURNING LOG DATA
As per bug #20548, if DB logging is enabled for jobservice and the parameter is also set for maximum log size the log data is not being returned and 'Clean Up->Show GC Logs' shows a blank page
Thank you for contributing to Harbor!
Comprehensive Summary of your change
Issue being fixed
Fixes #20548
Please indicate you've done the following:
- [x] Well Written Title and Summary of the PR
- [x] Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
- [x] Accepted the DCO. Commits without the DCO will delay acceptance.
- [x] Made sure tests are passing and test coverage is added if needed.
- [x] Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.
I'm getting emails about this not being labeled but theres no ability to label
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 66.23%. Comparing base (
b7b8847) to head (29bf1fb). Report is 220 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #20549 +/- ##
==========================================
- Coverage 67.56% 66.23% -1.33%
==========================================
Files 991 1045 +54
Lines 109181 113481 +4300
Branches 2719 2845 +126
==========================================
+ Hits 73768 75167 +1399
- Misses 31449 34210 +2761
- Partials 3964 4104 +140
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 66.23% <100.00%> (-1.33%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Files | Coverage Δ | |
|---|---|---|
| src/jobservice/logger/getter/db_getter.go | 81.81% <100.00%> (ø) |
@nickhyoti could you please help to resolve this DCO issue? Thanks
I'm a bit new and confused on some of the things , what are we supposed to add unit tests on? that it now produces the logs properly ?
I'm a bit new and confused on some of the things , what are we supposed to add unit tests on? that it now produces the logs properly ?
Thanks for the contribution. We do reproduce the issue mentioned here https://github.com/goharbor/harbor/issues/20548.
Maybe update the TestDBGetter of this file, to something like below:
// TestDBGetter
func TestDBGetter(t *testing.T) {
uuid := "uuid_for_unit_test_getter"
l, err := backend.NewDBLogger(uuid, "DEBUG", 4)
require.Nil(t, err)
l.Debug("JobLog Debug: TestDBLoggerGetter")
err = l.Close()
require.NoError(t, err)
_ = config.DefaultConfig.Load("../../config_test.yml", true) <====== add this line
dbGetter := NewDBGetter()
ll, err := dbGetter.Retrieve(uuid)
require.Nil(t, err)
require.NotEqual(t, 0, len(ll)) <====== add this line
log.Infof("get logger %s", ll)
err = sweeper.PrepareDBSweep()
require.NoError(t, err)
dbSweeper := sweeper.NewDBSweeper(-1)
count, err := dbSweeper.Sweep()
require.Nil(t, err)
require.Equal(t, 1, count)
}
@nickhyoti @shishir-11 could you please help to resolve the DCO issue displayed in the CI as well? Thanks!
cc: @wy65701436
Hey, how do you guys check out how the changes you make in the code reflect in the working except the tests , do I need to rebuild the harbor project everytime I want to see a change ? I'm also having some trouble building the project as i keep getting
Error happened in config validation...
ERROR:root:Please specify hostname
make: *** [Makefile:372: prepare] Error 255
this error even though the harbor.yml file has reg.mydomain.com as hostname
Hey, how do you guys check out how the changes you make in the code reflect in the working except the tests , do I need to rebuild the harbor project everytime I want to see a change ? I'm also having some trouble building the project as i keep getting
Error happened in config validation... ERROR:root:Please specify hostname make: *** [Makefile:372: prepare] Error 255this error even though the harbor.yml file has reg.mydomain.com as hostname
maybe you wanna try the following command to build an offline-installer against your source code:
Harbor_Build_Base_Tag=dev
Harbor_Assets_Version=dev
Harbor_Package_Version=dev-build.2024050702
build_base_params=" BUILD_BASE=true"
sudo make package_offline GOBUILDTAGS="include_oss include_gcs" BASEIMAGETAG=${Harbor_Build_Base_Tag} VERSIONTAG=${Harbor_Assets_Version} PKGVERSIONTAG=${Harbor_Package_Version} TRIVYFLAG=true HTTPPROXY= ${build_base_params}
If everything goes well, it will generate an offline-installer of the source codebase. Then you can install it in a docker-compose way.
Hey @zyyw,
I've just submitted a PR for this issue, including the test and DCO (saving credits to @nickhyoti for the main fix). I was able to test the fix before and after, as you can see in the attached images:
Before (Blank Page)
After (Logs appear)
I would love to receive any feedback, Thanks! 🙏
#20684
Closed as merged in https://github.com/goharbor/harbor/pull/20684