From 000fb4708670384f6c64b8430f8f6f303dbc5cbb Mon Sep 17 00:00:00 2001 From: Marlon Gamez Date: Thu, 3 Jun 2021 15:29:05 -0700 Subject: [PATCH 1/2] add event logger type --- pkg/skaffold/event/v2/logger.go | 62 ++++++++++++++++++++++++++++ pkg/skaffold/event/v2/logger_test.go | 54 ++++++++++++++++++++++++ pkg/skaffold/output/output.go | 16 +++++++ pkg/skaffold/output/output_test.go | 42 +++++++++++++++++++ 4 files changed, 174 insertions(+) create mode 100644 pkg/skaffold/event/v2/logger.go create mode 100644 pkg/skaffold/event/v2/logger_test.go diff --git a/pkg/skaffold/event/v2/logger.go b/pkg/skaffold/event/v2/logger.go new file mode 100644 index 00000000000..5913f9776ec --- /dev/null +++ b/pkg/skaffold/event/v2/logger.go @@ -0,0 +1,62 @@ +/* +Copyright 2021 The Skaffold 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 v2 + +import ( + "fmt" + "io" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" + proto "github.com/GoogleContainerTools/skaffold/proto/v2" +) + +type logger struct { + handler *eventHandler + + phase constants.Phase + subtaskID string + origin string +} + +func NewLogger(phase constants.Phase, subtaskID, origin string) io.Writer { + return logger{ + handler: handler, + phase: phase, + subtaskID: subtaskID, + origin: origin, + } +} + +func (l logger) Write(p []byte) (int, error) { + l.handler.handleSkaffoldLogEvent(&proto.SkaffoldLogEvent{ + TaskId: fmt.Sprintf("%s-%d", l.phase, l.handler.iteration), + SubtaskId: l.subtaskID, + Origin: l.origin, + Level: 0, + Message: string(p), + }) + + return len(p), nil +} + +func (ev *eventHandler) handleSkaffoldLogEvent(e *proto.SkaffoldLogEvent) { + ev.handle(&proto.Event{ + EventType: &proto.Event_SkaffoldLogEvent{ + SkaffoldLogEvent: e, + }, + }) +} \ No newline at end of file diff --git a/pkg/skaffold/event/v2/logger_test.go b/pkg/skaffold/event/v2/logger_test.go new file mode 100644 index 00000000000..60dcca07b86 --- /dev/null +++ b/pkg/skaffold/event/v2/logger_test.go @@ -0,0 +1,54 @@ +/* +Copyright 2021 The Skaffold 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 v2 + +import ( + "testing" + + latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" + "github.com/GoogleContainerTools/skaffold/proto/enums" + proto "github.com/GoogleContainerTools/skaffold/proto/v2" +) + +func TestHandleSkaffoldLogEvent(t *testing.T) { + testHandler := newHandler() + testHandler.state = emptyState(mockCfg([]latestV1.Pipeline{{}}, "test")) + + messages := []string{ + "hi!", + "how's it going", + "hope you're well", + "this is a skaffold test", + } + + // ensure that messages sent through the SkaffoldLog function are populating the event log + for _, message := range messages { + testHandler.handleSkaffoldLogEvent(&proto.SkaffoldLogEvent{ + TaskId: "Test-0", + SubtaskId: "1", + Origin: "skaffold-test", + Level: enums.LogLevel_INFO, + Message: message, + }) + } + wait(t, func() bool { + testHandler.logLock.Lock() + logLen := len(testHandler.eventLog) + testHandler.logLock.Unlock() + return logLen == len(messages) + }) +} diff --git a/pkg/skaffold/output/output.go b/pkg/skaffold/output/output.go index cc83646c84e..a96626ac173 100644 --- a/pkg/skaffold/output/output.go +++ b/pkg/skaffold/output/output.go @@ -20,6 +20,9 @@ import ( "io" "io/ioutil" "os" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" + eventV2 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event/v2" ) type skaffoldWriter struct { @@ -77,3 +80,16 @@ func GetUnderlyingWriter(out io.Writer) io.Writer { } return out } + +// WithEventContext will return a new skaffoldWriter with the given parameters to be used for the event writer. +// If the passed io.Writer is not a skaffoldWriter, then it is simply returned. +func WithEventContext(out io.Writer, phase constants.Phase, subtaskID, origin string) io.Writer { + if sw, isSW := out.(skaffoldWriter); isSW { + return skaffoldWriter{ + MainWriter: sw.MainWriter, + EventWriter: eventV2.NewLogger(phase, subtaskID, origin), + } + } + + return out +} diff --git a/pkg/skaffold/output/output_test.go b/pkg/skaffold/output/output_test.go index db171d7312a..f4d8a6b3b39 100644 --- a/pkg/skaffold/output/output_test.go +++ b/pkg/skaffold/output/output_test.go @@ -23,6 +23,8 @@ import ( "os" "testing" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" + eventV2 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event/v2" "github.com/GoogleContainerTools/skaffold/testutil" ) @@ -116,3 +118,43 @@ func TestGetUnderlyingWriter(t *testing.T) { }) } } + +func TestWithEventContext(t *testing.T) { + tests := []struct{ + name string + writer io.Writer + phase constants.Phase + subtaskID string + origin string + + expected io.Writer + }{ + { + name: "skaffoldWriter update info", + writer: skaffoldWriter{ + MainWriter: os.Stdout, + EventWriter: eventV2.NewLogger(constants.Build, "1", "skaffold-test"), + }, + phase: constants.Test, + subtaskID: "2", + origin: "skaffold-test-change", + expected: skaffoldWriter{ + MainWriter: os.Stdout, + EventWriter: eventV2.NewLogger(constants.Test, "2", "skaffold-test-change"), + }, + }, + { + name: "non skaffoldWriter returns same", + writer: os.Stdout, + expected: os.Stdout, + }, + } + + for _, test := range tests { + testutil.Run(t, test.name, func (t *testutil.T) { + got := WithEventContext(test.writer, test.phase, test.subtaskID, test.origin) + t.CheckDeepEqual(test.expected, got) + }) + } + +} From 203acac7e59c6ea5c45ff5035944490a655e65e9 Mon Sep 17 00:00:00 2001 From: Marlon Gamez Date: Fri, 4 Jun 2021 11:10:27 -0700 Subject: [PATCH 2/2] fix import cycle and tests --- pkg/skaffold/event/v2/config.go | 29 +++++++++++++++++++++++++++++ pkg/skaffold/event/v2/event.go | 9 ++++----- pkg/skaffold/event/v2/logger.go | 25 +++++++++++-------------- pkg/skaffold/output/output_test.go | 27 +++++++++++++-------------- 4 files changed, 57 insertions(+), 33 deletions(-) create mode 100644 pkg/skaffold/event/v2/config.go diff --git a/pkg/skaffold/event/v2/config.go b/pkg/skaffold/event/v2/config.go new file mode 100644 index 00000000000..32dd60b196e --- /dev/null +++ b/pkg/skaffold/event/v2/config.go @@ -0,0 +1,29 @@ +/* +Copyright 2021 The Skaffold 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 v2 + +import ( + latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" +) + +type Config interface { + GetKubeContext() string + AutoBuild() bool + AutoDeploy() bool + AutoSync() bool + GetPipelines() []latestV1.Pipeline +} diff --git a/pkg/skaffold/event/v2/event.go b/pkg/skaffold/event/v2/event.go index b5ce403d834..b06e086a007 100644 --- a/pkg/skaffold/event/v2/event.go +++ b/pkg/skaffold/event/v2/event.go @@ -30,7 +30,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event" proto "github.com/GoogleContainerTools/skaffold/proto/v2" ) @@ -69,7 +68,7 @@ type eventHandler struct { logLock sync.Mutex applicationLogs []proto.Event applicationLogsLock sync.Mutex - cfg event.Config + cfg Config iteration int state proto.State @@ -179,7 +178,7 @@ func (ev *eventHandler) forEachApplicationLog(callback func(*proto.Event) error) return ev.forEach(&ev.applicationLogListeners, &ev.applicationLogs, &ev.applicationLogsLock, callback) } -func emptyState(cfg event.Config) proto.State { +func emptyState(cfg Config) proto.State { builds := map[string]string{} for _, p := range cfg.GetPipelines() { for _, a := range p.Build.Artifacts { @@ -272,7 +271,7 @@ func emptyStatusCheckState() *proto.StatusCheckState { } // InitializeState instantiates the global state of the skaffold runner, as well as the event log. -func InitializeState(cfg event.Config) { +func InitializeState(cfg Config) { handler.cfg = cfg handler.setState(emptyState(cfg)) } @@ -286,7 +285,7 @@ func AutoTriggerDiff(phase constants.Phase, val bool) (bool, error) { case constants.Deploy: return val != handler.getState().DeployState.AutoTrigger, nil default: - return false, fmt.Errorf("unknown phase %v not found in handler state", phase) + return false, fmt.Errorf("unknown Phase %v not found in handler state", phase) } } diff --git a/pkg/skaffold/event/v2/logger.go b/pkg/skaffold/event/v2/logger.go index 5913f9776ec..1b0e1ceba52 100644 --- a/pkg/skaffold/event/v2/logger.go +++ b/pkg/skaffold/event/v2/logger.go @@ -25,27 +25,24 @@ import ( ) type logger struct { - handler *eventHandler - - phase constants.Phase - subtaskID string - origin string + Phase constants.Phase + SubtaskID string + Origin string } func NewLogger(phase constants.Phase, subtaskID, origin string) io.Writer { return logger{ - handler: handler, - phase: phase, - subtaskID: subtaskID, - origin: origin, + Phase: phase, + SubtaskID: subtaskID, + Origin: origin, } } func (l logger) Write(p []byte) (int, error) { - l.handler.handleSkaffoldLogEvent(&proto.SkaffoldLogEvent{ - TaskId: fmt.Sprintf("%s-%d", l.phase, l.handler.iteration), - SubtaskId: l.subtaskID, - Origin: l.origin, + handler.handleSkaffoldLogEvent(&proto.SkaffoldLogEvent{ + TaskId: fmt.Sprintf("%s-%d", l.Phase, handler.iteration), + SubtaskId: l.SubtaskID, + Origin: l.Origin, Level: 0, Message: string(p), }) @@ -59,4 +56,4 @@ func (ev *eventHandler) handleSkaffoldLogEvent(e *proto.SkaffoldLogEvent) { SkaffoldLogEvent: e, }, }) -} \ No newline at end of file +} diff --git a/pkg/skaffold/output/output_test.go b/pkg/skaffold/output/output_test.go index f4d8a6b3b39..eb7d50a5faf 100644 --- a/pkg/skaffold/output/output_test.go +++ b/pkg/skaffold/output/output_test.go @@ -120,41 +120,40 @@ func TestGetUnderlyingWriter(t *testing.T) { } func TestWithEventContext(t *testing.T) { - tests := []struct{ - name string - writer io.Writer - phase constants.Phase + tests := []struct { + name string + writer io.Writer + phase constants.Phase subtaskID string - origin string + origin string expected io.Writer }{ { name: "skaffoldWriter update info", writer: skaffoldWriter{ - MainWriter: os.Stdout, + MainWriter: ioutil.Discard, EventWriter: eventV2.NewLogger(constants.Build, "1", "skaffold-test"), }, - phase: constants.Test, + phase: constants.Test, subtaskID: "2", - origin: "skaffold-test-change", + origin: "skaffold-test-change", expected: skaffoldWriter{ - MainWriter: os.Stdout, + MainWriter: ioutil.Discard, EventWriter: eventV2.NewLogger(constants.Test, "2", "skaffold-test-change"), }, }, { - name: "non skaffoldWriter returns same", - writer: os.Stdout, - expected: os.Stdout, + name: "non skaffoldWriter returns same", + writer: ioutil.Discard, + expected: ioutil.Discard, }, } for _, test := range tests { - testutil.Run(t, test.name, func (t *testutil.T) { + testutil.Run(t, test.name, func(t *testutil.T) { got := WithEventContext(test.writer, test.phase, test.subtaskID, test.origin) t.CheckDeepEqual(test.expected, got) }) } - }