From c67fdc40f55f1817e745df7f549fda57c57f6370 Mon Sep 17 00:00:00 2001 From: peimingming Date: Mon, 26 Nov 2018 18:58:57 +0800 Subject: [PATCH 1/2] Support store job log in DB (#6144) Signed-off-by: peimingming --- .../postgresql/0002_1.7.0_schema.up.sql | 10 ++ src/common/dao/joblog.go | 40 +++++++ src/common/models/base.go | 3 +- src/common/models/joblog.go | 23 ++++ src/jobservice/job/impl/context.go | 31 +++++- src/jobservice/logger/backend/db_logger.go | 105 ++++++++++++++++++ .../logger/backend/db_logger_test.go | 1 + src/jobservice/logger/entry.go | 10 ++ src/jobservice/logger/factory.go | 21 ++++ src/jobservice/logger/getter/db_getter.go | 28 +++++ src/jobservice/logger/getter_factory.go | 5 + src/jobservice/logger/known_loggers.go | 4 + src/jobservice/logger/sweeper/db_sweeper.go | 48 ++++++++ src/jobservice/logger/sweeper_factory.go | 16 +++ 14 files changed, 338 insertions(+), 7 deletions(-) create mode 100644 src/common/dao/joblog.go create mode 100644 src/common/models/joblog.go create mode 100644 src/jobservice/logger/backend/db_logger.go create mode 100644 src/jobservice/logger/backend/db_logger_test.go create mode 100644 src/jobservice/logger/getter/db_getter.go create mode 100644 src/jobservice/logger/sweeper/db_sweeper.go diff --git a/make/migrations/postgresql/0002_1.7.0_schema.up.sql b/make/migrations/postgresql/0002_1.7.0_schema.up.sql index 257e76940..d285fe27c 100644 --- a/make/migrations/postgresql/0002_1.7.0_schema.up.sql +++ b/make/migrations/postgresql/0002_1.7.0_schema.up.sql @@ -1,2 +1,12 @@ ALTER TABLE properties ALTER COLUMN v TYPE varchar(1024); DELETE FROM properties where k='scan_all_policy'; + +create table job_log ( + log_id SERIAL NOT NULL, + job_uuid varchar (64) NOT NULL, + creation_time timestamp default CURRENT_TIMESTAMP, + content text, + primary key (log_id) +); + +CREATE UNIQUE INDEX job_log_uuid ON job_log (job_uuid); diff --git a/src/common/dao/joblog.go b/src/common/dao/joblog.go new file mode 100644 index 000000000..5eb73f229 --- /dev/null +++ b/src/common/dao/joblog.go @@ -0,0 +1,40 @@ +package dao + +import ( + "github.com/goharbor/harbor/src/common/models" + "github.com/astaxie/beego/orm" + "time" +) + +// CreateOrUpdateJobLog ... +func CreateOrUpdateJobLog(log *models.JobLog) (int64, error) { + o := GetOrmer() + count, err := o.InsertOrUpdate(log, "job_uuid") + if err != nil { + return 0, err + } + + return count, nil +} + +// GetJobLog ... +func GetJobLog(uuid string) (*models.JobLog, error) { + o := GetOrmer() + jl := models.JobLog{UUID: uuid} + err := o.Read(&jl, "UUID") + if err == orm.ErrNoRows { + return nil, err + } + return &jl, nil +} + +// DeleteJobLogsBefore ... +func DeleteJobLogsBefore(t time.Time) (int64, error) { + o := GetOrmer() + sql := `delete from job_log where creation_time < ?` + res, err := o.Raw(sql, t).Exec() + if err != nil { + return 0, err + } + return res.RowsAffected() +} diff --git a/src/common/models/base.go b/src/common/models/base.go index 76890dcfa..e7d08daeb 100644 --- a/src/common/models/base.go +++ b/src/common/models/base.go @@ -36,5 +36,6 @@ func init() { new(Label), new(ResourceLabel), new(UserGroup), - new(AdminJob)) + new(AdminJob), + new(JobLog)) } diff --git a/src/common/models/joblog.go b/src/common/models/joblog.go new file mode 100644 index 000000000..5efbc810f --- /dev/null +++ b/src/common/models/joblog.go @@ -0,0 +1,23 @@ +package models + +import ( + "time" +) + +// JobLogTable is the name of the table that record the job execution result. +const JobLogTable = "job_log" + +// JobLog holds information about logs which are used to record the result of execution of a job. +type JobLog struct { + LogID int `orm:"pk;auto;column(log_id)" json:"log_id"` + UUID string `orm:"column(job_uuid)" json:"uuid"` + CreationTime time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"` + Content string `orm:"column(content)" json:"content"` +} + +// TableName is required by by beego orm to map JobLog to table job_log +func (a *JobLog) TableName() string { + return JobLogTable +} + + diff --git a/src/jobservice/job/impl/context.go b/src/jobservice/job/impl/context.go index 127eb7d35..0f7dbdb91 100644 --- a/src/jobservice/job/impl/context.go +++ b/src/jobservice/job/impl/context.go @@ -31,6 +31,7 @@ import ( "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/jobservice/logger" jmodel "github.com/goharbor/harbor/src/jobservice/models" + "github.com/goharbor/harbor/src/jobservice/logger/sweeper" ) const ( @@ -95,7 +96,14 @@ func (c *Context) Init() error { db := getDBFromConfig(configs) - return dao.InitDatabase(db) + err = dao.InitDatabase(db) + if err != nil { + return err + } + + // Initialize DB finished + initDBCompleted() + return nil } // Build implements the same method in env.JobContext interface @@ -237,22 +245,28 @@ func setLoggers(setter func(lg logger.Interface), jobID string) error { lOptions := []logger.Option{} for _, lc := range config.DefaultConfig.JobLoggerConfigs { // For running job, the depth should be 5 - if lc.Name == logger.LoggerNameFile || lc.Name == logger.LoggerNameStdOutput { + if lc.Name == logger.LoggerNameFile || lc.Name == logger.LoggerNameStdOutput || lc.Name == logger.LoggerNameDB { if lc.Settings == nil { lc.Settings = map[string]interface{}{} } lc.Settings["depth"] = 5 } - if lc.Name == logger.LoggerNameFile { + if lc.Name == logger.LoggerNameFile || lc.Name == logger.LoggerNameDB { // Need extra param fSettings := map[string]interface{}{} for k, v := range lc.Settings { // Copy settings fSettings[k] = v } - // Append file name param - fSettings["filename"] = fmt.Sprintf("%s.log", jobID) - lOptions = append(lOptions, logger.BackendOption(lc.Name, lc.Level, fSettings)) + if lc.Name == logger.LoggerNameFile { + // Append file name param + fSettings["filename"] = fmt.Sprintf("%s.log", jobID) + lOptions = append(lOptions, logger.BackendOption(lc.Name, lc.Level, fSettings)) + } else { // DB Logger + // Append DB key + fSettings["key"] = jobID + lOptions = append(lOptions, logger.BackendOption(lc.Name, lc.Level, fSettings)) + } } else { lOptions = append(lOptions, logger.BackendOption(lc.Name, lc.Level, lc.Settings)) } @@ -267,3 +281,8 @@ func setLoggers(setter func(lg logger.Interface), jobID string) error { return nil } + +func initDBCompleted() error { + sweeper.PrepareDBSweep() + return nil +} diff --git a/src/jobservice/logger/backend/db_logger.go b/src/jobservice/logger/backend/db_logger.go new file mode 100644 index 000000000..3d8d82127 --- /dev/null +++ b/src/jobservice/logger/backend/db_logger.go @@ -0,0 +1,105 @@ +package backend + +import ( + "github.com/goharbor/harbor/src/common/utils/log" + "bufio" + "bytes" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/dao" +) + +// DBLogger is an implementation of logger.Interface. +// It outputs logs to PGSql. +type DBLogger struct { + backendLogger *log.Logger + bw *bufio.Writer + buffer *bytes.Buffer + key string +} + +// NewDBLogger crates a new DB logger +// nil might be returned +func NewDBLogger(key string, level string, depth int) (*DBLogger, error) { + buffer := bytes.NewBuffer(make([]byte, 0)) + bw := bufio.NewWriter(buffer) + logLevel := parseLevel(level) + + backendLogger := log.New(bw, log.NewTextFormatter(), logLevel, depth) + + return &DBLogger{ + backendLogger: backendLogger, + bw: bw, + buffer: buffer, + key: key, + }, nil +} + +// Close the opened io stream and flush data into DB +// Implements logger.Closer interface +func (dbl *DBLogger) Close() error { + err := dbl.bw.Flush() + if err != nil { + return err + } + + jobLog := models.JobLog{ + UUID: dbl.key, + Content: dbl.buffer.String(), + } + + _, err = dao.CreateOrUpdateJobLog(&jobLog) + if err != nil { + return err + } + return nil +} + +// Debug ... +func (dbl *DBLogger) Debug(v ...interface{}) { + dbl.backendLogger.Debug(v...) +} + +// Debugf with format +func (dbl *DBLogger) Debugf(format string, v ...interface{}) { + dbl.backendLogger.Debugf(format, v...) +} + +// Info ... +func (dbl *DBLogger) Info(v ...interface{}) { + dbl.backendLogger.Info(v...) +} + +// Infof with format +func (dbl *DBLogger) Infof(format string, v ...interface{}) { + dbl.backendLogger.Infof(format, v...) +} + +// Warning ... +func (dbl *DBLogger) Warning(v ...interface{}) { + dbl.backendLogger.Warning(v...) +} + +// Warningf with format +func (dbl *DBLogger) Warningf(format string, v ...interface{}) { + dbl.backendLogger.Warningf(format, v...) +} + +// Error ... +func (dbl *DBLogger) Error(v ...interface{}) { + dbl.backendLogger.Error(v...) +} + +// Errorf with format +func (dbl *DBLogger) Errorf(format string, v ...interface{}) { + dbl.backendLogger.Errorf(format, v...) +} + +// Fatal error +func (dbl *DBLogger) Fatal(v ...interface{}) { + dbl.backendLogger.Fatal(v...) +} + +// Fatalf error +func (dbl *DBLogger) Fatalf(format string, v ...interface{}) { + dbl.backendLogger.Fatalf(format, v...) +} diff --git a/src/jobservice/logger/backend/db_logger_test.go b/src/jobservice/logger/backend/db_logger_test.go new file mode 100644 index 000000000..248094365 --- /dev/null +++ b/src/jobservice/logger/backend/db_logger_test.go @@ -0,0 +1 @@ +package backend diff --git a/src/jobservice/logger/entry.go b/src/jobservice/logger/entry.go index f32e798b3..32f700183 100644 --- a/src/jobservice/logger/entry.go +++ b/src/jobservice/logger/entry.go @@ -82,3 +82,13 @@ func (e *Entry) Fatalf(format string, v ...interface{}) { l.Fatalf(format, v...) } } + +// Close logger +func (e *Entry) Close() error { + for _, l := range e.loggers { + if closer, ok := l.(Closer); ok { + closer.Close() + } + } + return nil +} diff --git a/src/jobservice/logger/factory.go b/src/jobservice/logger/factory.go index e2296e594..36391d372 100644 --- a/src/jobservice/logger/factory.go +++ b/src/jobservice/logger/factory.go @@ -62,3 +62,24 @@ func StdFactory(options ...OptionItem) (Interface, error) { return backend.NewStdOutputLogger(level, output, depth), nil } + +// DBFactory is factory of file logger +func DBFactory(options ...OptionItem) (Interface, error) { + var ( + level, key string + depth int + ) + for _, op := range options { + switch op.Field() { + case "level": + level = op.String() + case "key": + key = op.String() + case "depth": + depth = op.Int() + default: + } + } + + return backend.NewDBLogger(key, level, depth) +} diff --git a/src/jobservice/logger/getter/db_getter.go b/src/jobservice/logger/getter/db_getter.go new file mode 100644 index 000000000..e4be8f46e --- /dev/null +++ b/src/jobservice/logger/getter/db_getter.go @@ -0,0 +1,28 @@ +package getter + +import ( + "errors" + "github.com/goharbor/harbor/src/common/dao" +) +// DBGetter is responsible for retrieving DB log data +type DBGetter struct { +} + +// NewDBGetter is constructor of DBGetter +func NewDBGetter() *DBGetter { + return &DBGetter{} +} + +// Retrieve implements @Interface.Retrieve +func (dbg *DBGetter) Retrieve(logID string) ([]byte, error) { + if len(logID) == 0 { + return nil, errors.New("empty log identify") + } + + jobLog, err := dao.GetJobLog(logID) + if err != nil { + return nil, err + } + + return []byte(jobLog.Content), nil +} diff --git a/src/jobservice/logger/getter_factory.go b/src/jobservice/logger/getter_factory.go index fdd04071a..238b2f479 100644 --- a/src/jobservice/logger/getter_factory.go +++ b/src/jobservice/logger/getter_factory.go @@ -25,3 +25,8 @@ func FileGetterFactory(options ...OptionItem) (getter.Interface, error) { return getter.NewFileGetter(baseDir), nil } + +// DBGetterFactory creates a getter for the DB logger +func DBGetterFactory(options ...OptionItem) (getter.Interface, error) { + return getter.NewDBGetter(), nil +} diff --git a/src/jobservice/logger/known_loggers.go b/src/jobservice/logger/known_loggers.go index fb69bcdcf..7480a5476 100644 --- a/src/jobservice/logger/known_loggers.go +++ b/src/jobservice/logger/known_loggers.go @@ -7,6 +7,8 @@ const ( LoggerNameFile = "FILE" // LoggerNameStdOutput is the unique name of the std logger. LoggerNameStdOutput = "STD_OUTPUT" + // LoggerNameDB is the unique name of the DB logger. + LoggerNameDB = "DB" ) // Declaration is used to declare a supported logger. @@ -28,6 +30,8 @@ var knownLoggers = map[string]*Declaration{ LoggerNameFile: {FileFactory, FileSweeperFactory, FileGetterFactory, false}, // STD output(both stdout and stderr) logger LoggerNameStdOutput: {StdFactory, nil, nil, true}, + // DB logger + LoggerNameDB: {DBFactory, DBSweeperFactory, DBGetterFactory, false}, } // IsKnownLogger checks if the logger is supported with name. diff --git a/src/jobservice/logger/sweeper/db_sweeper.go b/src/jobservice/logger/sweeper/db_sweeper.go new file mode 100644 index 000000000..8808b622f --- /dev/null +++ b/src/jobservice/logger/sweeper/db_sweeper.go @@ -0,0 +1,48 @@ +package sweeper + +import ( + "time" + "fmt" + "github.com/goharbor/harbor/src/common/dao" +) + +var dbInit = make(chan int, 1) + +// DBSweeper is used to sweep the DB logs +type DBSweeper struct { + duration int +} + +// NewDBSweeper is constructor of DBSweeper +func NewDBSweeper(duration int) *DBSweeper { + return &DBSweeper{ + duration: duration, + } +} + +// Sweep logs +func (dbs *DBSweeper) Sweep() (int, error) { + // DB initialization not completed, waiting + <-dbInit + + // Start to sweep logs + before := time.Now().Add(time.Duration(dbs.duration) * oneDay * -1) + count, err := dao.DeleteJobLogsBefore(before) + + if err != nil { + return 0, fmt.Errorf("sweep logs in DB failed before %s with error: %s", before, err) + } + + return int(count), nil +} + +// Duration for sweeping +func (dbs *DBSweeper) Duration() int { + return dbs.duration +} + +// prepare sweeping +func PrepareDBSweep() error { + dbInit <- 1 + return nil +} \ No newline at end of file diff --git a/src/jobservice/logger/sweeper_factory.go b/src/jobservice/logger/sweeper_factory.go index 5c45c0f31..44c6a36fb 100644 --- a/src/jobservice/logger/sweeper_factory.go +++ b/src/jobservice/logger/sweeper_factory.go @@ -30,3 +30,19 @@ func FileSweeperFactory(options ...OptionItem) (sweeper.Interface, error) { return sweeper.NewFileSweeper(workDir, duration), nil } + +// DBSweeperFactory creates DB sweeper. +func DBSweeperFactory(options ...OptionItem) (sweeper.Interface, error) { + var duration = 1 + for _, op := range options { + switch op.Field() { + case "duration": + if op.Int() > 0 { + duration = op.Int() + } + default: + } + } + + return sweeper.NewDBSweeper(duration), nil +} From 238dbc03470672732939fe1646ab881891cdbd41 Mon Sep 17 00:00:00 2001 From: peimingming Date: Wed, 28 Nov 2018 14:49:57 +0800 Subject: [PATCH 2/2] Add UT and review comments and issue fix (#6144) Signed-off-by: peimingming --- src/common/dao/joblog.go | 2 +- src/common/dao/joblog_test.go | 44 +++++++++++++ src/common/models/joblog.go | 2 - src/jobservice/job/impl/context.go | 2 +- src/jobservice/logger/backend/db_logger.go | 12 ++-- .../logger/backend/db_logger_test.go | 63 +++++++++++++++++++ src/jobservice/logger/entry.go | 15 ++++- src/jobservice/logger/factory.go | 4 +- src/jobservice/logger/getter/db_getter.go | 1 + src/jobservice/logger/known_loggers.go | 27 +++++++- src/jobservice/logger/known_loggers_test.go | 24 +++++++ src/jobservice/logger/sweeper/db_sweeper.go | 17 +++-- src/jobservice/pool/redis_job_wrapper.go | 5 +- 13 files changed, 199 insertions(+), 19 deletions(-) create mode 100644 src/common/dao/joblog_test.go create mode 100644 src/jobservice/logger/known_loggers_test.go diff --git a/src/common/dao/joblog.go b/src/common/dao/joblog.go index 5eb73f229..7fb0882ea 100644 --- a/src/common/dao/joblog.go +++ b/src/common/dao/joblog.go @@ -1,8 +1,8 @@ package dao import ( - "github.com/goharbor/harbor/src/common/models" "github.com/astaxie/beego/orm" + "github.com/goharbor/harbor/src/common/models" "time" ) diff --git a/src/common/dao/joblog_test.go b/src/common/dao/joblog_test.go new file mode 100644 index 000000000..42d9ff851 --- /dev/null +++ b/src/common/dao/joblog_test.go @@ -0,0 +1,44 @@ +package dao + +import ( + "testing" + + "github.com/goharbor/harbor/src/common/models" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "time" +) + +func TestMethodsOfJobLog(t *testing.T) { + uuid := "uuid_for_unit_test" + now := time.Now() + content := "content for unit text" + jobLog := &models.JobLog{ + UUID: uuid, + CreationTime: now, + Content: content, + } + + // create + _, err := CreateOrUpdateJobLog(jobLog) + require.Nil(t, err) + + // update + updateContent := "content for unit text update" + jobLog.Content = updateContent + _, err = CreateOrUpdateJobLog(jobLog) + require.Nil(t, err) + + // get + log, err := GetJobLog(uuid) + require.Nil(t, err) + assert.Equal(t, now.Second(), log.CreationTime.Second()) + assert.Equal(t, updateContent, log.Content) + assert.Equal(t, jobLog.LogID, log.LogID) + + // delete + count, err := DeleteJobLogsBefore(time.Now().Add(time.Duration(time.Minute))) + require.Nil(t, err) + assert.Equal(t, int64(1), count) +} diff --git a/src/common/models/joblog.go b/src/common/models/joblog.go index 5efbc810f..b004621ee 100644 --- a/src/common/models/joblog.go +++ b/src/common/models/joblog.go @@ -19,5 +19,3 @@ type JobLog struct { func (a *JobLog) TableName() string { return JobLogTable } - - diff --git a/src/jobservice/job/impl/context.go b/src/jobservice/job/impl/context.go index 0f7dbdb91..c47f69167 100644 --- a/src/jobservice/job/impl/context.go +++ b/src/jobservice/job/impl/context.go @@ -30,8 +30,8 @@ import ( "github.com/goharbor/harbor/src/jobservice/env" "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/jobservice/logger" - jmodel "github.com/goharbor/harbor/src/jobservice/models" "github.com/goharbor/harbor/src/jobservice/logger/sweeper" + jmodel "github.com/goharbor/harbor/src/jobservice/models" ) const ( diff --git a/src/jobservice/logger/backend/db_logger.go b/src/jobservice/logger/backend/db_logger.go index 3d8d82127..6887e4f82 100644 --- a/src/jobservice/logger/backend/db_logger.go +++ b/src/jobservice/logger/backend/db_logger.go @@ -1,11 +1,11 @@ package backend import ( - "github.com/goharbor/harbor/src/common/utils/log" "bufio" "bytes" - "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/utils/log" ) // DBLogger is an implementation of logger.Interface. @@ -27,10 +27,10 @@ func NewDBLogger(key string, level string, depth int) (*DBLogger, error) { backendLogger := log.New(bw, log.NewTextFormatter(), logLevel, depth) return &DBLogger{ - backendLogger: backendLogger, - bw: bw, - buffer: buffer, - key: key, + backendLogger: backendLogger, + bw: bw, + buffer: buffer, + key: key, }, nil } diff --git a/src/jobservice/logger/backend/db_logger_test.go b/src/jobservice/logger/backend/db_logger_test.go index 248094365..7b47aa48e 100644 --- a/src/jobservice/logger/backend/db_logger_test.go +++ b/src/jobservice/logger/backend/db_logger_test.go @@ -1 +1,64 @@ package backend + +import ( + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/utils/log" + "github.com/goharbor/harbor/src/jobservice/logger/getter" + "github.com/goharbor/harbor/src/jobservice/logger/sweeper" + "github.com/stretchr/testify/require" + "os" + "testing" +) + +func TestMain(m *testing.M) { + + // databases := []string{"mysql", "sqlite"} + databases := []string{"postgresql"} + for _, database := range databases { + log.Infof("run test cases for database: %s", database) + + result := 1 + switch database { + case "postgresql": + dao.PrepareTestForPostgresSQL() + default: + log.Fatalf("invalid database: %s", database) + } + + result = m.Run() + + if result != 0 { + os.Exit(result) + } + } + +} + +// Test DB logger +func TestDBLogger(t *testing.T) { + uuid := "uuid_for_unit_test" + l, err := NewDBLogger(uuid, "DEBUG", 4) + require.Nil(t, err) + + l.Debug("JobLog Debug: TestDBLogger") + l.Info("JobLog Info: TestDBLogger") + l.Warning("JobLog Warning: TestDBLogger") + l.Error("JobLog Error: TestDBLogger") + l.Debugf("JobLog Debugf: %s", "TestDBLogger") + l.Infof("JobLog Infof: %s", "TestDBLogger") + l.Warningf("JobLog Warningf: %s", "TestDBLogger") + l.Errorf("JobLog Errorf: %s", "TestDBLogger") + + l.Close() + + dbGetter := getter.NewDBGetter() + ll, err := dbGetter.Retrieve(uuid) + require.Nil(t, err) + log.Infof("get logger %s", ll) + + sweeper.PrepareDBSweep() + dbSweeper := sweeper.NewDBSweeper(-1) + count, err := dbSweeper.Sweep() + require.Nil(t, err) + require.Equal(t, 1, count) +} diff --git a/src/jobservice/logger/entry.go b/src/jobservice/logger/entry.go index 32f700183..64c54e93a 100644 --- a/src/jobservice/logger/entry.go +++ b/src/jobservice/logger/entry.go @@ -1,5 +1,7 @@ package logger +import "fmt" + // Entry provides unique interfaces on top of multiple logger backends. // Entry also implements @Interface. type Entry struct { @@ -85,10 +87,21 @@ func (e *Entry) Fatalf(format string, v ...interface{}) { // Close logger func (e *Entry) Close() error { + var errMsg string for _, l := range e.loggers { if closer, ok := l.(Closer); ok { - closer.Close() + err := closer.Close() + if err != nil { + if errMsg == "" { + errMsg = fmt.Sprintf("logger: %s, err: %s", GetLoggerName(l), err) + } else { + errMsg = fmt.Sprintf("%s; logger: %s, err: %s", errMsg, GetLoggerName(l), err) + } + } } } + if errMsg != "" { + return fmt.Errorf(errMsg) + } return nil } diff --git a/src/jobservice/logger/factory.go b/src/jobservice/logger/factory.go index 36391d372..3e16ad72f 100644 --- a/src/jobservice/logger/factory.go +++ b/src/jobservice/logger/factory.go @@ -66,8 +66,8 @@ func StdFactory(options ...OptionItem) (Interface, error) { // DBFactory is factory of file logger func DBFactory(options ...OptionItem) (Interface, error) { var ( - level, key string - depth int + level, key string + depth int ) for _, op := range options { switch op.Field() { diff --git a/src/jobservice/logger/getter/db_getter.go b/src/jobservice/logger/getter/db_getter.go index e4be8f46e..42f5aa4e5 100644 --- a/src/jobservice/logger/getter/db_getter.go +++ b/src/jobservice/logger/getter/db_getter.go @@ -4,6 +4,7 @@ import ( "errors" "github.com/goharbor/harbor/src/common/dao" ) + // DBGetter is responsible for retrieving DB log data type DBGetter struct { } diff --git a/src/jobservice/logger/known_loggers.go b/src/jobservice/logger/known_loggers.go index 7480a5476..4aae1f238 100644 --- a/src/jobservice/logger/known_loggers.go +++ b/src/jobservice/logger/known_loggers.go @@ -1,6 +1,10 @@ package logger -import "strings" +import ( + "github.com/goharbor/harbor/src/jobservice/logger/backend" + "reflect" + "strings" +) const ( // LoggerNameFile is unique name of the file logger. @@ -83,3 +87,24 @@ func IsKnownLevel(level string) bool { return false } + +// GetLoggerName return a logger name by Interface +func GetLoggerName(l Interface) string { + var name string + if l == nil { + return name + } + + switch l.(type) { + case *backend.DBLogger: + name = LoggerNameDB + case *backend.StdOutputLogger: + name = LoggerNameStdOutput + case *backend.FileLogger: + name = LoggerNameFile + default: + name = reflect.TypeOf(l).String() + } + + return name +} diff --git a/src/jobservice/logger/known_loggers_test.go b/src/jobservice/logger/known_loggers_test.go new file mode 100644 index 000000000..9cf4d722c --- /dev/null +++ b/src/jobservice/logger/known_loggers_test.go @@ -0,0 +1,24 @@ +package logger + +import ( + "github.com/goharbor/harbor/src/jobservice/logger/backend" + "github.com/stretchr/testify/require" + "os" + "path" + "testing" +) + +// Test GetLoggerName +func TestGetLoggerName(t *testing.T) { + uuid := "uuid_for_unit_test" + l, err := backend.NewDBLogger(uuid, "DEBUG", 4) + require.Nil(t, err) + require.Equal(t, LoggerNameDB, GetLoggerName(l)) + + stdLog := backend.NewStdOutputLogger("DEBUG", backend.StdErr, 4) + require.Equal(t, LoggerNameStdOutput, GetLoggerName(stdLog)) + + fileLog, err := backend.NewFileLogger("DEBUG", path.Join(os.TempDir(), "TestFileLogger.log"), 4) + require.Nil(t, err) + require.Equal(t, LoggerNameFile, GetLoggerName(fileLog)) +} diff --git a/src/jobservice/logger/sweeper/db_sweeper.go b/src/jobservice/logger/sweeper/db_sweeper.go index 8808b622f..763d531b6 100644 --- a/src/jobservice/logger/sweeper/db_sweeper.go +++ b/src/jobservice/logger/sweeper/db_sweeper.go @@ -1,12 +1,13 @@ package sweeper import ( - "time" "fmt" "github.com/goharbor/harbor/src/common/dao" + "time" ) var dbInit = make(chan int, 1) +var isDBInit = false // DBSweeper is used to sweep the DB logs type DBSweeper struct { @@ -23,7 +24,7 @@ func NewDBSweeper(duration int) *DBSweeper { // Sweep logs func (dbs *DBSweeper) Sweep() (int, error) { // DB initialization not completed, waiting - <-dbInit + WaitingDBInit() // Start to sweep logs before := time.Now().Add(time.Duration(dbs.duration) * oneDay * -1) @@ -41,8 +42,16 @@ func (dbs *DBSweeper) Duration() int { return dbs.duration } -// prepare sweeping +// WaitingDBInit waiting DB init +func WaitingDBInit() { + if !isDBInit { + <-dbInit + } +} + +// PrepareDBSweep invoked after DB init func PrepareDBSweep() error { + isDBInit = true dbInit <- 1 return nil -} \ No newline at end of file +} diff --git a/src/jobservice/pool/redis_job_wrapper.go b/src/jobservice/pool/redis_job_wrapper.go index 052561816..2584f5567 100644 --- a/src/jobservice/pool/redis_job_wrapper.go +++ b/src/jobservice/pool/redis_job_wrapper.go @@ -107,7 +107,10 @@ func (rj *RedisJob) Run(j *work.Job) error { defer func() { // Close open io stream first if closer, ok := execContext.GetLogger().(logger.Closer); ok { - closer.Close() + err := closer.Close() + if err != nil { + logger.Errorf("Close job logger failed: %s", err) + } } }()