harbor icon indicating copy to clipboard operation
harbor copied to clipboard

20548 MISSING CONDITION FOR RETURNING LOG DATA

Open nickhyoti opened this issue 1 year ago • 7 comments

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.

nickhyoti avatar Jun 04 '24 13:06 nickhyoti

I'm getting emails about this not being labeled but theres no ability to label

nickhyoti avatar Jun 05 '24 09:06 nickhyoti

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

Impacted file tree graph

@@            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%> (ø)

... and 571 files with indirect coverage changes

codecov[bot] avatar Jun 05 '24 10:06 codecov[bot]

@nickhyoti could you please help to resolve this DCO issue? Thanks

zyyw avatar Jun 16 '24 07:06 zyyw

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 ?

shishir-11 avatar Jun 25 '24 12:06 shishir-11

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

zyyw avatar Jun 27 '24 11:06 zyyw

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

shishir-11 avatar Jun 28 '24 04:06 shishir-11

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

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.

zyyw avatar Jun 28 '24 08:06 zyyw

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)
Screenshot 2024-07-02 at 5 57 34 AM

After (Logs appear)
Screenshot 2024-07-02 at 5 57 07 AM

I would love to receive any feedback, Thanks! 🙏

#20684

mohamedawnallah avatar Jul 02 '24 04:07 mohamedawnallah

Closed as merged in https://github.com/goharbor/harbor/pull/20684

chlins avatar Jul 04 '24 03:07 chlins