From 45bf8600ef5abcbdbb5b1e177f94810f8db3de76 Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Sun, 14 Jan 2024 10:54:02 +0100 Subject: [PATCH] Remove multipart logger (#3200) --- agent/logger.go | 11 +-- cli/exec/exec.go | 10 +-- pipeline/logger.go | 16 +--- pipeline/multipart/reader.go | 117 ------------------------------ pipeline/multipart/reader_test.go | 90 ----------------------- pipeline/pipeline.go | 35 ++++----- server/pipeline/cancel.go | 6 +- 7 files changed, 24 insertions(+), 261 deletions(-) delete mode 100644 pipeline/multipart/reader.go delete mode 100644 pipeline/multipart/reader_test.go diff --git a/agent/logger.go b/agent/logger.go index bfa4baeb8..99541ede0 100644 --- a/agent/logger.go +++ b/agent/logger.go @@ -23,21 +23,16 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline" backend "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" - "go.woodpecker-ci.org/woodpecker/v2/pipeline/multipart" "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" ) -func (r *Runner) createLogger(logger zerolog.Logger, uploads *sync.WaitGroup, workflow *rpc.Workflow) pipeline.LogFunc { - return func(step *backend.Step, rc multipart.Reader) error { +func (r *Runner) createLogger(logger zerolog.Logger, uploads *sync.WaitGroup, workflow *rpc.Workflow) pipeline.Logger { + return func(step *backend.Step, rc io.Reader) error { loglogger := logger.With(). Str("image", step.Image). Str("workflowID", workflow.ID). Logger() - part, rerr := rc.NextPart() - if rerr != nil { - return rerr - } uploads.Add(1) var secrets []string @@ -50,7 +45,7 @@ func (r *Runner) createLogger(logger zerolog.Logger, uploads *sync.WaitGroup, wo loglogger.Debug().Msg("log stream opened") logStream := rpc.NewLineWriter(r.client, step.UUID, secrets...) - if _, err := io.Copy(logStream, part); err != nil { + if _, err := io.Copy(logStream, rc); err != nil { log.Error().Err(err).Msg("copy limited logStream part") } diff --git a/cli/exec/exec.go b/cli/exec/exec.go index 38394357c..7a2859316 100644 --- a/cli/exec/exec.go +++ b/cli/exec/exec.go @@ -39,7 +39,6 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/compiler" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/linter" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/matrix" - "go.woodpecker-ci.org/woodpecker/v2/pipeline/multipart" "go.woodpecker-ci.org/woodpecker/v2/shared/utils" ) @@ -253,13 +252,8 @@ func convertPathForWindows(path string) string { return filepath.ToSlash(path) } -var defaultLogger = pipeline.LogFunc(func(step *backendTypes.Step, rc multipart.Reader) error { - part, err := rc.NextPart() - if err != nil { - return err - } - +var defaultLogger = pipeline.Logger(func(step *backendTypes.Step, rc io.Reader) error { logStream := NewLineWriter(step.Name, step.UUID) - _, err = io.Copy(logStream, part) + _, err := io.Copy(logStream, rc) return err }) diff --git a/pipeline/logger.go b/pipeline/logger.go index 043dd798f..c557e0e94 100644 --- a/pipeline/logger.go +++ b/pipeline/logger.go @@ -15,20 +15,10 @@ package pipeline import ( + "io" + backend "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" - "go.woodpecker-ci.org/woodpecker/v2/pipeline/multipart" ) // Logger handles the process logging. -type Logger interface { - Log(*backend.Step, multipart.Reader) error -} - -// LogFunc type is an adapter to allow the use of an ordinary -// function for process logging. -type LogFunc func(*backend.Step, multipart.Reader) error - -// Log calls f(step, r). -func (f LogFunc) Log(step *backend.Step, r multipart.Reader) error { - return f(step, r) -} +type Logger func(*backend.Step, io.Reader) error diff --git a/pipeline/multipart/reader.go b/pipeline/multipart/reader.go deleted file mode 100644 index ab35b6f4c..000000000 --- a/pipeline/multipart/reader.go +++ /dev/null @@ -1,117 +0,0 @@ -// Copyright 2023 Woodpecker Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package multipart - -import ( - "bufio" - "bytes" - "io" - "mime/multipart" - "net/textproto" -) - -type ( - // Reader is an iterator over parts in a multipart log stream. - Reader interface { - // NextPart returns the next part in the multipart or - // an error. When there are no more parts, the error - // io.EOF is returned. - NextPart() (Part, error) - } - - // A Part represents a single part in a multipart body. - Part interface { - io.Reader - - // Header returns the headers of the body with the - // keys canonicalized. - Header() textproto.MIMEHeader - - // FileName returns the filename parameter of the - // Content-Disposition header. - FileName() string - - // FormName returns the name parameter if p has a - // Content-Disposition of type form-data. - FormName() string - } -) - -// New returns a new multipart Reader. -func New(r io.Reader) Reader { - buf := bufio.NewReader(r) - out, _ := buf.Peek(8) - - if bytes.Equal(out, []byte("PIPELINE")) { - return &multipartReader{ - reader: multipart.NewReader(buf, "boundary"), - } - } - return &textReader{ - reader: buf, - } -} - -// -// wraps the stdlib multi-part reader -// - -type multipartReader struct { - reader *multipart.Reader -} - -func (r *multipartReader) NextPart() (Part, error) { - next, err := r.reader.NextPart() - if err != nil { - return nil, err - } - part := new(part) - part.Reader = next - part.filename = next.FileName() - part.formname = next.FormName() - part.header = next.Header - return part, nil -} - -// -// wraps a simple io.Reader to satisfy the multi-part interface -// - -type textReader struct { - reader io.Reader - done bool -} - -func (r *textReader) NextPart() (Part, error) { - if r.done { - return nil, io.EOF - } - r.done = true - p := new(part) - p.Reader = r.reader - return p, nil -} - -type part struct { - io.Reader - - filename string - formname string - header textproto.MIMEHeader -} - -func (p *part) Header() textproto.MIMEHeader { return p.header } -func (p *part) FileName() string { return p.filename } -func (p *part) FormName() string { return p.filename } diff --git a/pipeline/multipart/reader_test.go b/pipeline/multipart/reader_test.go deleted file mode 100644 index c60532503..000000000 --- a/pipeline/multipart/reader_test.go +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright 2023 Woodpecker Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package multipart - -import ( - "bytes" - "io" - "testing" -) - -func TestReader(t *testing.T) { - b := bytes.NewBufferString(sample) - m := New(b) - - part, err := m.NextPart() - if err != nil { - t.Error(err) - return - } - - header := part.Header() - if got, want := header.Get("Content-Type"), "text/plain"; got != want { - t.Errorf("Want Content-Type %q, got %q", want, got) - } - body, err := io.ReadAll(part) - if err != nil { - t.Error(err) - return - } - if got, want := string(body), sampleTextPlain; got != want { - t.Errorf("Want body %q, got %q", want, got) - } - - part, err = m.NextPart() - if err != nil { - t.Error(err) - return - } - header = part.Header() - if got, want := header.Get("Content-Type"), "application/json+coverage"; got != want { - t.Errorf("Want Content-Type %q, got %q", want, got) - } - if got, want := header.Get("X-Covered"), "96.00"; got != want { - t.Errorf("Want X-Covered %q, got %q", want, got) - } - if got, want := header.Get("X-Covered-Lines"), "96"; got != want { - t.Errorf("Want X-Covered-Lines %q, got %q", want, got) - } - if got, want := header.Get("X-Total-Lines"), "100"; got != want { - t.Errorf("Want X-Total-Lines %q, got %q", want, got) - } -} - -var sample = `PIPELINE -Content-Type: multipart/mixed; boundary=boundary - ---boundary -Content-Type: text/plain - -match: pipeline/frontend/yaml/compiler/coverage.out -match: pipeline/frontend/yaml/coverage.out -match: pipeline/frontend/yaml/linter/coverage.out - ---boundary -Content-Type: application/json+coverage -X-Covered: 96.00 -X-Covered-Lines: 96 -X-Total-Lines: 100 - -{"metrics":{"covered_lines":96,"total_lines":100}} - ---boundary-- -` - -var sampleTextPlain = `match: pipeline/frontend/yaml/compiler/coverage.out -match: pipeline/frontend/yaml/coverage.out -match: pipeline/frontend/yaml/linter/coverage.out -` diff --git a/pipeline/pipeline.go b/pipeline/pipeline.go index 8be2338aa..a68277f42 100644 --- a/pipeline/pipeline.go +++ b/pipeline/pipeline.go @@ -28,7 +28,6 @@ import ( backend "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/metadata" - "go.woodpecker-ci.org/woodpecker/v2/pipeline/multipart" ) // TODO: move runtime into "runtime" subpackage @@ -89,7 +88,7 @@ func (r *Runtime) MakeLogger() zerolog.Logger { return logCtx.Logger() } -// Starts the execution of an workflow and waits for it to complete +// Run starts the execution of a workflow and waits for it to complete func (r *Runtime) Run(runnerCtx context.Context) error { logger := r.MakeLogger() logger.Debug().Msgf("executing %d stages, in order of:", len(r.spec.Stages)) @@ -166,27 +165,27 @@ func (r *Runtime) execAll(steps []*backend.Step) <-chan error { logger := r.MakeLogger() for _, step := range steps { - // required since otherwise the loop variable + // Required since otherwise the loop variable // will be captured by the function. This will // recreate the step "variable" step := step g.Go(func() error { // Case the pipeline was already complete. logger.Debug(). - Str("Step", step.Name). - Msg("Prepare") + Str("step", step.Name). + Msg("prepare") switch { case r.err != nil && !step.OnFailure: logger.Debug(). - Str("Step", step.Name). + Str("step", step.Name). Err(r.err). - Msgf("Skipped due to OnFailure=%t", step.OnFailure) + Msgf("skipped due to OnFailure=%t", step.OnFailure) return nil case r.err == nil && !step.OnSuccess: logger.Debug(). - Str("Step", step.Name). - Msgf("Skipped due to OnSuccess=%t", step.OnSuccess) + Str("step", step.Name). + Msgf("skipped due to OnSuccess=%t", step.OnSuccess) return nil } @@ -200,22 +199,14 @@ func (r *Runtime) execAll(steps []*backend.Step) <-chan error { metadata.SetDroneEnviron(step.Environment) logger.Debug(). - Str("Step", step.Name). - Msg("Executing") + Str("step", step.Name). + Msg("executing") processState, err := r.exec(step) logger.Debug(). - Str("Step", step.Name). - Msg("Complete") - - // if we got a nil process but an error state - // then we need to log the internal error to the step. - if r.logger != nil && err != nil && !errors.Is(err, ErrCancel) && processState == nil { - _ = r.logger.Log(step, multipart.New(strings.NewReader( - "Backend engine error while running step: "+err.Error(), - ))) - } + Str("step", step.Name). + Msg("complete") // Return the error after tracing it. err = r.traceStep(processState, err, step) @@ -251,7 +242,7 @@ func (r *Runtime) exec(step *backend.Step) (*backend.State, error) { defer wg.Done() logger := r.MakeLogger() - if err := r.logger.Log(step, multipart.New(rc)); err != nil { + if err := r.logger(step, rc); err != nil { logger.Error().Err(err).Msg("process logging failed") } _ = rc.Close() diff --git a/server/pipeline/cancel.go b/server/pipeline/cancel.go index 71df7dd06..46407436e 100644 --- a/server/pipeline/cancel.go +++ b/server/pipeline/cancel.go @@ -153,9 +153,9 @@ func cancelPreviousPipelines( if err = Cancel(ctx, _store, repo, user, active); err != nil { log.Error(). Err(err). - Str("Ref", active.Ref). - Int64("ID", active.ID). - Msg("Failed to cancel pipeline") + Str("ref", active.Ref). + Int64("id", active.ID). + Msg("failed to cancel pipeline") } }