From bb53cc1e4a0ca59f9b112f8cbe759aa37228f71a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 18 Feb 2022 17:54:43 +0100 Subject: [PATCH] Task log storage service --- Makefile | 4 +- cmd/flamenco-manager-poc/main.go | 4 +- go.mod | 2 + go.sum | 2 + internal/manager/api_impl/api_impl.go | 13 +- .../api_impl/mocks/api_impl_mock.gen.go | 52 ++++++- internal/manager/api_impl/test_support.go | 3 +- internal/manager/task_logs/log_rotation.go | 111 ++++++++++++++ .../manager/task_logs/log_rotation_test.go | 142 ++++++++++++++++++ internal/manager/task_logs/task_logs.go | 111 ++++++++++++++ internal/manager/task_logs/task_logs_test.go | 95 ++++++++++++ 11 files changed, 532 insertions(+), 7 deletions(-) create mode 100644 internal/manager/task_logs/log_rotation.go create mode 100644 internal/manager/task_logs/log_rotation_test.go create mode 100644 internal/manager/task_logs/task_logs.go create mode 100644 internal/manager/task_logs/task_logs_test.go diff --git a/Makefile b/Makefile index bf4d65f6..8aeb00a9 100644 --- a/Makefile +++ b/Makefile @@ -25,8 +25,8 @@ all: application # Install generators and build the software. with-deps: - go install github.com/deepmap/oapi-codegen/cmd/oapi-codegen - go install github.com/golang/mock/mockgen@v1.6.0 + go get github.com/deepmap/oapi-codegen/cmd/oapi-codegen@v1.9.0 + go get github.com/golang/mock/mockgen@v1.6.0 make -s application application: ${RESOURCES} generate flamenco-manager-poc flamenco-worker-poc socketio-poc diff --git a/cmd/flamenco-manager-poc/main.go b/cmd/flamenco-manager-poc/main.go index 14d1e751..981c08e1 100644 --- a/cmd/flamenco-manager-poc/main.go +++ b/cmd/flamenco-manager-poc/main.go @@ -39,6 +39,7 @@ import ( "gitlab.com/blender/flamenco-ng-poc/internal/manager/job_compilers" "gitlab.com/blender/flamenco-ng-poc/internal/manager/persistence" "gitlab.com/blender/flamenco-ng-poc/internal/manager/swagger_ui" + "gitlab.com/blender/flamenco-ng-poc/internal/manager/task_logs" "gitlab.com/blender/flamenco-ng-poc/pkg/api" ) @@ -67,7 +68,8 @@ func main() { if err != nil { log.Fatal().Err(err).Msg("error loading job compilers") } - flamenco := api_impl.NewFlamenco(compiler, persist) + logStorage := task_logs.NewStorage("./task-logs") // TODO: load job storage path from configuration. + flamenco := api_impl.NewFlamenco(compiler, persist, logStorage) e := buildWebService(flamenco, persist) // Start the web server. diff --git a/go.mod b/go.mod index e68aa1e6..268cc059 100644 --- a/go.mod +++ b/go.mod @@ -52,9 +52,11 @@ require ( github.com/shopspring/decimal v1.2.0 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect github.com/valyala/fasttemplate v1.2.1 // indirect + golang.org/x/mod v0.4.2 // indirect golang.org/x/sys v0.0.0-20211103235746-7861aae1554b // indirect golang.org/x/text v0.3.7 // indirect golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba // indirect + golang.org/x/tools v0.1.7 // indirect golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect ) diff --git a/go.sum b/go.sum index 5fec5fb4..b27ed44d 100644 --- a/go.sum +++ b/go.sum @@ -259,6 +259,7 @@ golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHl golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -325,6 +326,7 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20200918232735-d647fc253266/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU= golang.org/x/tools v0.0.0-20210114065538-d78b04bdf963/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= +golang.org/x/tools v0.1.7 h1:6j8CgantCy3yc8JGBqkDLMKWqZ0RDU2g1HVgacojGWQ= golang.org/x/tools v0.1.7/go.mod h1:LGqMHiF4EqQNHR1JncWGqT5BVaXmza+X+BDGol+dOxo= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/manager/api_impl/api_impl.go b/internal/manager/api_impl/api_impl.go index b427839c..5ea37a53 100644 --- a/internal/manager/api_impl/api_impl.go +++ b/internal/manager/api_impl/api_impl.go @@ -26,6 +26,7 @@ import ( "fmt" "github.com/labstack/echo/v4" + "github.com/rs/zerolog" "gitlab.com/blender/flamenco-ng-poc/internal/manager/job_compilers" "gitlab.com/blender/flamenco-ng-poc/internal/manager/persistence" "gitlab.com/blender/flamenco-ng-poc/pkg/api" @@ -34,10 +35,11 @@ import ( type Flamenco struct { jobCompiler JobCompiler persist PersistenceService + logStorage LogStorage } // Generate mock implementations of these interfaces. -//go:generate go run github.com/golang/mock/mockgen -destination mocks/api_impl_mock.gen.go -package mocks gitlab.com/blender/flamenco-ng-poc/internal/manager/api_impl PersistenceService,JobCompiler +//go:generate go run github.com/golang/mock/mockgen -destination mocks/api_impl_mock.gen.go -package mocks gitlab.com/blender/flamenco-ng-poc/internal/manager/api_impl PersistenceService,JobCompiler,LogStorage type PersistenceService interface { StoreAuthoredJob(ctx context.Context, authoredJob job_compilers.AuthoredJob) error @@ -60,13 +62,20 @@ type JobCompiler interface { Compile(ctx context.Context, job api.SubmittedJob) (*job_compilers.AuthoredJob, error) } +// LogStorage handles incoming task logs. +type LogStorage interface { + Write(logger zerolog.Logger, jobID, taskID string, logText string) error + RotateFile(logger zerolog.Logger, jobID, taskID string) +} + var _ api.ServerInterface = (*Flamenco)(nil) // NewFlamenco creates a new Flamenco service, using the given JobCompiler. -func NewFlamenco(jc JobCompiler, jps PersistenceService) *Flamenco { +func NewFlamenco(jc JobCompiler, jps PersistenceService, ls LogStorage) *Flamenco { return &Flamenco{ jobCompiler: jc, persist: jps, + logStorage: ls, } } diff --git a/internal/manager/api_impl/mocks/api_impl_mock.gen.go b/internal/manager/api_impl/mocks/api_impl_mock.gen.go index d0cd5baf..d505e61d 100644 --- a/internal/manager/api_impl/mocks/api_impl_mock.gen.go +++ b/internal/manager/api_impl/mocks/api_impl_mock.gen.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: gitlab.com/blender/flamenco-ng-poc/internal/manager/api_impl (interfaces: PersistenceService,JobCompiler) +// Source: gitlab.com/blender/flamenco-ng-poc/internal/manager/api_impl (interfaces: PersistenceService,JobCompiler,LogStorage) // Package mocks is a generated GoMock package. package mocks @@ -9,6 +9,7 @@ import ( reflect "reflect" gomock "github.com/golang/mock/gomock" + zerolog "github.com/rs/zerolog" job_compilers "gitlab.com/blender/flamenco-ng-poc/internal/manager/job_compilers" persistence "gitlab.com/blender/flamenco-ng-poc/internal/manager/persistence" api "gitlab.com/blender/flamenco-ng-poc/pkg/api" @@ -218,3 +219,52 @@ func (mr *MockJobCompilerMockRecorder) ListJobTypes() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListJobTypes", reflect.TypeOf((*MockJobCompiler)(nil).ListJobTypes)) } + +// MockLogStorage is a mock of LogStorage interface. +type MockLogStorage struct { + ctrl *gomock.Controller + recorder *MockLogStorageMockRecorder +} + +// MockLogStorageMockRecorder is the mock recorder for MockLogStorage. +type MockLogStorageMockRecorder struct { + mock *MockLogStorage +} + +// NewMockLogStorage creates a new mock instance. +func NewMockLogStorage(ctrl *gomock.Controller) *MockLogStorage { + mock := &MockLogStorage{ctrl: ctrl} + mock.recorder = &MockLogStorageMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockLogStorage) EXPECT() *MockLogStorageMockRecorder { + return m.recorder +} + +// RotateFile mocks base method. +func (m *MockLogStorage) RotateFile(arg0 zerolog.Logger, arg1, arg2 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "RotateFile", arg0, arg1, arg2) +} + +// RotateFile indicates an expected call of RotateFile. +func (mr *MockLogStorageMockRecorder) RotateFile(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RotateFile", reflect.TypeOf((*MockLogStorage)(nil).RotateFile), arg0, arg1, arg2) +} + +// Write mocks base method. +func (m *MockLogStorage) Write(arg0 zerolog.Logger, arg1, arg2, arg3 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Write", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(error) + return ret0 +} + +// Write indicates an expected call of Write. +func (mr *MockLogStorageMockRecorder) Write(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Write", reflect.TypeOf((*MockLogStorage)(nil).Write), arg0, arg1, arg2, arg3) +} diff --git a/internal/manager/api_impl/test_support.go b/internal/manager/api_impl/test_support.go index a7833174..bce71f18 100644 --- a/internal/manager/api_impl/test_support.go +++ b/internal/manager/api_impl/test_support.go @@ -44,7 +44,8 @@ type mockedFlamenco struct { func newMockedFlamenco(mockCtrl *gomock.Controller) mockedFlamenco { jc := mocks.NewMockJobCompiler(mockCtrl) ps := mocks.NewMockPersistenceService(mockCtrl) - f := NewFlamenco(jc, ps) + ls := mocks.NewMockLogStorage(mockCtrl) + f := NewFlamenco(jc, ps, ls) return mockedFlamenco{ flamenco: f, diff --git a/internal/manager/task_logs/log_rotation.go b/internal/manager/task_logs/log_rotation.go new file mode 100644 index 00000000..b76f510a --- /dev/null +++ b/internal/manager/task_logs/log_rotation.go @@ -0,0 +1,111 @@ +package task_logs + +/* ***** BEGIN GPL LICENSE BLOCK ***** + * + * Original Code Copyright (C) 2022 Blender Foundation. + * + * This file is part of Flamenco. + * + * Flamenco is free software: you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation, either version 3 of the License, or (at your option) any later + * version. + * + * Flamenco is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR + * A PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * Flamenco. If not, see . + * + * ***** END GPL LICENSE BLOCK ***** */ + +import ( + "os" + "path/filepath" + "sort" + "strconv" + "strings" + + "github.com/rs/zerolog" +) + +type numberedPath struct { + path string + number int + basepath string +} + +// byNumber implements the sort.Interface for numberedPath objects, +// and sorts in reverse (so highest number first). +type byNumber []numberedPath + +func (a byNumber) Len() int { return len(a) } +func (a byNumber) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a byNumber) Less(i, j int) bool { return a[i].number > a[j].number } + +func createNumberedPath(path string) numberedPath { + dotIndex := strings.LastIndex(path, ".") + if dotIndex < 0 { + return numberedPath{path, -1, path} + } + asInt, err := strconv.Atoi(path[dotIndex+1:]) + if err != nil { + return numberedPath{path, -1, path} + } + return numberedPath{path, asInt, path[:dotIndex]} +} + +// rotateLogFile renames 'logpath' to 'logpath.1', and increases numbers for already-existing files. +// NOTE: not thread-safe when calling with the same `logpath`. +func rotateLogFile(logger zerolog.Logger, logpath string) error { + // Don't do anything if the file doesn't exist yet. + _, err := os.Stat(logpath) + if err != nil { + if os.IsNotExist(err) { + logger.Debug().Msg("log file does not exist, no need to rotate") + return nil + } + logger.Warn().Err(err).Msg("unable to stat logfile") + return err + } + + // Rotate logpath.3 to logpath.2, logpath.1 to logpath.2, etc. + pattern := logpath + ".*" + existing, err := filepath.Glob(pattern) + if err != nil { + logger.Warn().Err(err).Str("glob", pattern).Msg("rotateLogFile: unable to glob") + return err + } + if existing == nil { + logger.Debug().Msg("rotateLogFile: no existing files to rotate") + } else { + // Rotate the files in reverse numerical order (so renaming n→n+1 comes after n+1→n+2) + var numbered = make(byNumber, len(existing)) + for idx := range existing { + numbered[idx] = createNumberedPath(existing[idx]) + } + sort.Sort(numbered) + + for _, numberedPath := range numbered { + newName := numberedPath.basepath + "." + strconv.Itoa(numberedPath.number+1) + err := os.Rename(numberedPath.path, newName) + if err != nil { + logger.Error(). + Str("from_path", numberedPath.path). + Str("to_path", newName). + Err(err). + Msg("rotateLogFile: unable to rename log file") + } + } + } + + // Rotate the pointed-to file. + newName := logpath + ".1" + if err := os.Rename(logpath, newName); err != nil { + logger.Error().Str("new_name", newName).Err(err).Msg("rotateLogFile: unable to rename log file for rotating") + return err + } + + return nil +} diff --git a/internal/manager/task_logs/log_rotation_test.go b/internal/manager/task_logs/log_rotation_test.go new file mode 100644 index 00000000..b1732c52 --- /dev/null +++ b/internal/manager/task_logs/log_rotation_test.go @@ -0,0 +1,142 @@ +package task_logs + +/* ***** BEGIN GPL LICENSE BLOCK ***** + * + * Original Code Copyright (C) 2022 Blender Foundation. + * + * This file is part of Flamenco. + * + * Flamenco is free software: you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation, either version 3 of the License, or (at your option) any later + * version. + * + * Flamenco is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR + * A PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * Flamenco. If not, see . + * + * ***** END GPL LICENSE BLOCK ***** */ + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" +) + +func setUpTest(t *testing.T) string { + temppath, err := ioutil.TempDir("", "testlogs") + assert.NoError(t, err) + return temppath +} + +func tearDownTest(temppath string) { + os.RemoveAll(temppath) +} + +func TestCreateNumberedPath(t *testing.T) { + temppath := setUpTest(t) + defer tearDownTest(temppath) + + numtest := func(path string, number int, basepath string) { + result := createNumberedPath(path) + assert.Equal(t, numberedPath{path, number, basepath}, result) + } + + numtest("", -1, "") + numtest(" ", -1, " ") + numtest("jemoeder.1", 1, "jemoeder") + numtest("jemoeder.", -1, "jemoeder.") + numtest("jemoeder", -1, "jemoeder") + numtest("jemoeder.abc", -1, "jemoeder.abc") + numtest("jemoeder.-4", -4, "jemoeder") + numtest("jemoeder.1.2.3", 3, "jemoeder.1.2") + numtest("jemoeder.001", 1, "jemoeder") + numtest("jemoeder.01", 1, "jemoeder") + numtest("jemoeder.010", 10, "jemoeder") + numtest("jemoeder 47 42.327", 327, "jemoeder 47 42") + numtest("/path/üničøde.327/.47", 47, "/path/üničøde.327/") + numtest("üničøde.327.what?", -1, "üničøde.327.what?") +} + +func TestNoFiles(t *testing.T) { + temppath := setUpTest(t) + defer tearDownTest(temppath) + + filepath := filepath.Join(temppath, "nonexisting.txt") + err := rotateLogFile(zerolog.Nop(), filepath) + assert.NoError(t, err) + assert.False(t, fileExists(filepath)) +} + +func TestOneFile(t *testing.T) { + temppath := setUpTest(t) + defer tearDownTest(temppath) + + filepath := filepath.Join(temppath, "existing.txt") + fileTouch(filepath) + + err := rotateLogFile(zerolog.Nop(), filepath) + assert.NoError(t, err) + assert.False(t, fileExists(filepath)) + assert.True(t, fileExists(filepath+".1")) +} + +func TestMultipleFilesWithHoles(t *testing.T) { + temppath := setUpTest(t) + defer tearDownTest(temppath) + + filepath := filepath.Join(temppath, "existing.txt") + assert.NoError(t, ioutil.WriteFile(filepath, []byte("thefile"), 0666)) + assert.NoError(t, ioutil.WriteFile(filepath+".1", []byte("file .1"), 0666)) + assert.NoError(t, ioutil.WriteFile(filepath+".2", []byte("file .2"), 0666)) + assert.NoError(t, ioutil.WriteFile(filepath+".3", []byte("file .3"), 0666)) + assert.NoError(t, ioutil.WriteFile(filepath+".5", []byte("file .5"), 0666)) + assert.NoError(t, ioutil.WriteFile(filepath+".7", []byte("file .7"), 0666)) + + err := rotateLogFile(zerolog.Nop(), filepath) + + assert.NoError(t, err) + assert.False(t, fileExists(filepath)) + assert.True(t, fileExists(filepath+".1")) + assert.True(t, fileExists(filepath+".2")) + assert.True(t, fileExists(filepath+".3")) + assert.True(t, fileExists(filepath+".4")) + assert.False(t, fileExists(filepath+".5")) + assert.True(t, fileExists(filepath+".6")) + assert.False(t, fileExists(filepath+".7")) + assert.True(t, fileExists(filepath+".8")) + assert.False(t, fileExists(filepath+".9")) + + read := func(filename string) string { + content, err := ioutil.ReadFile(filename) + assert.NoError(t, err) + return string(content) + } + + assert.Equal(t, "thefile", read(filepath+".1")) + assert.Equal(t, "file .1", read(filepath+".2")) + assert.Equal(t, "file .2", read(filepath+".3")) + assert.Equal(t, "file .3", read(filepath+".4")) + assert.Equal(t, "file .5", read(filepath+".6")) + assert.Equal(t, "file .7", read(filepath+".8")) +} + +func fileExists(filename string) bool { + _, err := os.Stat(filename) + return !os.IsNotExist(err) +} + +func fileTouch(filename string) { + file, err := os.OpenFile(filename, os.O_CREATE|os.O_APPEND|os.O_RDONLY, 0666) + if err != nil { + panic(err.Error()) + } + file.Close() +} diff --git a/internal/manager/task_logs/task_logs.go b/internal/manager/task_logs/task_logs.go new file mode 100644 index 00000000..4916a9fd --- /dev/null +++ b/internal/manager/task_logs/task_logs.go @@ -0,0 +1,111 @@ +package task_logs + +/* ***** BEGIN GPL LICENSE BLOCK ***** + * + * Original Code Copyright (C) 2022 Blender Foundation. + * + * This file is part of Flamenco. + * + * Flamenco is free software: you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation, either version 3 of the License, or (at your option) any later + * version. + * + * Flamenco is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR + * A PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * Flamenco. If not, see . + * + * ***** END GPL LICENSE BLOCK ***** */ + +import ( + "fmt" + "os" + "path" + + "github.com/rs/zerolog" +) + +// Storage can write data to task logs, rotate logs, etc. +type Storage struct { + BasePath string // Directory where task logs are stored. +} + +// NewStorage creates a new log storage rooted at `basePath`. +func NewStorage(basePath string) *Storage { + return &Storage{ + BasePath: basePath, + } +} + +func (s *Storage) Write(logger zerolog.Logger, jobID, taskID string, logText string) error { + // Shortcut to avoid creating an empty log file. It also solves an + // index out of bounds error further down when we check the last character. + if logText == "" { + return nil + } + + filepath := s.filepath(jobID, taskID) + logger = logger.With().Str("filepath", filepath).Logger() + + if err := os.MkdirAll(path.Dir(filepath), 0755); err != nil { + logger.Error().Err(err).Msg("unable to create directory for log file") + return fmt.Errorf("error creating directory: %w", err) + } + + file, err := os.OpenFile(filepath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + logger.Error().Err(err).Msg("unable to open log file for append/create/write") + return fmt.Errorf("unable to open log file for append/create/write: %w", err) + } + + if n, err := file.WriteString(logText); n < len(logText) || err != nil { + logger.Error(). + Int("written", n). + Int("totalLength", len(logText)). + Err(err). + Msg("could only write partial log file") + file.Close() + return fmt.Errorf("could only write partial log file: %w", err) + } + + if logText[len(logText)-1] != '\n' { + if n, err := file.WriteString("\n"); n < 1 || err != nil { + logger.Error().Err(err).Msg("could not append line end") + file.Close() + return err + } + } + + if err := file.Close(); err != nil { + logger.Error().Err(err).Msg("error closing log file") + return err + } + return nil +} + +// RotateFile rotates the task's log file, ignoring (but logging) any errors that occur. +func (s *Storage) RotateFile(logger zerolog.Logger, jobID, taskID string) { + logpath := s.filepath(jobID, taskID) + logger = logger.With().Str("logpath", logpath).Logger() + + err := rotateLogFile(logger, logpath) + if err != nil { + // rotateLogFile() has already logged something, so we can ignore `err`. + logger.Trace().Err(err).Msg("ignoring error from log rotation") + } +} + +// filepath returns the file path suitable to write a log file. +func (s *Storage) filepath(jobID, taskID string) string { + var dirpath string + if jobID == "" { + dirpath = path.Join(s.BasePath, "jobless") + } else { + dirpath = path.Join(s.BasePath, "job-"+jobID[:4], jobID) + } + filename := fmt.Sprintf("task-%v.txt", taskID) + return path.Join(dirpath, filename) +} diff --git a/internal/manager/task_logs/task_logs_test.go b/internal/manager/task_logs/task_logs_test.go new file mode 100644 index 00000000..aad30615 --- /dev/null +++ b/internal/manager/task_logs/task_logs_test.go @@ -0,0 +1,95 @@ +package task_logs + +/* ***** BEGIN GPL LICENSE BLOCK ***** + * + * Original Code Copyright (C) 2022 Blender Foundation. + * + * This file is part of Flamenco. + * + * Flamenco is free software: you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation, either version 3 of the License, or (at your option) any later + * version. + * + * Flamenco is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR + * A PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * Flamenco. If not, see . + * + * ***** END GPL LICENSE BLOCK ***** */ + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" +) + +func tempStorage() *Storage { + temppath, err := ioutil.TempDir("", "testlogs") + if err != nil { + panic(err) + } + return &Storage{temppath} +} + +func TestLogWriting(t *testing.T) { + s := tempStorage() + defer os.RemoveAll(s.BasePath) + + err := s.Write(zerolog.Nop(), + "25c5a51c-e0dd-44f7-9f87-74f3d1fbbd8c", + "20ff9d06-53ec-4019-9e2e-1774f05f170a", + "Ovo je priča") + assert.NoError(t, err) + + err = s.Write(zerolog.Nop(), + "25c5a51c-e0dd-44f7-9f87-74f3d1fbbd8c", + "20ff9d06-53ec-4019-9e2e-1774f05f170a", + "Ima dvije linije") + assert.NoError(t, err) + + filename := filepath.Join( + s.BasePath, + "job-25c5", + "25c5a51c-e0dd-44f7-9f87-74f3d1fbbd8c", + "task-20ff9d06-53ec-4019-9e2e-1774f05f170a.txt") + + contents, err := ioutil.ReadFile(filename) + assert.NoError(t, err, "the log file should exist") + assert.Equal(t, "Ovo je priča\nIma dvije linije\n", string(contents)) +} + +func TestLogRotation(t *testing.T) { + s := tempStorage() + defer os.RemoveAll(s.BasePath) + + err := s.Write(zerolog.Nop(), + "25c5a51c-e0dd-44f7-9f87-74f3d1fbbd8c", + "20ff9d06-53ec-4019-9e2e-1774f05f170a", + "Ovo je priča") + assert.NoError(t, err) + + s.RotateFile(zerolog.Nop(), + "25c5a51c-e0dd-44f7-9f87-74f3d1fbbd8c", + "20ff9d06-53ec-4019-9e2e-1774f05f170a") + + filename := filepath.Join( + s.BasePath, + "job-25c5", + "25c5a51c-e0dd-44f7-9f87-74f3d1fbbd8c", + "task-20ff9d06-53ec-4019-9e2e-1774f05f170a.txt") + rotatedFilename := filename + ".1" + + contents, err := ioutil.ReadFile(rotatedFilename) + assert.NoError(t, err, "the rotated log file should exist") + assert.Equal(t, "Ovo je priča\n", string(contents)) + + _, err = os.Stat(filename) + assert.True(t, os.IsNotExist(err)) +}