From ea3d8157f498ea961e729fc0a1cf1b2a4bc1146f Mon Sep 17 00:00:00 2001 From: Jeev B Date: Wed, 12 Jul 2023 21:28:00 -0700 Subject: [PATCH 01/22] Refactor task log templates to support extra vars --- go/tasks/logs/logging_utils.go | 4 +- go/tasks/pluginmachinery/tasklog/plugin.go | 8 +- go/tasks/pluginmachinery/tasklog/template.go | 167 ++++++++---------- .../pluginmachinery/tasklog/template_test.go | 31 ++-- 4 files changed, 100 insertions(+), 110 deletions(-) diff --git a/go/tasks/logs/logging_utils.go b/go/tasks/logs/logging_utils.go index 38e895d78..64073256b 100755 --- a/go/tasks/logs/logging_utils.go +++ b/go/tasks/logs/logging_utils.go @@ -62,12 +62,12 @@ type taskLogPluginWrapper struct { logPlugins []logPlugin } -func (t taskLogPluginWrapper) GetTaskLogs(input tasklog.Input) (logOutput tasklog.Output, err error) { +func (t taskLogPluginWrapper) GetTaskLogs(input tasklog.Input, p ...tasklog.TemplateVarsProvider) (logOutput tasklog.Output, err error) { logs := make([]*core.TaskLog, 0, len(t.logPlugins)) suffix := input.LogName for _, plugin := range t.logPlugins { input.LogName = plugin.Name + suffix - o, err := plugin.Plugin.GetTaskLogs(input) + o, err := plugin.Plugin.GetTaskLogs(input, p...) if err != nil { return tasklog.Output{}, err } diff --git a/go/tasks/pluginmachinery/tasklog/plugin.go b/go/tasks/pluginmachinery/tasklog/plugin.go index f96c9cf8a..160fb3860 100644 --- a/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/go/tasks/pluginmachinery/tasklog/plugin.go @@ -2,6 +2,12 @@ package tasklog import "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" +type TemplateVars map[string]interface{} + +type TemplateVarsProvider interface { + ToTemplateVars() (TemplateVars, error) +} + // Input contains all available information about task's execution that a log plugin can use to construct task's // log links. type Input struct { @@ -24,5 +30,5 @@ type Output struct { // Plugin represents an interface for task log plugins to implement to plug generated task log links into task events. type Plugin interface { // Generates a TaskLog object given necessary computation information - GetTaskLogs(input Input) (logs Output, err error) + GetTaskLogs(input Input, p ...TemplateVarsProvider) (logs Output, err error) } diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index 4bb52aef4..14e06d075 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -1,14 +1,57 @@ package tasklog import ( - "fmt" - "regexp" + "bytes" "strconv" "strings" + "text/template" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" ) +func (t TemplateVars) Merge(others ...TemplateVars) { + for _, other := range others { + for k, v := range other { + t[k] = v + } + } +} + +func (t TemplateVars) MergeProviders(providers ...TemplateVarsProvider) error { + var others []TemplateVars + for _, p := range providers { + pTemplateVars, err := p.ToTemplateVars() + if err != nil { + return err + } + others = append(others, pTemplateVars) + } + t.Merge(others...) + return nil +} + +func (input Input) ToTemplateVars() (TemplateVars, error) { + // Container IDs are prefixed with docker://, cri-o://, etc. which is stripped by fluentd before pushing to a log + // stream. Therefore, we must also strip the prefix. + containerID := input.ContainerID + stripDelimiter := "://" + if split := strings.Split(input.ContainerID, stripDelimiter); len(split) > 1 { + containerID = split[1] + } + + return TemplateVars{ + "podName": input.PodName, + "podUID": input.PodUID, + "namespace": input.Namespace, + "containerName": input.ContainerName, + "containerId": containerID, + "logName": input.LogName, + "hostname": input.HostName, + "podUnixStartTime": strconv.FormatInt(input.PodUnixStartTime, 10), + "podUnixFinishTime": strconv.FormatInt(input.PodUnixFinishTime, 10), + }, nil +} + // A simple log plugin that supports templates in urls to build the final log link. Supported templates are: // {{ .podName }}: Gets the pod name as it shows in k8s dashboard, // {{ .podUID }}: Gets the pod UID, @@ -24,52 +67,10 @@ type TemplateLogPlugin struct { messageFormat core.TaskLog_MessageFormat } -type regexValPair struct { - regex *regexp.Regexp - val string -} - -type templateRegexes struct { - PodName *regexp.Regexp - PodUID *regexp.Regexp - Namespace *regexp.Regexp - ContainerName *regexp.Regexp - ContainerID *regexp.Regexp - LogName *regexp.Regexp - Hostname *regexp.Regexp - PodUnixStartTime *regexp.Regexp - PodUnixFinishTime *regexp.Regexp -} - -func mustInitTemplateRegexes() templateRegexes { - return templateRegexes{ - PodName: mustCreateRegex("podName"), - PodUID: mustCreateRegex("podUID"), - Namespace: mustCreateRegex("namespace"), - ContainerName: mustCreateRegex("containerName"), - ContainerID: mustCreateRegex("containerID"), - LogName: mustCreateRegex("logName"), - Hostname: mustCreateRegex("hostname"), - PodUnixStartTime: mustCreateRegex("podUnixStartTime"), - PodUnixFinishTime: mustCreateRegex("podUnixFinishTime"), - } -} - -var regexes = mustInitTemplateRegexes() - -func mustCreateRegex(varName string) *regexp.Regexp { - return regexp.MustCompile(fmt.Sprintf(`(?i){{\s*[\.$]%s\s*}}`, varName)) -} - -func replaceAll(template string, values []regexValPair) string { - for _, v := range values { - template = v.regex.ReplaceAllString(template, v.val) - } - - return template -} - -func (s TemplateLogPlugin) GetTaskLog(podName, podUID, namespace, containerName, containerID, logName string, podUnixStartTime, podUnixFinishTime int64) (core.TaskLog, error) { +func (s TemplateLogPlugin) GetTaskLog( + podName, podUID, namespace, containerName, containerID, logName string, + podUnixStartTime, podUnixFinishTime int64, +) (core.TaskLog, error) { o, err := s.GetTaskLogs(Input{ LogName: logName, Namespace: namespace, @@ -88,59 +89,28 @@ func (s TemplateLogPlugin) GetTaskLog(podName, podUID, namespace, containerName, return *o.TaskLogs[0], nil } -func (s TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { - // Container IDs are prefixed with docker://, cri-o://, etc. which is stripped by fluentd before pushing to a log - // stream. Therefore, we must also strip the prefix. - containerID := input.ContainerID - stripDelimiter := "://" - if split := strings.Split(input.ContainerID, stripDelimiter); len(split) > 1 { - containerID = split[1] +func (s TemplateLogPlugin) GetTaskLogs(input Input, p ...TemplateVarsProvider) (Output, error) { + var err error + templateVars, err := input.ToTemplateVars() + if err != nil { + return Output{}, err + } + err = templateVars.MergeProviders(p...) + if err != nil { + return Output{}, err } taskLogs := make([]*core.TaskLog, 0, len(s.templateUris)) for _, templateURI := range s.templateUris { + var buf bytes.Buffer + t := template.Must(template.New("uri").Parse(templateURI)) + err := t.Execute(&buf, templateVars) + if err != nil { + return Output{}, err + } + taskLogs = append(taskLogs, &core.TaskLog{ - Uri: replaceAll( - templateURI, - []regexValPair{ - { - regex: regexes.PodName, - val: input.PodName, - }, - { - regex: regexes.PodUID, - val: input.PodUID, - }, - { - regex: regexes.Namespace, - val: input.Namespace, - }, - { - regex: regexes.ContainerName, - val: input.ContainerName, - }, - { - regex: regexes.ContainerID, - val: containerID, - }, - { - regex: regexes.LogName, - val: input.LogName, - }, - { - regex: regexes.Hostname, - val: input.HostName, - }, - { - regex: regexes.PodUnixStartTime, - val: strconv.FormatInt(input.PodUnixStartTime, 10), - }, - { - regex: regexes.PodUnixFinishTime, - val: strconv.FormatInt(input.PodUnixFinishTime, 10), - }, - }, - ), + Uri: buf.String(), Name: input.LogName, MessageFormat: s.messageFormat, }) @@ -162,7 +132,10 @@ func (s TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { // {{ .hostname }}: The hostname where the pod is running and where logs reside. // {{ .podUnixStartTime }}: The pod creation time (in unix seconds, not millis) // {{ .podUnixFinishTime }}: Don't have a good mechanism for this yet, but approximating with time.Now for now -func NewTemplateLogPlugin(templateUris []string, messageFormat core.TaskLog_MessageFormat) TemplateLogPlugin { +func NewTemplateLogPlugin( + templateUris []string, + messageFormat core.TaskLog_MessageFormat, +) TemplateLogPlugin { return TemplateLogPlugin{ templateUris: templateUris, messageFormat: messageFormat, diff --git a/go/tasks/pluginmachinery/tasklog/template_test.go b/go/tasks/pluginmachinery/tasklog/template_test.go index b74d4efa7..58221dd3d 100644 --- a/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/go/tasks/pluginmachinery/tasklog/template_test.go @@ -11,7 +11,12 @@ import ( ) func TestTemplateLog(t *testing.T) { - p := NewTemplateLogPlugin([]string{"https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.{{.podName}}_{{.podUID}}_{{.namespace}}_{{.containerName}}-{{.containerId}}.log"}, core.TaskLog_JSON) + p := NewTemplateLogPlugin( + []string{ + "https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.{{.podName}}_{{.podUID}}_{{.namespace}}_{{.containerName}}-{{.containerId}}.log", + }, + core.TaskLog_JSON, + ) tl, err := p.GetTaskLog( "f-uuid-driver", "pod-uid", @@ -25,14 +30,11 @@ func TestTemplateLog(t *testing.T) { assert.NoError(t, err) assert.Equal(t, tl.GetName(), "main_logs") assert.Equal(t, tl.GetMessageFormat(), core.TaskLog_JSON) - assert.Equal(t, "https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.f-uuid-driver_pod-uid_flyteexamples-production_spark-kubernetes-driver-abc.log", tl.Uri) -} - -// Latest Run: Benchmark_mustInitTemplateRegexes-16 45960 26914 ns/op -func Benchmark_mustInitTemplateRegexes(b *testing.B) { - for i := 0; i < b.N; i++ { - mustInitTemplateRegexes() - } + assert.Equal( + t, + "https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.f-uuid-driver_pod-uid_flyteexamples-production_spark-kubernetes-driver-abc.log", + tl.Uri, + ) } func Test_templateLogPlugin_Regression(t *testing.T) { @@ -134,7 +136,16 @@ func Test_templateLogPlugin_Regression(t *testing.T) { messageFormat: tt.fields.messageFormat, } - got, err := s.GetTaskLog(tt.args.podName, tt.args.podUID, tt.args.namespace, tt.args.containerName, tt.args.containerID, tt.args.logName, tt.args.podUnixStartTime, tt.args.podUnixFinishTime) + got, err := s.GetTaskLog( + tt.args.podName, + tt.args.podUID, + tt.args.namespace, + tt.args.containerName, + tt.args.containerID, + tt.args.logName, + tt.args.podUnixStartTime, + tt.args.podUnixFinishTime, + ) if (err != nil) != tt.wantErr { t.Errorf("GetTaskLog() error = %v, wantErr %v", err, tt.wantErr) return From 4e150b23e7f465652fc2946f27da2a69a2551e88 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Wed, 12 Jul 2023 23:07:15 -0700 Subject: [PATCH 02/22] plumbing for k8s --- go/tasks/logs/logging_utils.go | 6 +++--- go/tasks/pluginmachinery/tasklog/template.go | 18 ++++++++++++++++++ go/tasks/plugins/array/k8s/subtask.go | 2 +- .../plugins/array/k8s/subtask_exec_context.go | 15 +++++++++++++++ go/tasks/plugins/k8s/pod/plugin.go | 6 +++--- 5 files changed, 40 insertions(+), 7 deletions(-) diff --git a/go/tasks/logs/logging_utils.go b/go/tasks/logs/logging_utils.go index 64073256b..57d77dc13 100755 --- a/go/tasks/logs/logging_utils.go +++ b/go/tasks/logs/logging_utils.go @@ -5,9 +5,8 @@ import ( "fmt" "time" - "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/tasklog" - "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" + "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/tasklog" "github.com/flyteorg/flytestdlib/logger" v1 "k8s.io/api/core/v1" ) @@ -18,7 +17,7 @@ type logPlugin struct { } // Internal -func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, pod *v1.Pod, index uint32, nameSuffix string) ([]*core.TaskLog, error) { +func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVarProviders ...tasklog.TemplateVarsProvider) ([]*core.TaskLog, error) { if logPlugin == nil { return nil, nil } @@ -49,6 +48,7 @@ func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, pod PodUnixStartTime: pod.CreationTimestamp.Unix(), PodUnixFinishTime: time.Now().Unix(), }, + extraLogTemplateVarProviders..., ) if err != nil { diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index 14e06d075..e1c854b56 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -2,6 +2,7 @@ package tasklog import ( "bytes" + "encoding/json" "strconv" "strings" "text/template" @@ -52,6 +53,23 @@ func (input Input) ToTemplateVars() (TemplateVars, error) { }, nil } +type TaskExecutionIdentifierTemplateVarsProvider struct { + core.TaskExecutionIdentifier +} + +func (p TaskExecutionIdentifierTemplateVarsProvider) ToTemplateVars() (TemplateVars, error) { + var templateVars TemplateVars + var err error + + serialized, err := json.Marshal(p.TaskExecutionIdentifier) + if err != nil { + return templateVars, err + } + + err = json.Unmarshal(serialized, &templateVars) + return templateVars, err +} + // A simple log plugin that supports templates in urls to build the final log link. Supported templates are: // {{ .podName }}: Gets the pod name as it shows in k8s dashboard, // {{ .podUID }}: Gets the pod UID, diff --git a/go/tasks/plugins/array/k8s/subtask.go b/go/tasks/plugins/array/k8s/subtask.go index 6411f06c0..3653a6556 100644 --- a/go/tasks/plugins/array/k8s/subtask.go +++ b/go/tasks/plugins/array/k8s/subtask.go @@ -304,7 +304,7 @@ func getSubtaskPhaseInfo(ctx context.Context, stCtx SubTaskExecutionContext, cfg } stID, _ := stCtx.TaskExecutionMetadata().GetTaskExecutionID().(SubTaskExecutionID) - phaseInfo, err := podPlugin.DefaultPodPlugin.GetTaskPhaseWithLogs(ctx, stCtx, pod, logPlugin, stID.GetLogSuffix()) + phaseInfo, err := podPlugin.DefaultPodPlugin.GetTaskPhaseWithLogs(ctx, stCtx, pod, logPlugin, stID.GetLogSuffix(), stID) if err != nil { logger.Warnf(ctx, "failed to check status of resource in plugin [%s], with error: %s", executorName, err.Error()) return pluginsCore.PhaseInfoUndefined, err diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context.go b/go/tasks/plugins/array/k8s/subtask_exec_context.go index 1f18b1009..217f1902e 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context.go @@ -12,6 +12,7 @@ import ( "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/ioutils" "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/utils" "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/utils/secrets" + "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/tasklog" "github.com/flyteorg/flyteplugins/go/tasks/plugins/array" podPlugin "github.com/flyteorg/flyteplugins/go/tasks/plugins/k8s/pod" @@ -175,6 +176,20 @@ func (s SubTaskExecutionID) GetLogSuffix() string { return fmt.Sprintf(" #%d-%d-%d", s.taskRetryAttempt, s.executionIndex, s.subtaskRetryAttempt) } +func (s SubTaskExecutionID) ToTemplateVars() (tasklog.TemplateVars, error) { + v, err := tasklog.TaskExecutionIdentifierTemplateVarsProvider{s.GetID()}.ToTemplateVars() + if err != nil { + return v, err + } + v.Merge(tasklog.TemplateVars{ + "executionIndex": s.executionIndex, + "parentName": s.parentName, + "subtaskRetryAttempt": s.subtaskRetryAttempt, + "taskRetryAttempt": s.taskRetryAttempt, + }) + return v, nil +} + // NewSubtaskExecutionID constructs a SubTaskExecutionID using the provided parameters func NewSubTaskExecutionID(taskExecutionID pluginsCore.TaskExecutionID, executionIndex int, retryAttempt uint64) SubTaskExecutionID { return SubTaskExecutionID{ diff --git a/go/tasks/plugins/k8s/pod/plugin.go b/go/tasks/plugins/k8s/pod/plugin.go index ac0d8c24c..111afa6d5 100644 --- a/go/tasks/plugins/k8s/pod/plugin.go +++ b/go/tasks/plugins/k8s/pod/plugin.go @@ -143,10 +143,10 @@ func (p plugin) GetTaskPhase(ctx context.Context, pluginContext k8s.PluginContex return pluginsCore.PhaseInfoUndefined, err } - return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)") + return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)", tasklog.TaskExecutionIdentifierTemplateVarsProvider{pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()}) } -func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string) (pluginsCore.PhaseInfo, error) { +func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string, extraLogTemplateVarProviders ...tasklog.TemplateVarsProvider) (pluginsCore.PhaseInfo, error) { pluginState := k8s.PluginState{} _, err := pluginContext.PluginStateReader().Get(&pluginState) if err != nil { @@ -167,7 +167,7 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin } if pod.Status.Phase != v1.PodPending && pod.Status.Phase != v1.PodUnknown { - taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, pod, 0, logSuffix) + taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, pod, 0, logSuffix, extraLogTemplateVarProviders...) if err != nil { return pluginsCore.PhaseInfoUndefined, err } From d85df0528270cb9e61ce71d350593c9cb5d35e0e Mon Sep 17 00:00:00 2001 From: Jeev B Date: Wed, 12 Jul 2023 23:32:45 -0700 Subject: [PATCH 03/22] Cleanup use of providers --- go/tasks/logs/logging_utils.go | 12 ++++++++---- go/tasks/pluginmachinery/tasklog/plugin.go | 2 +- go/tasks/pluginmachinery/tasklog/template.go | 7 ++----- go/tasks/plugins/array/k8s/subtask.go | 7 ++++++- go/tasks/plugins/k8s/pod/plugin.go | 11 ++++++++--- 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/go/tasks/logs/logging_utils.go b/go/tasks/logs/logging_utils.go index 57d77dc13..49f3fc842 100755 --- a/go/tasks/logs/logging_utils.go +++ b/go/tasks/logs/logging_utils.go @@ -17,7 +17,7 @@ type logPlugin struct { } // Internal -func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVarProviders ...tasklog.TemplateVarsProvider) ([]*core.TaskLog, error) { +func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) ([]*core.TaskLog, error) { if logPlugin == nil { return nil, nil } @@ -48,7 +48,7 @@ func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, pod PodUnixStartTime: pod.CreationTimestamp.Unix(), PodUnixFinishTime: time.Now().Unix(), }, - extraLogTemplateVarProviders..., + extraLogTemplateVars..., ) if err != nil { @@ -62,12 +62,16 @@ type taskLogPluginWrapper struct { logPlugins []logPlugin } -func (t taskLogPluginWrapper) GetTaskLogs(input tasklog.Input, p ...tasklog.TemplateVarsProvider) (logOutput tasklog.Output, err error) { +func (t taskLogPluginWrapper) GetTaskLogs(input tasklog.Input, extraTemplateVars ...tasklog.TemplateVars) (logOutput tasklog.Output, err error) { logs := make([]*core.TaskLog, 0, len(t.logPlugins)) suffix := input.LogName + + mergedTemplateVars := make(tasklog.TemplateVars) + mergedTemplateVars.Merge(extraTemplateVars...) + for _, plugin := range t.logPlugins { input.LogName = plugin.Name + suffix - o, err := plugin.Plugin.GetTaskLogs(input, p...) + o, err := plugin.Plugin.GetTaskLogs(input, mergedTemplateVars) if err != nil { return tasklog.Output{}, err } diff --git a/go/tasks/pluginmachinery/tasklog/plugin.go b/go/tasks/pluginmachinery/tasklog/plugin.go index 160fb3860..c734968ed 100644 --- a/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/go/tasks/pluginmachinery/tasklog/plugin.go @@ -30,5 +30,5 @@ type Output struct { // Plugin represents an interface for task log plugins to implement to plug generated task log links into task events. type Plugin interface { // Generates a TaskLog object given necessary computation information - GetTaskLogs(input Input, p ...TemplateVarsProvider) (logs Output, err error) + GetTaskLogs(i Input, v ...TemplateVars) (logs Output, err error) } diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index e1c854b56..e65204a6d 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -107,16 +107,13 @@ func (s TemplateLogPlugin) GetTaskLog( return *o.TaskLogs[0], nil } -func (s TemplateLogPlugin) GetTaskLogs(input Input, p ...TemplateVarsProvider) (Output, error) { +func (s TemplateLogPlugin) GetTaskLogs(input Input, extraTemplateVars ...TemplateVars) (Output, error) { var err error templateVars, err := input.ToTemplateVars() if err != nil { return Output{}, err } - err = templateVars.MergeProviders(p...) - if err != nil { - return Output{}, err - } + templateVars.Merge(extraTemplateVars...) taskLogs := make([]*core.TaskLog, 0, len(s.templateUris)) for _, templateURI := range s.templateUris { diff --git a/go/tasks/plugins/array/k8s/subtask.go b/go/tasks/plugins/array/k8s/subtask.go index 3653a6556..3b5f1e991 100644 --- a/go/tasks/plugins/array/k8s/subtask.go +++ b/go/tasks/plugins/array/k8s/subtask.go @@ -304,7 +304,12 @@ func getSubtaskPhaseInfo(ctx context.Context, stCtx SubTaskExecutionContext, cfg } stID, _ := stCtx.TaskExecutionMetadata().GetTaskExecutionID().(SubTaskExecutionID) - phaseInfo, err := podPlugin.DefaultPodPlugin.GetTaskPhaseWithLogs(ctx, stCtx, pod, logPlugin, stID.GetLogSuffix(), stID) + extraLogTemplateVars, err := stID.ToTemplateVars() + if err != nil { + logger.Warnf(ctx, "failed to check status of resource in plugin [%s], with error: %s", executorName, err.Error()) + return pluginsCore.PhaseInfoUndefined, err + } + phaseInfo, err := podPlugin.DefaultPodPlugin.GetTaskPhaseWithLogs(ctx, stCtx, pod, logPlugin, stID.GetLogSuffix(), extraLogTemplateVars) if err != nil { logger.Warnf(ctx, "failed to check status of resource in plugin [%s], with error: %s", executorName, err.Error()) return pluginsCore.PhaseInfoUndefined, err diff --git a/go/tasks/plugins/k8s/pod/plugin.go b/go/tasks/plugins/k8s/pod/plugin.go index 111afa6d5..ebc331b99 100644 --- a/go/tasks/plugins/k8s/pod/plugin.go +++ b/go/tasks/plugins/k8s/pod/plugin.go @@ -143,10 +143,15 @@ func (p plugin) GetTaskPhase(ctx context.Context, pluginContext k8s.PluginContex return pluginsCore.PhaseInfoUndefined, err } - return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)", tasklog.TaskExecutionIdentifierTemplateVarsProvider{pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()}) + extraLogTemplateVars, err := tasklog.TaskExecutionIdentifierTemplateVarsProvider{pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()}.ToTemplateVars() + if err != nil { + return pluginsCore.PhaseInfoUndefined, err + } + + return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)", extraLogTemplateVars) } -func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string, extraLogTemplateVarProviders ...tasklog.TemplateVarsProvider) (pluginsCore.PhaseInfo, error) { +func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) (pluginsCore.PhaseInfo, error) { pluginState := k8s.PluginState{} _, err := pluginContext.PluginStateReader().Get(&pluginState) if err != nil { @@ -167,7 +172,7 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin } if pod.Status.Phase != v1.PodPending && pod.Status.Phase != v1.PodUnknown { - taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, pod, 0, logSuffix, extraLogTemplateVarProviders...) + taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, pod, 0, logSuffix, extraLogTemplateVars...) if err != nil { return pluginsCore.PhaseInfoUndefined, err } From 8f78033572e025ca40649b785719855ce93414c9 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Wed, 12 Jul 2023 23:40:08 -0700 Subject: [PATCH 04/22] cleanup --- go/tasks/pluginmachinery/tasklog/plugin.go | 2 +- go/tasks/pluginmachinery/tasklog/template.go | 33 +++++-------------- go/tasks/plugins/array/k8s/subtask.go | 7 +--- .../plugins/array/k8s/subtask_exec_context.go | 17 ++++------ go/tasks/plugins/k8s/pod/plugin.go | 6 +--- 5 files changed, 19 insertions(+), 46 deletions(-) diff --git a/go/tasks/pluginmachinery/tasklog/plugin.go b/go/tasks/pluginmachinery/tasklog/plugin.go index c734968ed..65d61a906 100644 --- a/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/go/tasks/pluginmachinery/tasklog/plugin.go @@ -5,7 +5,7 @@ import "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" type TemplateVars map[string]interface{} type TemplateVarsProvider interface { - ToTemplateVars() (TemplateVars, error) + ToTemplateVars() TemplateVars } // Input contains all available information about task's execution that a log plugin can use to construct task's diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index e65204a6d..acfee9702 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -18,20 +18,15 @@ func (t TemplateVars) Merge(others ...TemplateVars) { } } -func (t TemplateVars) MergeProviders(providers ...TemplateVarsProvider) error { +func (t TemplateVars) MergeProviders(providers ...TemplateVarsProvider) { var others []TemplateVars for _, p := range providers { - pTemplateVars, err := p.ToTemplateVars() - if err != nil { - return err - } - others = append(others, pTemplateVars) + others = append(others, p.ToTemplateVars()) } t.Merge(others...) - return nil } -func (input Input) ToTemplateVars() (TemplateVars, error) { +func (input Input) ToTemplateVars() TemplateVars { // Container IDs are prefixed with docker://, cri-o://, etc. which is stripped by fluentd before pushing to a log // stream. Therefore, we must also strip the prefix. containerID := input.ContainerID @@ -50,24 +45,18 @@ func (input Input) ToTemplateVars() (TemplateVars, error) { "hostname": input.HostName, "podUnixStartTime": strconv.FormatInt(input.PodUnixStartTime, 10), "podUnixFinishTime": strconv.FormatInt(input.PodUnixFinishTime, 10), - }, nil + } } type TaskExecutionIdentifierTemplateVarsProvider struct { core.TaskExecutionIdentifier } -func (p TaskExecutionIdentifierTemplateVarsProvider) ToTemplateVars() (TemplateVars, error) { +func (p TaskExecutionIdentifierTemplateVarsProvider) ToTemplateVars() TemplateVars { var templateVars TemplateVars - var err error - - serialized, err := json.Marshal(p.TaskExecutionIdentifier) - if err != nil { - return templateVars, err - } - - err = json.Unmarshal(serialized, &templateVars) - return templateVars, err + serialized, _ := json.Marshal(p.TaskExecutionIdentifier) + _ = json.Unmarshal(serialized, &templateVars) + return templateVars } // A simple log plugin that supports templates in urls to build the final log link. Supported templates are: @@ -108,11 +97,7 @@ func (s TemplateLogPlugin) GetTaskLog( } func (s TemplateLogPlugin) GetTaskLogs(input Input, extraTemplateVars ...TemplateVars) (Output, error) { - var err error - templateVars, err := input.ToTemplateVars() - if err != nil { - return Output{}, err - } + templateVars := input.ToTemplateVars() templateVars.Merge(extraTemplateVars...) taskLogs := make([]*core.TaskLog, 0, len(s.templateUris)) diff --git a/go/tasks/plugins/array/k8s/subtask.go b/go/tasks/plugins/array/k8s/subtask.go index 3b5f1e991..f24713fac 100644 --- a/go/tasks/plugins/array/k8s/subtask.go +++ b/go/tasks/plugins/array/k8s/subtask.go @@ -304,12 +304,7 @@ func getSubtaskPhaseInfo(ctx context.Context, stCtx SubTaskExecutionContext, cfg } stID, _ := stCtx.TaskExecutionMetadata().GetTaskExecutionID().(SubTaskExecutionID) - extraLogTemplateVars, err := stID.ToTemplateVars() - if err != nil { - logger.Warnf(ctx, "failed to check status of resource in plugin [%s], with error: %s", executorName, err.Error()) - return pluginsCore.PhaseInfoUndefined, err - } - phaseInfo, err := podPlugin.DefaultPodPlugin.GetTaskPhaseWithLogs(ctx, stCtx, pod, logPlugin, stID.GetLogSuffix(), extraLogTemplateVars) + phaseInfo, err := podPlugin.DefaultPodPlugin.GetTaskPhaseWithLogs(ctx, stCtx, pod, logPlugin, stID.GetLogSuffix(), stID.ToTemplateVars()) if err != nil { logger.Warnf(ctx, "failed to check status of resource in plugin [%s], with error: %s", executorName, err.Error()) return pluginsCore.PhaseInfoUndefined, err diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context.go b/go/tasks/plugins/array/k8s/subtask_exec_context.go index 217f1902e..ad0b3edad 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context.go @@ -10,9 +10,9 @@ import ( pluginsCore "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/core" "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/io" "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/ioutils" + "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/tasklog" "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/utils" "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/utils/secrets" - "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/tasklog" "github.com/flyteorg/flyteplugins/go/tasks/plugins/array" podPlugin "github.com/flyteorg/flyteplugins/go/tasks/plugins/k8s/pod" @@ -176,18 +176,15 @@ func (s SubTaskExecutionID) GetLogSuffix() string { return fmt.Sprintf(" #%d-%d-%d", s.taskRetryAttempt, s.executionIndex, s.subtaskRetryAttempt) } -func (s SubTaskExecutionID) ToTemplateVars() (tasklog.TemplateVars, error) { - v, err := tasklog.TaskExecutionIdentifierTemplateVarsProvider{s.GetID()}.ToTemplateVars() - if err != nil { - return v, err - } +func (s SubTaskExecutionID) ToTemplateVars() tasklog.TemplateVars { + v := tasklog.TaskExecutionIdentifierTemplateVarsProvider{s.GetID()}.ToTemplateVars() v.Merge(tasklog.TemplateVars{ - "executionIndex": s.executionIndex, - "parentName": s.parentName, + "executionIndex": s.executionIndex, + "parentName": s.parentName, "subtaskRetryAttempt": s.subtaskRetryAttempt, - "taskRetryAttempt": s.taskRetryAttempt, + "taskRetryAttempt": s.taskRetryAttempt, }) - return v, nil + return v } // NewSubtaskExecutionID constructs a SubTaskExecutionID using the provided parameters diff --git a/go/tasks/plugins/k8s/pod/plugin.go b/go/tasks/plugins/k8s/pod/plugin.go index ebc331b99..f4349cac4 100644 --- a/go/tasks/plugins/k8s/pod/plugin.go +++ b/go/tasks/plugins/k8s/pod/plugin.go @@ -143,11 +143,7 @@ func (p plugin) GetTaskPhase(ctx context.Context, pluginContext k8s.PluginContex return pluginsCore.PhaseInfoUndefined, err } - extraLogTemplateVars, err := tasklog.TaskExecutionIdentifierTemplateVarsProvider{pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()}.ToTemplateVars() - if err != nil { - return pluginsCore.PhaseInfoUndefined, err - } - + extraLogTemplateVars := tasklog.TaskExecutionIdentifierTemplateVarsProvider{pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()}.ToTemplateVars() return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)", extraLogTemplateVars) } From 9624d87a72741f06dab1e45341c5a8b1709ac476 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Wed, 12 Jul 2023 23:44:03 -0700 Subject: [PATCH 05/22] more cleanups --- go/tasks/pluginmachinery/tasklog/plugin.go | 4 ---- go/tasks/pluginmachinery/tasklog/template.go | 8 -------- 2 files changed, 12 deletions(-) diff --git a/go/tasks/pluginmachinery/tasklog/plugin.go b/go/tasks/pluginmachinery/tasklog/plugin.go index 65d61a906..c7036aec5 100644 --- a/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/go/tasks/pluginmachinery/tasklog/plugin.go @@ -4,10 +4,6 @@ import "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" type TemplateVars map[string]interface{} -type TemplateVarsProvider interface { - ToTemplateVars() TemplateVars -} - // Input contains all available information about task's execution that a log plugin can use to construct task's // log links. type Input struct { diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index acfee9702..4304bb057 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -18,14 +18,6 @@ func (t TemplateVars) Merge(others ...TemplateVars) { } } -func (t TemplateVars) MergeProviders(providers ...TemplateVarsProvider) { - var others []TemplateVars - for _, p := range providers { - others = append(others, p.ToTemplateVars()) - } - t.Merge(others...) -} - func (input Input) ToTemplateVars() TemplateVars { // Container IDs are prefixed with docker://, cri-o://, etc. which is stripped by fluentd before pushing to a log // stream. Therefore, we must also strip the prefix. From 41a4a4967e1ad483bfcb6350975625d3b54608b1 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Wed, 12 Jul 2023 23:49:37 -0700 Subject: [PATCH 06/22] more cleanups --- go/tasks/logs/logging_utils.go | 3 +- go/tasks/pluginmachinery/tasklog/template.go | 10 ++---- .../pluginmachinery/tasklog/template_test.go | 31 ++++++------------- 3 files changed, 14 insertions(+), 30 deletions(-) diff --git a/go/tasks/logs/logging_utils.go b/go/tasks/logs/logging_utils.go index 49f3fc842..0b70d21db 100755 --- a/go/tasks/logs/logging_utils.go +++ b/go/tasks/logs/logging_utils.go @@ -5,8 +5,9 @@ import ( "fmt" "time" - "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/tasklog" + + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" "github.com/flyteorg/flytestdlib/logger" v1 "k8s.io/api/core/v1" ) diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index 4304bb057..e296baf81 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -66,10 +66,7 @@ type TemplateLogPlugin struct { messageFormat core.TaskLog_MessageFormat } -func (s TemplateLogPlugin) GetTaskLog( - podName, podUID, namespace, containerName, containerID, logName string, - podUnixStartTime, podUnixFinishTime int64, -) (core.TaskLog, error) { +func (s TemplateLogPlugin) GetTaskLog(podName, podUID, namespace, containerName, containerID, logName string, podUnixStartTime, podUnixFinishTime int64) (core.TaskLog, error) { o, err := s.GetTaskLogs(Input{ LogName: logName, Namespace: namespace, @@ -124,10 +121,7 @@ func (s TemplateLogPlugin) GetTaskLogs(input Input, extraTemplateVars ...Templat // {{ .hostname }}: The hostname where the pod is running and where logs reside. // {{ .podUnixStartTime }}: The pod creation time (in unix seconds, not millis) // {{ .podUnixFinishTime }}: Don't have a good mechanism for this yet, but approximating with time.Now for now -func NewTemplateLogPlugin( - templateUris []string, - messageFormat core.TaskLog_MessageFormat, -) TemplateLogPlugin { +func NewTemplateLogPlugin(templateUris []string, messageFormat core.TaskLog_MessageFormat) TemplateLogPlugin { return TemplateLogPlugin{ templateUris: templateUris, messageFormat: messageFormat, diff --git a/go/tasks/pluginmachinery/tasklog/template_test.go b/go/tasks/pluginmachinery/tasklog/template_test.go index 58221dd3d..b74d4efa7 100644 --- a/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/go/tasks/pluginmachinery/tasklog/template_test.go @@ -11,12 +11,7 @@ import ( ) func TestTemplateLog(t *testing.T) { - p := NewTemplateLogPlugin( - []string{ - "https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.{{.podName}}_{{.podUID}}_{{.namespace}}_{{.containerName}}-{{.containerId}}.log", - }, - core.TaskLog_JSON, - ) + p := NewTemplateLogPlugin([]string{"https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.{{.podName}}_{{.podUID}}_{{.namespace}}_{{.containerName}}-{{.containerId}}.log"}, core.TaskLog_JSON) tl, err := p.GetTaskLog( "f-uuid-driver", "pod-uid", @@ -30,11 +25,14 @@ func TestTemplateLog(t *testing.T) { assert.NoError(t, err) assert.Equal(t, tl.GetName(), "main_logs") assert.Equal(t, tl.GetMessageFormat(), core.TaskLog_JSON) - assert.Equal( - t, - "https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.f-uuid-driver_pod-uid_flyteexamples-production_spark-kubernetes-driver-abc.log", - tl.Uri, - ) + assert.Equal(t, "https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.f-uuid-driver_pod-uid_flyteexamples-production_spark-kubernetes-driver-abc.log", tl.Uri) +} + +// Latest Run: Benchmark_mustInitTemplateRegexes-16 45960 26914 ns/op +func Benchmark_mustInitTemplateRegexes(b *testing.B) { + for i := 0; i < b.N; i++ { + mustInitTemplateRegexes() + } } func Test_templateLogPlugin_Regression(t *testing.T) { @@ -136,16 +134,7 @@ func Test_templateLogPlugin_Regression(t *testing.T) { messageFormat: tt.fields.messageFormat, } - got, err := s.GetTaskLog( - tt.args.podName, - tt.args.podUID, - tt.args.namespace, - tt.args.containerName, - tt.args.containerID, - tt.args.logName, - tt.args.podUnixStartTime, - tt.args.podUnixFinishTime, - ) + got, err := s.GetTaskLog(tt.args.podName, tt.args.podUID, tt.args.namespace, tt.args.containerName, tt.args.containerID, tt.args.logName, tt.args.podUnixStartTime, tt.args.podUnixFinishTime) if (err != nil) != tt.wantErr { t.Errorf("GetTaskLog() error = %v, wantErr %v", err, tt.wantErr) return From 3dc64197d1aacccb8fc7a1493c7b6491c07dd865 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Wed, 12 Jul 2023 23:55:02 -0700 Subject: [PATCH 07/22] more cleanups --- go/tasks/pluginmachinery/tasklog/template.go | 8 ++------ go/tasks/pluginmachinery/tasklog/template_test.go | 7 ------- go/tasks/plugins/array/k8s/subtask_exec_context.go | 2 +- go/tasks/plugins/k8s/pod/plugin.go | 2 +- 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index e296baf81..60d1d059b 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -40,13 +40,9 @@ func (input Input) ToTemplateVars() TemplateVars { } } -type TaskExecutionIdentifierTemplateVarsProvider struct { - core.TaskExecutionIdentifier -} - -func (p TaskExecutionIdentifierTemplateVarsProvider) ToTemplateVars() TemplateVars { +func GetTaskExecutionIdentifierTemplateVars(id core.TaskExecutionIdentifier) TemplateVars { var templateVars TemplateVars - serialized, _ := json.Marshal(p.TaskExecutionIdentifier) + serialized, _ := json.Marshal(id) _ = json.Unmarshal(serialized, &templateVars) return templateVars } diff --git a/go/tasks/pluginmachinery/tasklog/template_test.go b/go/tasks/pluginmachinery/tasklog/template_test.go index b74d4efa7..5a2ee05cc 100644 --- a/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/go/tasks/pluginmachinery/tasklog/template_test.go @@ -28,13 +28,6 @@ func TestTemplateLog(t *testing.T) { assert.Equal(t, "https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.f-uuid-driver_pod-uid_flyteexamples-production_spark-kubernetes-driver-abc.log", tl.Uri) } -// Latest Run: Benchmark_mustInitTemplateRegexes-16 45960 26914 ns/op -func Benchmark_mustInitTemplateRegexes(b *testing.B) { - for i := 0; i < b.N; i++ { - mustInitTemplateRegexes() - } -} - func Test_templateLogPlugin_Regression(t *testing.T) { type fields struct { templateURI string diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context.go b/go/tasks/plugins/array/k8s/subtask_exec_context.go index ad0b3edad..a803840e1 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context.go @@ -177,7 +177,7 @@ func (s SubTaskExecutionID) GetLogSuffix() string { } func (s SubTaskExecutionID) ToTemplateVars() tasklog.TemplateVars { - v := tasklog.TaskExecutionIdentifierTemplateVarsProvider{s.GetID()}.ToTemplateVars() + v := tasklog.GetTaskExecutionIdentifierTemplateVars(s.GetID()) v.Merge(tasklog.TemplateVars{ "executionIndex": s.executionIndex, "parentName": s.parentName, diff --git a/go/tasks/plugins/k8s/pod/plugin.go b/go/tasks/plugins/k8s/pod/plugin.go index f4349cac4..248317384 100644 --- a/go/tasks/plugins/k8s/pod/plugin.go +++ b/go/tasks/plugins/k8s/pod/plugin.go @@ -143,7 +143,7 @@ func (p plugin) GetTaskPhase(ctx context.Context, pluginContext k8s.PluginContex return pluginsCore.PhaseInfoUndefined, err } - extraLogTemplateVars := tasklog.TaskExecutionIdentifierTemplateVarsProvider{pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()}.ToTemplateVars() + extraLogTemplateVars := tasklog.GetTaskExecutionIdentifierTemplateVars(pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()) return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)", extraLogTemplateVars) } From d7b9f2206b15c4a1961ab6a460aa356e00ed582f Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 13 Jul 2023 00:59:12 -0700 Subject: [PATCH 08/22] Plumb through task context for plugins --- go/tasks/plugins/k8s/dask/dask.go | 12 ++++--- go/tasks/plugins/k8s/dask/dask_test.go | 13 ++++--- .../k8s/kfoperators/common/common_operator.go | 12 ++++--- .../common/common_operator_test.go | 35 +++++++++++++++++-- go/tasks/plugins/k8s/kfoperators/mpi/mpi.go | 2 +- .../plugins/k8s/kfoperators/mpi/mpi_test.go | 14 ++++---- .../k8s/kfoperators/pytorch/pytorch.go | 2 +- .../k8s/kfoperators/pytorch/pytorch_test.go | 17 +++++---- .../k8s/kfoperators/tensorflow/tensorflow.go | 2 +- .../kfoperators/tensorflow/tensorflow_test.go | 14 ++++---- go/tasks/plugins/k8s/spark/spark.go | 14 ++++---- go/tasks/plugins/k8s/spark/spark_test.go | 28 ++++++++------- 12 files changed, 107 insertions(+), 58 deletions(-) diff --git a/go/tasks/plugins/k8s/dask/dask.go b/go/tasks/plugins/k8s/dask/dask.go index cbf61c09e..f2b7648f3 100755 --- a/go/tasks/plugins/k8s/dask/dask.go +++ b/go/tasks/plugins/k8s/dask/dask.go @@ -324,11 +324,13 @@ func (p daskResourceHandler) GetTaskPhase(ctx context.Context, pluginContext k8s status == daskAPI.DaskJobClusterCreated if !isQueued { - o, err := logPlugin.GetTaskLogs(tasklog.Input{ - Namespace: job.ObjectMeta.Namespace, - PodName: job.Status.JobRunnerPodName, - LogName: "(User logs)", - }, + o, err := logPlugin.GetTaskLogs( + tasklog.Input{ + Namespace: job.ObjectMeta.Namespace, + PodName: job.Status.JobRunnerPodName, + LogName: "(User logs)", + }, + tasklog.GetTaskExecutionIdentifierTemplateVars(pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()), ) if err != nil { return pluginsCore.PhaseInfoUndefined, err diff --git a/go/tasks/plugins/k8s/dask/dask_test.go b/go/tasks/plugins/k8s/dask/dask_test.go index a04292fef..b31d0e256 100644 --- a/go/tasks/plugins/k8s/dask/dask_test.go +++ b/go/tasks/plugins/k8s/dask/dask_test.go @@ -491,35 +491,38 @@ func TestGetTaskPhaseDask(t *testing.T) { daskResourceHandler := daskResourceHandler{} ctx := context.TODO() - taskPhase, err := daskResourceHandler.GetTaskPhase(ctx, nil, dummyDaskJob(daskAPI.DaskJobCreated)) + taskTemplate := dummyDaskTaskTemplate("", nil) + taskCtx := dummyDaskTaskContext(taskTemplate, &v1.ResourceRequirements{}, false) + + taskPhase, err := daskResourceHandler.GetTaskPhase(ctx, taskCtx, dummyDaskJob(daskAPI.DaskJobCreated)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseInitializing) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, taskPhase.Info().Logs) assert.Nil(t, err) - taskPhase, err = daskResourceHandler.GetTaskPhase(ctx, nil, dummyDaskJob(daskAPI.DaskJobClusterCreated)) + taskPhase, err = daskResourceHandler.GetTaskPhase(ctx, taskCtx, dummyDaskJob(daskAPI.DaskJobClusterCreated)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseInitializing) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, taskPhase.Info().Logs) assert.Nil(t, err) - taskPhase, err = daskResourceHandler.GetTaskPhase(ctx, nil, dummyDaskJob(daskAPI.DaskJobRunning)) + taskPhase, err = daskResourceHandler.GetTaskPhase(ctx, taskCtx, dummyDaskJob(daskAPI.DaskJobRunning)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseRunning) assert.NotNil(t, taskPhase.Info()) assert.NotNil(t, taskPhase.Info().Logs) assert.Nil(t, err) - taskPhase, err = daskResourceHandler.GetTaskPhase(ctx, nil, dummyDaskJob(daskAPI.DaskJobSuccessful)) + taskPhase, err = daskResourceHandler.GetTaskPhase(ctx, taskCtx, dummyDaskJob(daskAPI.DaskJobSuccessful)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseSuccess) assert.NotNil(t, taskPhase.Info()) assert.NotNil(t, taskPhase.Info().Logs) assert.Nil(t, err) - taskPhase, err = daskResourceHandler.GetTaskPhase(ctx, nil, dummyDaskJob(daskAPI.DaskJobFailed)) + taskPhase, err = daskResourceHandler.GetTaskPhase(ctx, taskCtx, dummyDaskJob(daskAPI.DaskJobFailed)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseRetryableFailure) assert.NotNil(t, taskPhase.Info()) diff --git a/go/tasks/plugins/k8s/kfoperators/common/common_operator.go b/go/tasks/plugins/k8s/kfoperators/common/common_operator.go index 19e8a9724..b3831ced1 100644 --- a/go/tasks/plugins/k8s/kfoperators/common/common_operator.go +++ b/go/tasks/plugins/k8s/kfoperators/common/common_operator.go @@ -6,6 +6,7 @@ import ( "time" "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/flytek8s" + "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/k8s" "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/tasklog" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" @@ -104,8 +105,10 @@ func GetMPIPhaseInfo(currentCondition commonOp.JobCondition, occurredAt time.Tim } // GetLogs will return the logs for kubeflow job -func GetLogs(taskType string, name string, namespace string, hasMaster bool, +func GetLogs(pluginContext k8s.PluginContext, taskType string, name string, namespace string, hasMaster bool, workersCount int32, psReplicasCount int32, chiefReplicasCount int32) ([]*core.TaskLog, error) { + extraLogTemplateVars := tasklog.GetTaskExecutionIdentifierTemplateVars(pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()) + taskLogs := make([]*core.TaskLog, 0, 10) logPlugin, err := logs.InitializeLogPlugins(logs.GetLogConfig()) @@ -125,6 +128,7 @@ func GetLogs(taskType string, name string, namespace string, hasMaster bool, Namespace: namespace, LogName: "master", }, + extraLogTemplateVars, ) if masterErr != nil { return nil, masterErr @@ -137,7 +141,7 @@ func GetLogs(taskType string, name string, namespace string, hasMaster bool, workerLog, err := logPlugin.GetTaskLogs(tasklog.Input{ PodName: name + fmt.Sprintf("-worker-%d", workerIndex), Namespace: namespace, - }) + }, extraLogTemplateVars) if err != nil { return nil, err } @@ -153,7 +157,7 @@ func GetLogs(taskType string, name string, namespace string, hasMaster bool, psReplicaLog, err := logPlugin.GetTaskLogs(tasklog.Input{ PodName: name + fmt.Sprintf("-psReplica-%d", psReplicaIndex), Namespace: namespace, - }) + }, extraLogTemplateVars) if err != nil { return nil, err } @@ -164,7 +168,7 @@ func GetLogs(taskType string, name string, namespace string, hasMaster bool, chiefReplicaLog, err := logPlugin.GetTaskLogs(tasklog.Input{ PodName: name + fmt.Sprintf("-chiefReplica-%d", 0), Namespace: namespace, - }) + }, extraLogTemplateVars) if err != nil { return nil, err } diff --git a/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go b/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go index 96c4bcd87..ce4fa0e11 100644 --- a/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go +++ b/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go @@ -9,6 +9,7 @@ import ( "github.com/flyteorg/flyteplugins/go/tasks/logs" pluginsCore "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/core" + "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/core/mocks" commonOp "github.com/kubeflow/common/pkg/apis/common/v1" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" @@ -167,18 +168,19 @@ func TestGetLogs(t *testing.T) { workers := int32(1) launcher := int32(1) - jobLogs, err := GetLogs(MPITaskType, "test", "mpi-namespace", false, workers, launcher, 0) + taskCtx := dummyTaskContext() + jobLogs, err := GetLogs(taskCtx, MPITaskType, "test", "mpi-namespace", false, workers, launcher, 0) assert.NoError(t, err) assert.Equal(t, 1, len(jobLogs)) assert.Equal(t, fmt.Sprintf("k8s.com/#!/log/%s/%s-worker-0/pod?namespace=mpi-namespace", "mpi-namespace", "test"), jobLogs[0].Uri) - jobLogs, err = GetLogs(PytorchTaskType, "test", "pytorch-namespace", true, workers, launcher, 0) + jobLogs, err = GetLogs(taskCtx, PytorchTaskType, "test", "pytorch-namespace", true, workers, launcher, 0) assert.NoError(t, err) assert.Equal(t, 2, len(jobLogs)) assert.Equal(t, fmt.Sprintf("k8s.com/#!/log/%s/%s-master-0/pod?namespace=pytorch-namespace", "pytorch-namespace", "test"), jobLogs[0].Uri) assert.Equal(t, fmt.Sprintf("k8s.com/#!/log/%s/%s-worker-0/pod?namespace=pytorch-namespace", "pytorch-namespace", "test"), jobLogs[1].Uri) - jobLogs, err = GetLogs(TensorflowTaskType, "test", "tensorflow-namespace", false, workers, launcher, 1) + jobLogs, err = GetLogs(taskCtx, TensorflowTaskType, "test", "tensorflow-namespace", false, workers, launcher, 1) assert.NoError(t, err) assert.Equal(t, 3, len(jobLogs)) assert.Equal(t, fmt.Sprintf("k8s.com/#!/log/%s/%s-worker-0/pod?namespace=tensorflow-namespace", "tensorflow-namespace", "test"), jobLogs[0].Uri) @@ -284,3 +286,30 @@ func TestOverrideContainerNilResources(t *testing.T) { assert.Nil(t, podSpec.Containers[0].Resources.Limits) assert.Nil(t, podSpec.Containers[0].Resources.Requests) } + +func dummyTaskContext() pluginsCore.TaskExecutionContext { + taskCtx := &mocks.TaskExecutionContext{} + + tID := &mocks.TaskExecutionID{} + tID.OnGetID().Return(core.TaskExecutionIdentifier{ + TaskId: &core.Identifier{ + ResourceType: core.ResourceType_TASK, + Name: "my_name", + Project: "my_project", + Domain: "my_domain", + Version: "1", + }, + NodeExecutionId: &core.NodeExecutionIdentifier{ + ExecutionId: &core.WorkflowExecutionIdentifier{ + Name: "my_name", + Project: "my_project", + Domain: "my_domain", + }, + }, + }) + + taskExecutionMetadata := &mocks.TaskExecutionMetadata{} + taskExecutionMetadata.OnGetTaskExecutionID().Return(tID) + taskCtx.OnTaskExecutionMetadata().Return(taskExecutionMetadata) + return taskCtx +} diff --git a/go/tasks/plugins/k8s/kfoperators/mpi/mpi.go b/go/tasks/plugins/k8s/kfoperators/mpi/mpi.go index e9e1f6037..8ca9cfc88 100644 --- a/go/tasks/plugins/k8s/kfoperators/mpi/mpi.go +++ b/go/tasks/plugins/k8s/kfoperators/mpi/mpi.go @@ -204,7 +204,7 @@ func (mpiOperatorResourceHandler) GetTaskPhase(_ context.Context, pluginContext numWorkers = app.Spec.MPIReplicaSpecs[kubeflowv1.MPIJobReplicaTypeWorker].Replicas numLauncherReplicas = app.Spec.MPIReplicaSpecs[kubeflowv1.MPIJobReplicaTypeLauncher].Replicas - taskLogs, err := common.GetLogs(common.MPITaskType, app.Name, app.Namespace, false, + taskLogs, err := common.GetLogs(pluginContext, common.MPITaskType, app.Name, app.Namespace, false, *numWorkers, *numLauncherReplicas, 0) if err != nil { return pluginsCore.PhaseInfoUndefined, err diff --git a/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go b/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go index 9ec11f5da..b35b368df 100644 --- a/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go +++ b/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go @@ -346,31 +346,32 @@ func TestGetTaskPhase(t *testing.T) { return dummyMPIJobResource(mpiResourceHandler, 2, 1, 1, conditionType) } - taskPhase, err := mpiResourceHandler.GetTaskPhase(ctx, nil, dummyMPIJobResourceCreator(mpiOp.JobCreated)) + taskCtx := dummyMPITaskContext(dummyMPITaskTemplate("", dummyMPICustomObj(2, 1, 1))) + taskPhase, err := mpiResourceHandler.GetTaskPhase(ctx, taskCtx, dummyMPIJobResourceCreator(mpiOp.JobCreated)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseQueued, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = mpiResourceHandler.GetTaskPhase(ctx, nil, dummyMPIJobResourceCreator(mpiOp.JobRunning)) + taskPhase, err = mpiResourceHandler.GetTaskPhase(ctx, taskCtx, dummyMPIJobResourceCreator(mpiOp.JobRunning)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseRunning, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = mpiResourceHandler.GetTaskPhase(ctx, nil, dummyMPIJobResourceCreator(mpiOp.JobSucceeded)) + taskPhase, err = mpiResourceHandler.GetTaskPhase(ctx, taskCtx, dummyMPIJobResourceCreator(mpiOp.JobSucceeded)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseSuccess, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = mpiResourceHandler.GetTaskPhase(ctx, nil, dummyMPIJobResourceCreator(mpiOp.JobFailed)) + taskPhase, err = mpiResourceHandler.GetTaskPhase(ctx, taskCtx, dummyMPIJobResourceCreator(mpiOp.JobFailed)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseRetryableFailure, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = mpiResourceHandler.GetTaskPhase(ctx, nil, dummyMPIJobResourceCreator(mpiOp.JobRestarting)) + taskPhase, err = mpiResourceHandler.GetTaskPhase(ctx, taskCtx, dummyMPIJobResourceCreator(mpiOp.JobRestarting)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseRunning, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) @@ -389,7 +390,8 @@ func TestGetLogs(t *testing.T) { mpiResourceHandler := mpiOperatorResourceHandler{} mpiJob := dummyMPIJobResource(mpiResourceHandler, workers, launcher, slots, mpiOp.JobRunning) - jobLogs, err := common.GetLogs(common.MPITaskType, mpiJob.Name, mpiJob.Namespace, false, workers, launcher, 0) + taskCtx := dummyMPITaskContext(dummyMPITaskTemplate("", dummyMPICustomObj(workers, launcher, slots))) + jobLogs, err := common.GetLogs(taskCtx, common.MPITaskType, mpiJob.Name, mpiJob.Namespace, false, workers, launcher, 0) assert.NoError(t, err) assert.Equal(t, 2, len(jobLogs)) assert.Equal(t, fmt.Sprintf("k8s.com/#!/log/%s/%s-worker-0/pod?namespace=mpi-namespace", jobNamespace, jobName), jobLogs[0].Uri) diff --git a/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch.go b/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch.go index 1e9fe6115..39abc56f0 100644 --- a/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch.go +++ b/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch.go @@ -209,7 +209,7 @@ func (pytorchOperatorResourceHandler) GetTaskPhase(_ context.Context, pluginCont workersCount := app.Spec.PyTorchReplicaSpecs[kubeflowv1.PyTorchJobReplicaTypeWorker].Replicas - taskLogs, err := common.GetLogs(common.PytorchTaskType, app.Name, app.Namespace, hasMaster, *workersCount, 0, 0) + taskLogs, err := common.GetLogs(pluginContext, common.PytorchTaskType, app.Name, app.Namespace, hasMaster, *workersCount, 0, 0) if err != nil { return pluginsCore.PhaseInfoUndefined, err } diff --git a/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch_test.go b/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch_test.go index 4a17b7490..0e9f6de52 100644 --- a/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch_test.go +++ b/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch_test.go @@ -379,31 +379,32 @@ func TestGetTaskPhase(t *testing.T) { return dummyPytorchJobResource(pytorchResourceHandler, 2, conditionType) } - taskPhase, err := pytorchResourceHandler.GetTaskPhase(ctx, nil, dummyPytorchJobResourceCreator(commonOp.JobCreated)) + taskCtx := dummyPytorchTaskContext(dummyPytorchTaskTemplate("", dummyPytorchCustomObj(2))) + taskPhase, err := pytorchResourceHandler.GetTaskPhase(ctx, taskCtx, dummyPytorchJobResourceCreator(commonOp.JobCreated)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseQueued, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = pytorchResourceHandler.GetTaskPhase(ctx, nil, dummyPytorchJobResourceCreator(commonOp.JobRunning)) + taskPhase, err = pytorchResourceHandler.GetTaskPhase(ctx, taskCtx, dummyPytorchJobResourceCreator(commonOp.JobRunning)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseRunning, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = pytorchResourceHandler.GetTaskPhase(ctx, nil, dummyPytorchJobResourceCreator(commonOp.JobSucceeded)) + taskPhase, err = pytorchResourceHandler.GetTaskPhase(ctx, taskCtx, dummyPytorchJobResourceCreator(commonOp.JobSucceeded)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseSuccess, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = pytorchResourceHandler.GetTaskPhase(ctx, nil, dummyPytorchJobResourceCreator(commonOp.JobFailed)) + taskPhase, err = pytorchResourceHandler.GetTaskPhase(ctx, taskCtx, dummyPytorchJobResourceCreator(commonOp.JobFailed)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseRetryableFailure, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = pytorchResourceHandler.GetTaskPhase(ctx, nil, dummyPytorchJobResourceCreator(commonOp.JobRestarting)) + taskPhase, err = pytorchResourceHandler.GetTaskPhase(ctx, taskCtx, dummyPytorchJobResourceCreator(commonOp.JobRestarting)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseRunning, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) @@ -421,7 +422,8 @@ func TestGetLogs(t *testing.T) { pytorchResourceHandler := pytorchOperatorResourceHandler{} pytorchJob := dummyPytorchJobResource(pytorchResourceHandler, workers, commonOp.JobRunning) - jobLogs, err := common.GetLogs(common.PytorchTaskType, pytorchJob.Name, pytorchJob.Namespace, hasMaster, workers, 0, 0) + taskCtx := dummyPytorchTaskContext(dummyPytorchTaskTemplate("", dummyPytorchCustomObj(workers))) + jobLogs, err := common.GetLogs(taskCtx, common.PytorchTaskType, pytorchJob.Name, pytorchJob.Namespace, hasMaster, workers, 0, 0) assert.NoError(t, err) assert.Equal(t, 3, len(jobLogs)) assert.Equal(t, fmt.Sprintf("k8s.com/#!/log/%s/%s-master-0/pod?namespace=pytorch-namespace", jobNamespace, jobName), jobLogs[0].Uri) @@ -440,7 +442,8 @@ func TestGetLogsElastic(t *testing.T) { pytorchResourceHandler := pytorchOperatorResourceHandler{} pytorchJob := dummyPytorchJobResource(pytorchResourceHandler, workers, commonOp.JobRunning) - jobLogs, err := common.GetLogs(common.PytorchTaskType, pytorchJob.Name, pytorchJob.Namespace, hasMaster, workers, 0, 0) + taskCtx := dummyPytorchTaskContext(dummyPytorchTaskTemplate("", dummyPytorchCustomObj(workers))) + jobLogs, err := common.GetLogs(taskCtx, common.PytorchTaskType, pytorchJob.Name, pytorchJob.Namespace, hasMaster, workers, 0, 0) assert.NoError(t, err) assert.Equal(t, 2, len(jobLogs)) assert.Equal(t, fmt.Sprintf("k8s.com/#!/log/%s/%s-worker-0/pod?namespace=pytorch-namespace", jobNamespace, jobName), jobLogs[0].Uri) diff --git a/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow.go b/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow.go index ea2930d6f..762eaff3b 100644 --- a/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow.go +++ b/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow.go @@ -203,7 +203,7 @@ func (tensorflowOperatorResourceHandler) GetTaskPhase(_ context.Context, pluginC psReplicasCount := app.Spec.TFReplicaSpecs[kubeflowv1.TFJobReplicaTypePS].Replicas chiefCount := app.Spec.TFReplicaSpecs[kubeflowv1.TFJobReplicaTypeChief].Replicas - taskLogs, err := common.GetLogs(common.TensorflowTaskType, app.Name, app.Namespace, false, + taskLogs, err := common.GetLogs(pluginContext, common.TensorflowTaskType, app.Name, app.Namespace, false, *workersCount, *psReplicasCount, *chiefCount) if err != nil { return pluginsCore.PhaseInfoUndefined, err diff --git a/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow_test.go b/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow_test.go index 4e2bc1388..4a0ec5766 100644 --- a/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow_test.go +++ b/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow_test.go @@ -330,31 +330,32 @@ func TestGetTaskPhase(t *testing.T) { return dummyTensorFlowJobResource(tensorflowResourceHandler, 2, 1, 1, conditionType) } - taskPhase, err := tensorflowResourceHandler.GetTaskPhase(ctx, nil, dummyTensorFlowJobResourceCreator(commonOp.JobCreated)) + taskCtx := dummyTensorFlowTaskContext(dummyTensorFlowTaskTemplate("", dummyTensorFlowCustomObj(2, 1, 1))) + taskPhase, err := tensorflowResourceHandler.GetTaskPhase(ctx, taskCtx, dummyTensorFlowJobResourceCreator(commonOp.JobCreated)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseQueued, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = tensorflowResourceHandler.GetTaskPhase(ctx, nil, dummyTensorFlowJobResourceCreator(commonOp.JobRunning)) + taskPhase, err = tensorflowResourceHandler.GetTaskPhase(ctx, taskCtx, dummyTensorFlowJobResourceCreator(commonOp.JobRunning)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseRunning, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = tensorflowResourceHandler.GetTaskPhase(ctx, nil, dummyTensorFlowJobResourceCreator(commonOp.JobSucceeded)) + taskPhase, err = tensorflowResourceHandler.GetTaskPhase(ctx, taskCtx, dummyTensorFlowJobResourceCreator(commonOp.JobSucceeded)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseSuccess, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = tensorflowResourceHandler.GetTaskPhase(ctx, nil, dummyTensorFlowJobResourceCreator(commonOp.JobFailed)) + taskPhase, err = tensorflowResourceHandler.GetTaskPhase(ctx, taskCtx, dummyTensorFlowJobResourceCreator(commonOp.JobFailed)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseRetryableFailure, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = tensorflowResourceHandler.GetTaskPhase(ctx, nil, dummyTensorFlowJobResourceCreator(commonOp.JobRestarting)) + taskPhase, err = tensorflowResourceHandler.GetTaskPhase(ctx, taskCtx, dummyTensorFlowJobResourceCreator(commonOp.JobRestarting)) assert.NoError(t, err) assert.Equal(t, pluginsCore.PhaseRunning, taskPhase.Phase()) assert.NotNil(t, taskPhase.Info()) @@ -373,7 +374,8 @@ func TestGetLogs(t *testing.T) { tensorflowResourceHandler := tensorflowOperatorResourceHandler{} tensorFlowJob := dummyTensorFlowJobResource(tensorflowResourceHandler, workers, psReplicas, chiefReplicas, commonOp.JobRunning) - jobLogs, err := common.GetLogs(common.TensorflowTaskType, tensorFlowJob.Name, tensorFlowJob.Namespace, false, + taskCtx := dummyTensorFlowTaskContext(dummyTensorFlowTaskTemplate("", dummyTensorFlowCustomObj(workers, psReplicas, chiefReplicas))) + jobLogs, err := common.GetLogs(taskCtx, common.TensorflowTaskType, tensorFlowJob.Name, tensorFlowJob.Namespace, false, workers, psReplicas, chiefReplicas) assert.NoError(t, err) assert.Equal(t, 4, len(jobLogs)) diff --git a/go/tasks/plugins/k8s/spark/spark.go b/go/tasks/plugins/k8s/spark/spark.go index 474ce819c..0d735205f 100755 --- a/go/tasks/plugins/k8s/spark/spark.go +++ b/go/tasks/plugins/k8s/spark/spark.go @@ -305,12 +305,14 @@ func (sparkResourceHandler) BuildIdentityResource(ctx context.Context, taskCtx p }, nil } -func getEventInfoForSpark(sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, error) { +func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, error) { state := sj.Status.AppState.State isQueued := state == sparkOp.NewState || state == sparkOp.PendingSubmissionState || state == sparkOp.SubmittedState + extraLogTemplateVars := tasklog.GetTaskExecutionIdentifierTemplateVars(pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()) + sparkConfig := GetSparkConfig() taskLogs := make([]*core.TaskLog, 0, 3) @@ -326,7 +328,7 @@ func getEventInfoForSpark(sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, PodName: sj.Status.DriverInfo.PodName, Namespace: sj.Namespace, LogName: "(Driver Logs)", - }) + }, extraLogTemplateVars) if err != nil { return nil, err @@ -346,7 +348,7 @@ func getEventInfoForSpark(sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, PodName: sj.Status.DriverInfo.PodName, Namespace: sj.Namespace, LogName: "(User Logs)", - }) + }, extraLogTemplateVars) if err != nil { return nil, err @@ -365,7 +367,7 @@ func getEventInfoForSpark(sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, PodName: sj.Name, Namespace: sj.Namespace, LogName: "(System Logs)", - }) + }, extraLogTemplateVars) if err != nil { return nil, err @@ -385,7 +387,7 @@ func getEventInfoForSpark(sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, PodName: sj.Name, Namespace: sj.Namespace, LogName: "(Spark-Submit/All User Logs)", - }) + }, extraLogTemplateVars) if err != nil { return nil, err @@ -432,7 +434,7 @@ func getEventInfoForSpark(sj *sparkOp.SparkApplication) (*pluginsCore.TaskInfo, func (sparkResourceHandler) GetTaskPhase(ctx context.Context, pluginContext k8s.PluginContext, resource client.Object) (pluginsCore.PhaseInfo, error) { app := resource.(*sparkOp.SparkApplication) - info, err := getEventInfoForSpark(app) + info, err := getEventInfoForSpark(pluginContext, app) if err != nil { return pluginsCore.PhaseInfoUndefined, err } diff --git a/go/tasks/plugins/k8s/spark/spark_test.go b/go/tasks/plugins/k8s/spark/spark_test.go index aef61c0f7..51a7c8935 100755 --- a/go/tasks/plugins/k8s/spark/spark_test.go +++ b/go/tasks/plugins/k8s/spark/spark_test.go @@ -88,7 +88,8 @@ func TestGetEventInfo(t *testing.T) { }, }, })) - info, err := getEventInfoForSpark(dummySparkApplication(sj.RunningState)) + taskCtx := dummySparkTaskContext(dummySparkTaskTemplate("blah-1", dummySparkConf), false) + info, err := getEventInfoForSpark(taskCtx, dummySparkApplication(sj.RunningState)) assert.NoError(t, err) assert.Len(t, info.Logs, 6) assert.Equal(t, fmt.Sprintf("https://%s", sparkUIAddress), info.CustomInfo.Fields[sparkDriverUI].GetStringValue()) @@ -108,7 +109,7 @@ func TestGetEventInfo(t *testing.T) { assert.Equal(t, expectedLinks, generatedLinks) - info, err = getEventInfoForSpark(dummySparkApplication(sj.SubmittedState)) + info, err = getEventInfoForSpark(taskCtx, dummySparkApplication(sj.SubmittedState)) assert.NoError(t, err) assert.Len(t, info.Logs, 1) assert.Equal(t, "https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logStream:group=/kubernetes/flyte;prefix=var.log.containers.spark-app-name;streamFilter=typeLogStreamPrefix", info.Logs[0].Uri) @@ -133,7 +134,7 @@ func TestGetEventInfo(t *testing.T) { }, })) - info, err = getEventInfoForSpark(dummySparkApplication(sj.FailedState)) + info, err = getEventInfoForSpark(taskCtx, dummySparkApplication(sj.FailedState)) assert.NoError(t, err) assert.Len(t, info.Logs, 5) assert.Equal(t, "spark-history.flyte/history/app-id", info.CustomInfo.Fields[sparkHistoryUI].GetStringValue()) @@ -157,61 +158,62 @@ func TestGetTaskPhase(t *testing.T) { sparkResourceHandler := sparkResourceHandler{} ctx := context.TODO() - taskPhase, err := sparkResourceHandler.GetTaskPhase(ctx, nil, dummySparkApplication(sj.NewState)) + taskCtx := dummySparkTaskContext(dummySparkTaskTemplate("blah-1", dummySparkConf), false) + taskPhase, err := sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.NewState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseQueued) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, nil, dummySparkApplication(sj.SubmittedState)) + taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.SubmittedState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseInitializing) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, nil, dummySparkApplication(sj.RunningState)) + taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.RunningState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseRunning) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, nil, dummySparkApplication(sj.CompletedState)) + taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.CompletedState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseSuccess) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, nil, dummySparkApplication(sj.InvalidatingState)) + taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.InvalidatingState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseRunning) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, nil, dummySparkApplication(sj.FailingState)) + taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.FailingState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseRunning) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, nil, dummySparkApplication(sj.PendingRerunState)) + taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.PendingRerunState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseRunning) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, nil, dummySparkApplication(sj.SucceedingState)) + taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.SucceedingState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseRunning) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, nil, dummySparkApplication(sj.FailedSubmissionState)) + taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.FailedSubmissionState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseRetryableFailure) assert.NotNil(t, taskPhase.Info()) assert.Nil(t, err) - taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, nil, dummySparkApplication(sj.FailedState)) + taskPhase, err = sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.FailedState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseRetryableFailure) assert.NotNil(t, taskPhase.Info()) From fb647701f8fce5c75a60c0115b53a2617736beb1 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 13 Jul 2023 02:06:56 -0700 Subject: [PATCH 09/22] fix Signed-off-by: Jeev B --- go/tasks/plugins/k8s/spark/spark_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/tasks/plugins/k8s/spark/spark_test.go b/go/tasks/plugins/k8s/spark/spark_test.go index 51a7c8935..5750643a2 100755 --- a/go/tasks/plugins/k8s/spark/spark_test.go +++ b/go/tasks/plugins/k8s/spark/spark_test.go @@ -158,7 +158,7 @@ func TestGetTaskPhase(t *testing.T) { sparkResourceHandler := sparkResourceHandler{} ctx := context.TODO() - taskCtx := dummySparkTaskContext(dummySparkTaskTemplate("blah-1", dummySparkConf), false) + taskCtx := dummySparkTaskContext(dummySparkTaskTemplate("", dummySparkConf), false) taskPhase, err := sparkResourceHandler.GetTaskPhase(ctx, taskCtx, dummySparkApplication(sj.NewState)) assert.NoError(t, err) assert.Equal(t, taskPhase.Phase(), pluginsCore.PhaseQueued) From b562f2fa995b7c17feac8f7e7f2e8d7f6a79a77d Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 13 Jul 2023 10:10:44 -0700 Subject: [PATCH 10/22] move task execution identifier into Input struct --- go/tasks/logs/logging_utils.go | 27 ++++--- go/tasks/logs/logging_utils_test.go | 35 ++++++--- go/tasks/pluginmachinery/tasklog/plugin.go | 25 +++--- go/tasks/pluginmachinery/tasklog/template.go | 78 ++++++++++++------- .../plugins/array/k8s/subtask_exec_context.go | 16 ++-- go/tasks/plugins/k8s/dask/dask.go | 8 +- .../k8s/kfoperators/common/common_operator.go | 47 +++++------ go/tasks/plugins/k8s/pod/plugin.go | 5 +- go/tasks/plugins/k8s/spark/spark.go | 39 +++++----- 9 files changed, 166 insertions(+), 114 deletions(-) diff --git a/go/tasks/logs/logging_utils.go b/go/tasks/logs/logging_utils.go index a4ed081d0..aabfb662c 100755 --- a/go/tasks/logs/logging_utils.go +++ b/go/tasks/logs/logging_utils.go @@ -18,7 +18,7 @@ type logPlugin struct { } // Internal -func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) ([]*core.TaskLog, error) { +func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecId core.TaskExecutionIdentifier, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) ([]*core.TaskLog, error) { if logPlugin == nil { return nil, nil } @@ -43,16 +43,17 @@ func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, pod logs, err := logPlugin.GetTaskLogs( tasklog.Input{ - PodName: pod.Name, - PodUID: string(pod.GetUID()), - Namespace: pod.Namespace, - ContainerName: pod.Spec.Containers[index].Name, - ContainerID: pod.Status.ContainerStatuses[index].ContainerID, - LogName: nameSuffix, - PodRFC3339StartTime: time.Unix(startTime, 0).Format(time.RFC3339), - PodRFC3339FinishTime: time.Unix(finishTime, 0).Format(time.RFC3339), - PodUnixStartTime: startTime, - PodUnixFinishTime: finishTime, + PodName: pod.Name, + PodUID: string(pod.GetUID()), + Namespace: pod.Namespace, + ContainerName: pod.Spec.Containers[index].Name, + ContainerID: pod.Status.ContainerStatuses[index].ContainerID, + LogName: nameSuffix, + PodRFC3339StartTime: time.Unix(startTime, 0).Format(time.RFC3339), + PodRFC3339FinishTime: time.Unix(finishTime, 0).Format(time.RFC3339), + PodUnixStartTime: startTime, + PodUnixFinishTime: finishTime, + TaskExecutionIdentifier: taskExecId, }, extraLogTemplateVars..., ) @@ -73,7 +74,9 @@ func (t taskLogPluginWrapper) GetTaskLogs(input tasklog.Input, extraTemplateVars suffix := input.LogName mergedTemplateVars := make(tasklog.TemplateVars) - mergedTemplateVars.Merge(extraTemplateVars...) + if err := mergedTemplateVars.Merge(extraTemplateVars...); err != nil { + return tasklog.Output{}, err + } for _, plugin := range t.logPlugins { input.LogName = plugin.Name + suffix diff --git a/go/tasks/logs/logging_utils_test.go b/go/tasks/logs/logging_utils_test.go index 02f5d778b..b9c3315bd 100755 --- a/go/tasks/logs/logging_utils_test.go +++ b/go/tasks/logs/logging_utils_test.go @@ -14,10 +14,27 @@ import ( const podName = "PodName" +var dummyTaskExecId = core.TaskExecutionIdentifier{ + TaskId: &core.Identifier{ + ResourceType: core.ResourceType_TASK, + Name: "my_name", + Project: "my_project", + Domain: "my_domain", + Version: "1", + }, + NodeExecutionId: &core.NodeExecutionIdentifier{ + ExecutionId: &core.WorkflowExecutionIdentifier{ + Name: "my_name", + Project: "my_project", + Domain: "my_domain", + }, + }, +} + func TestGetLogsForContainerInPod_NoPlugins(t *testing.T) { logPlugin, err := InitializeLogPlugins(&LogConfig{}) assert.NoError(t, err) - l, err := GetLogsForContainerInPod(context.TODO(), logPlugin, nil, 0, " Suffix") + l, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, nil, 0, " Suffix") assert.NoError(t, err) assert.Nil(t, l) } @@ -29,7 +46,7 @@ func TestGetLogsForContainerInPod_NoLogs(t *testing.T) { CloudwatchLogGroup: "/kubernetes/flyte-production", }) assert.NoError(t, err) - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, nil, 0, " Suffix") + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, nil, 0, " Suffix") assert.NoError(t, err) assert.Nil(t, p) } @@ -60,7 +77,7 @@ func TestGetLogsForContainerInPod_BadIndex(t *testing.T) { } pod.Name = podName - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, pod, 1, " Suffix") + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 1, " Suffix") assert.NoError(t, err) assert.Nil(t, p) } @@ -85,7 +102,7 @@ func TestGetLogsForContainerInPod_MissingStatus(t *testing.T) { } pod.Name = podName - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, pod, 1, " Suffix") + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 1, " Suffix") assert.NoError(t, err) assert.Nil(t, p) } @@ -115,7 +132,7 @@ func TestGetLogsForContainerInPod_Cloudwatch(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 0, " Suffix") assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -145,7 +162,7 @@ func TestGetLogsForContainerInPod_K8s(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 0, " Suffix") assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -178,7 +195,7 @@ func TestGetLogsForContainerInPod_All(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 0, " Suffix") assert.Nil(t, err) assert.Len(t, logs, 2) } @@ -209,7 +226,7 @@ func TestGetLogsForContainerInPod_Stackdriver(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 0, " Suffix") assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -283,7 +300,7 @@ func assertTestSucceeded(tb testing.TB, config *LogConfig, expectedTaskLogs []*c }, } - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, pod, 0, " my-Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 0, " my-Suffix") assert.Nil(tb, err) assert.Len(tb, logs, len(expectedTaskLogs)) if diff := deep.Equal(logs, expectedTaskLogs); len(diff) > 0 { diff --git a/go/tasks/pluginmachinery/tasklog/plugin.go b/go/tasks/pluginmachinery/tasklog/plugin.go index 3b8a3bad0..77204dc5c 100644 --- a/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/go/tasks/pluginmachinery/tasklog/plugin.go @@ -7,22 +7,23 @@ type TemplateVars map[string]interface{} // Input contains all available information about task's execution that a log plugin can use to construct task's // log links. type Input struct { - HostName string `json:"hostname"` - PodName string `json:"podName"` - Namespace string `json:"namespace"` - ContainerName string `json:"containerName"` - ContainerID string `json:"containerId"` - LogName string `json:"logName"` - PodRFC3339StartTime string `json:"podRFC3339StartTime"` - PodRFC3339FinishTime string `json:"podRFC3339FinishTime"` - PodUnixStartTime int64 `json:"podUnixStartTime"` - PodUnixFinishTime int64 `json:"podUnixFinishTime"` - PodUID string `json:"podUID"` + HostName string + PodName string + Namespace string + ContainerName string + ContainerID string + LogName string + PodRFC3339StartTime string + PodRFC3339FinishTime string + PodUnixStartTime int64 + PodUnixFinishTime int64 + PodUID string + TaskExecutionIdentifier core.TaskExecutionIdentifier } // Output contains all task logs a plugin generates for a given Input. type Output struct { - TaskLogs []*core.TaskLog `json:"taskLogs"` + TaskLogs []*core.TaskLog } // Plugin represents an interface for task log plugins to implement to plug generated task log links into task events. diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index db0c4ec51..90bf92e61 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -8,18 +8,21 @@ import ( "text/template" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" + + "github.com/imdario/mergo" ) -func (t TemplateVars) Merge(others ...TemplateVars) { - for _, other := range others { - for k, v := range other { - t[k] = v +func (t TemplateVars) Merge(others ...TemplateVars) error { + for _, o := range others { + err := mergo.Merge(&t, o) + if err != nil { + return err } } + return nil } -func (input Input) ToTemplateVars() TemplateVars { - // Container IDs are prefixed with docker://, cri-o://, etc. which is stripped by fluentd before pushing to a log +func (input Input) ToTemplateVars() (TemplateVars, error) { // stream. Therefore, we must also strip the prefix. containerID := input.ContainerID stripDelimiter := "://" @@ -27,26 +30,42 @@ func (input Input) ToTemplateVars() TemplateVars { containerID = split[1] } - return TemplateVars{ - "podName": input.PodName, - "podUID": input.PodUID, - "namespace": input.Namespace, - "containerName": input.ContainerName, - "containerId": containerID, - "logName": input.LogName, - "hostname": input.HostName, - "podRFC3339StartTime": input.PodRFC3339StartTime, - "podRFC3339FinishTime": input.PodRFC3339FinishTime, - "podUnixStartTime": strconv.FormatInt(input.PodUnixStartTime, 10), - "podUnixFinishTime": strconv.FormatInt(input.PodUnixFinishTime, 10), + var templateVars TemplateVars + var err error + + serialized, err := json.Marshal(&struct { + HostName string `json:"hostname,omitempty"` + PodName string `json:"podName,omitempty"` + Namespace string `json:"namespace,omitempty"` + ContainerName string `json:"containerName,omitempty"` + ContainerID string `json:"containerId,omitempty"` + LogName string `json:"logName,omitempty"` + PodRFC3339StartTime string `json:"podRFC3339StartTime,omitempty"` + PodRFC3339FinishTime string `json:"podRFC3339FinishTime,omitempty"` + PodUnixStartTime string `json:"podUnixStartTime,omitempty"` + PodUnixFinishTime string `json:"podUnixFinishTime,omitempty"` + PodUID string `json:"podUID,omitempty"` + TaskExecutionIdentifier core.TaskExecutionIdentifier `json:"taskExecution"` + }{ + input.HostName, + input.PodName, + input.Namespace, + input.ContainerName, + containerID, + input.LogName, + input.PodRFC3339StartTime, + input.PodRFC3339FinishTime, + strconv.FormatInt(input.PodUnixStartTime, 10), + strconv.FormatInt(input.PodUnixFinishTime, 10), + input.PodUID, + input.TaskExecutionIdentifier, + }) + if err != nil { + return templateVars, err } -} -func GetTaskExecutionIdentifierTemplateVars(id core.TaskExecutionIdentifier) TemplateVars { - var templateVars TemplateVars - serialized, _ := json.Marshal(id) - _ = json.Unmarshal(serialized, &templateVars) - return templateVars + err = json.Unmarshal(serialized, &templateVars) + return templateVars, err } // A simple log plugin that supports templates in urls to build the final log link. Supported templates are: @@ -88,8 +107,15 @@ func (s TemplateLogPlugin) GetTaskLog(podName, podUID, namespace, containerName, } func (s TemplateLogPlugin) GetTaskLogs(input Input, extraTemplateVars ...TemplateVars) (Output, error) { - templateVars := input.ToTemplateVars() - templateVars.Merge(extraTemplateVars...) + var err error + templateVars, err := input.ToTemplateVars() + if err != nil { + return Output{}, err + } + err = templateVars.Merge(extraTemplateVars...) + if err != nil { + return Output{}, err + } taskLogs := make([]*core.TaskLog, 0, len(s.templateUris)) for _, templateURI := range s.templateUris { diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context.go b/go/tasks/plugins/array/k8s/subtask_exec_context.go index a803840e1..0d4fd5ba5 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context.go @@ -177,14 +177,14 @@ func (s SubTaskExecutionID) GetLogSuffix() string { } func (s SubTaskExecutionID) ToTemplateVars() tasklog.TemplateVars { - v := tasklog.GetTaskExecutionIdentifierTemplateVars(s.GetID()) - v.Merge(tasklog.TemplateVars{ - "executionIndex": s.executionIndex, - "parentName": s.parentName, - "subtaskRetryAttempt": s.subtaskRetryAttempt, - "taskRetryAttempt": s.taskRetryAttempt, - }) - return v + return tasklog.TemplateVars{ + "taskExecution": tasklog.TemplateVars{ + "executionIndex": s.executionIndex, + "parentName": s.parentName, + "subtaskRetryAttempt": s.subtaskRetryAttempt, + "taskRetryAttempt": s.taskRetryAttempt, + }, + } } // NewSubtaskExecutionID constructs a SubTaskExecutionID using the provided parameters diff --git a/go/tasks/plugins/k8s/dask/dask.go b/go/tasks/plugins/k8s/dask/dask.go index 49bfc472f..bc6c7f663 100755 --- a/go/tasks/plugins/k8s/dask/dask.go +++ b/go/tasks/plugins/k8s/dask/dask.go @@ -328,11 +328,11 @@ func (p daskResourceHandler) GetTaskPhase(ctx context.Context, pluginContext k8s if !isQueued { o, err := logPlugin.GetTaskLogs( tasklog.Input{ - Namespace: job.ObjectMeta.Namespace, - PodName: job.Status.JobRunnerPodName, - LogName: "(User logs)", + Namespace: job.ObjectMeta.Namespace, + PodName: job.Status.JobRunnerPodName, + LogName: "(User logs)", + TaskExecutionIdentifier: pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID(), }, - tasklog.GetTaskExecutionIdentifierTemplateVars(pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()), ) if err != nil { return pluginsCore.PhaseInfoUndefined, err diff --git a/go/tasks/plugins/k8s/kfoperators/common/common_operator.go b/go/tasks/plugins/k8s/kfoperators/common/common_operator.go index 0296a7977..aaf68577a 100644 --- a/go/tasks/plugins/k8s/kfoperators/common/common_operator.go +++ b/go/tasks/plugins/k8s/kfoperators/common/common_operator.go @@ -112,9 +112,9 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v namespace := objectMeta.Namespace taskLogs := make([]*core.TaskLog, 0, 10) + taskExecId := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() logPlugin, err := logs.InitializeLogPlugins(logs.GetLogConfig()) - extraLogTemplateVars := tasklog.GetTaskExecutionIdentifierTemplateVars(pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()) if err != nil { return nil, err @@ -134,15 +134,15 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v if taskType == PytorchTaskType && hasMaster { masterTaskLog, masterErr := logPlugin.GetTaskLogs( tasklog.Input{ - PodName: name + "-master-0", - Namespace: namespace, - LogName: "master", - PodRFC3339StartTime: RFC3999StartTime, - PodRFC3339FinishTime: RFC3999FinishTime, - PodUnixStartTime: startTime, - PodUnixFinishTime: finishTime, + PodName: name + "-master-0", + Namespace: namespace, + LogName: "master", + PodRFC3339StartTime: RFC3999StartTime, + PodRFC3339FinishTime: RFC3999FinishTime, + PodUnixStartTime: startTime, + PodUnixFinishTime: finishTime, + TaskExecutionIdentifier: taskExecId, }, - extraLogTemplateVars, ) if masterErr != nil { return nil, masterErr @@ -153,13 +153,14 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v // get all workers log for workerIndex := int32(0); workerIndex < workersCount; workerIndex++ { workerLog, err := logPlugin.GetTaskLogs(tasklog.Input{ - PodName: name + fmt.Sprintf("-worker-%d", workerIndex), - Namespace: namespace, - PodRFC3339StartTime: RFC3999StartTime, - PodRFC3339FinishTime: RFC3999FinishTime, - PodUnixStartTime: startTime, - PodUnixFinishTime: finishTime, - }, extraLogTemplateVars) + PodName: name + fmt.Sprintf("-worker-%d", workerIndex), + Namespace: namespace, + PodRFC3339StartTime: RFC3999StartTime, + PodRFC3339FinishTime: RFC3999FinishTime, + PodUnixStartTime: startTime, + PodUnixFinishTime: finishTime, + TaskExecutionIdentifier: taskExecId, + }) if err != nil { return nil, err } @@ -173,9 +174,10 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v // get all parameter servers logs for psReplicaIndex := int32(0); psReplicaIndex < psReplicasCount; psReplicaIndex++ { psReplicaLog, err := logPlugin.GetTaskLogs(tasklog.Input{ - PodName: name + fmt.Sprintf("-psReplica-%d", psReplicaIndex), - Namespace: namespace, - }, extraLogTemplateVars) + PodName: name + fmt.Sprintf("-psReplica-%d", psReplicaIndex), + Namespace: namespace, + TaskExecutionIdentifier: taskExecId, + }) if err != nil { return nil, err } @@ -184,9 +186,10 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v // get chief worker log, and the max number of chief worker is 1 if chiefReplicasCount != 0 { chiefReplicaLog, err := logPlugin.GetTaskLogs(tasklog.Input{ - PodName: name + fmt.Sprintf("-chiefReplica-%d", 0), - Namespace: namespace, - }, extraLogTemplateVars) + PodName: name + fmt.Sprintf("-chiefReplica-%d", 0), + Namespace: namespace, + TaskExecutionIdentifier: taskExecId, + }) if err != nil { return nil, err } diff --git a/go/tasks/plugins/k8s/pod/plugin.go b/go/tasks/plugins/k8s/pod/plugin.go index 248317384..45b06def1 100644 --- a/go/tasks/plugins/k8s/pod/plugin.go +++ b/go/tasks/plugins/k8s/pod/plugin.go @@ -143,8 +143,7 @@ func (p plugin) GetTaskPhase(ctx context.Context, pluginContext k8s.PluginContex return pluginsCore.PhaseInfoUndefined, err } - extraLogTemplateVars := tasklog.GetTaskExecutionIdentifierTemplateVars(pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()) - return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)", extraLogTemplateVars) + return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)") } func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) (pluginsCore.PhaseInfo, error) { @@ -168,7 +167,7 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin } if pod.Status.Phase != v1.PodPending && pod.Status.Phase != v1.PodUnknown { - taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, pod, 0, logSuffix, extraLogTemplateVars...) + taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID(), pod, 0, logSuffix, extraLogTemplateVars...) if err != nil { return pluginsCore.PhaseInfoUndefined, err } diff --git a/go/tasks/plugins/k8s/spark/spark.go b/go/tasks/plugins/k8s/spark/spark.go index 0d735205f..6d664d92e 100755 --- a/go/tasks/plugins/k8s/spark/spark.go +++ b/go/tasks/plugins/k8s/spark/spark.go @@ -311,10 +311,9 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl state == sparkOp.PendingSubmissionState || state == sparkOp.SubmittedState - extraLogTemplateVars := tasklog.GetTaskExecutionIdentifierTemplateVars(pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID()) - sparkConfig := GetSparkConfig() taskLogs := make([]*core.TaskLog, 0, 3) + taskExecId := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() if !isQueued { if sj.Status.DriverInfo.PodName != "" { @@ -325,10 +324,11 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl if p != nil { o, err := p.GetTaskLogs(tasklog.Input{ - PodName: sj.Status.DriverInfo.PodName, - Namespace: sj.Namespace, - LogName: "(Driver Logs)", - }, extraLogTemplateVars) + PodName: sj.Status.DriverInfo.PodName, + Namespace: sj.Namespace, + LogName: "(Driver Logs)", + TaskExecutionIdentifier: taskExecId, + }) if err != nil { return nil, err @@ -345,10 +345,11 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl if p != nil { o, err := p.GetTaskLogs(tasklog.Input{ - PodName: sj.Status.DriverInfo.PodName, - Namespace: sj.Namespace, - LogName: "(User Logs)", - }, extraLogTemplateVars) + PodName: sj.Status.DriverInfo.PodName, + Namespace: sj.Namespace, + LogName: "(User Logs)", + TaskExecutionIdentifier: taskExecId, + }) if err != nil { return nil, err @@ -364,10 +365,11 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl if p != nil { o, err := p.GetTaskLogs(tasklog.Input{ - PodName: sj.Name, - Namespace: sj.Namespace, - LogName: "(System Logs)", - }, extraLogTemplateVars) + PodName: sj.Name, + Namespace: sj.Namespace, + LogName: "(System Logs)", + TaskExecutionIdentifier: taskExecId, + }) if err != nil { return nil, err @@ -384,10 +386,11 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl if p != nil { o, err := p.GetTaskLogs(tasklog.Input{ - PodName: sj.Name, - Namespace: sj.Namespace, - LogName: "(Spark-Submit/All User Logs)", - }, extraLogTemplateVars) + PodName: sj.Name, + Namespace: sj.Namespace, + LogName: "(Spark-Submit/All User Logs)", + TaskExecutionIdentifier: taskExecId, + }) if err != nil { return nil, err From d2c571419808367bea222dba87762038eeaa34bb Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 13 Jul 2023 10:16:32 -0700 Subject: [PATCH 11/22] use pointer --- go/tasks/logs/logging_utils.go | 2 +- go/tasks/logs/logging_utils_test.go | 2 +- go/tasks/pluginmachinery/tasklog/plugin.go | 2 +- go/tasks/pluginmachinery/tasklog/template.go | 24 +++++++++---------- go/tasks/plugins/k8s/dask/dask.go | 3 ++- .../k8s/kfoperators/common/common_operator.go | 8 +++---- go/tasks/plugins/k8s/pod/plugin.go | 2 +- go/tasks/plugins/k8s/spark/spark.go | 8 +++---- 8 files changed, 26 insertions(+), 25 deletions(-) diff --git a/go/tasks/logs/logging_utils.go b/go/tasks/logs/logging_utils.go index aabfb662c..87482aa0a 100755 --- a/go/tasks/logs/logging_utils.go +++ b/go/tasks/logs/logging_utils.go @@ -18,7 +18,7 @@ type logPlugin struct { } // Internal -func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecId core.TaskExecutionIdentifier, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) ([]*core.TaskLog, error) { +func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecId *core.TaskExecutionIdentifier, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) ([]*core.TaskLog, error) { if logPlugin == nil { return nil, nil } diff --git a/go/tasks/logs/logging_utils_test.go b/go/tasks/logs/logging_utils_test.go index b9c3315bd..30e4dade0 100755 --- a/go/tasks/logs/logging_utils_test.go +++ b/go/tasks/logs/logging_utils_test.go @@ -14,7 +14,7 @@ import ( const podName = "PodName" -var dummyTaskExecId = core.TaskExecutionIdentifier{ +var dummyTaskExecId = &core.TaskExecutionIdentifier{ TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, Name: "my_name", diff --git a/go/tasks/pluginmachinery/tasklog/plugin.go b/go/tasks/pluginmachinery/tasklog/plugin.go index 77204dc5c..4ae830e33 100644 --- a/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/go/tasks/pluginmachinery/tasklog/plugin.go @@ -18,7 +18,7 @@ type Input struct { PodUnixStartTime int64 PodUnixFinishTime int64 PodUID string - TaskExecutionIdentifier core.TaskExecutionIdentifier + TaskExecutionIdentifier *core.TaskExecutionIdentifier } // Output contains all task logs a plugin generates for a given Input. diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index 90bf92e61..373648aad 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -34,18 +34,18 @@ func (input Input) ToTemplateVars() (TemplateVars, error) { var err error serialized, err := json.Marshal(&struct { - HostName string `json:"hostname,omitempty"` - PodName string `json:"podName,omitempty"` - Namespace string `json:"namespace,omitempty"` - ContainerName string `json:"containerName,omitempty"` - ContainerID string `json:"containerId,omitempty"` - LogName string `json:"logName,omitempty"` - PodRFC3339StartTime string `json:"podRFC3339StartTime,omitempty"` - PodRFC3339FinishTime string `json:"podRFC3339FinishTime,omitempty"` - PodUnixStartTime string `json:"podUnixStartTime,omitempty"` - PodUnixFinishTime string `json:"podUnixFinishTime,omitempty"` - PodUID string `json:"podUID,omitempty"` - TaskExecutionIdentifier core.TaskExecutionIdentifier `json:"taskExecution"` + HostName string `json:"hostname,omitempty"` + PodName string `json:"podName,omitempty"` + Namespace string `json:"namespace,omitempty"` + ContainerName string `json:"containerName,omitempty"` + ContainerID string `json:"containerId,omitempty"` + LogName string `json:"logName,omitempty"` + PodRFC3339StartTime string `json:"podRFC3339StartTime,omitempty"` + PodRFC3339FinishTime string `json:"podRFC3339FinishTime,omitempty"` + PodUnixStartTime string `json:"podUnixStartTime,omitempty"` + PodUnixFinishTime string `json:"podUnixFinishTime,omitempty"` + PodUID string `json:"podUID,omitempty"` + TaskExecutionIdentifier *core.TaskExecutionIdentifier `json:"taskExecution,omitempty"` }{ input.HostName, input.PodName, diff --git a/go/tasks/plugins/k8s/dask/dask.go b/go/tasks/plugins/k8s/dask/dask.go index bc6c7f663..2d7e9027c 100755 --- a/go/tasks/plugins/k8s/dask/dask.go +++ b/go/tasks/plugins/k8s/dask/dask.go @@ -326,12 +326,13 @@ func (p daskResourceHandler) GetTaskPhase(ctx context.Context, pluginContext k8s status == daskAPI.DaskJobClusterCreated if !isQueued { + taskExecId := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() o, err := logPlugin.GetTaskLogs( tasklog.Input{ Namespace: job.ObjectMeta.Namespace, PodName: job.Status.JobRunnerPodName, LogName: "(User logs)", - TaskExecutionIdentifier: pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID(), + TaskExecutionIdentifier: &taskExecId, }, ) if err != nil { diff --git a/go/tasks/plugins/k8s/kfoperators/common/common_operator.go b/go/tasks/plugins/k8s/kfoperators/common/common_operator.go index aaf68577a..ce9caa545 100644 --- a/go/tasks/plugins/k8s/kfoperators/common/common_operator.go +++ b/go/tasks/plugins/k8s/kfoperators/common/common_operator.go @@ -141,7 +141,7 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v PodRFC3339FinishTime: RFC3999FinishTime, PodUnixStartTime: startTime, PodUnixFinishTime: finishTime, - TaskExecutionIdentifier: taskExecId, + TaskExecutionIdentifier: &taskExecId, }, ) if masterErr != nil { @@ -159,7 +159,7 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v PodRFC3339FinishTime: RFC3999FinishTime, PodUnixStartTime: startTime, PodUnixFinishTime: finishTime, - TaskExecutionIdentifier: taskExecId, + TaskExecutionIdentifier: &taskExecId, }) if err != nil { return nil, err @@ -176,7 +176,7 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v psReplicaLog, err := logPlugin.GetTaskLogs(tasklog.Input{ PodName: name + fmt.Sprintf("-psReplica-%d", psReplicaIndex), Namespace: namespace, - TaskExecutionIdentifier: taskExecId, + TaskExecutionIdentifier: &taskExecId, }) if err != nil { return nil, err @@ -188,7 +188,7 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v chiefReplicaLog, err := logPlugin.GetTaskLogs(tasklog.Input{ PodName: name + fmt.Sprintf("-chiefReplica-%d", 0), Namespace: namespace, - TaskExecutionIdentifier: taskExecId, + TaskExecutionIdentifier: &taskExecId, }) if err != nil { return nil, err diff --git a/go/tasks/plugins/k8s/pod/plugin.go b/go/tasks/plugins/k8s/pod/plugin.go index 45b06def1..bed8d1e6c 100644 --- a/go/tasks/plugins/k8s/pod/plugin.go +++ b/go/tasks/plugins/k8s/pod/plugin.go @@ -167,7 +167,7 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin } if pod.Status.Phase != v1.PodPending && pod.Status.Phase != v1.PodUnknown { - taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID(), pod, 0, logSuffix, extraLogTemplateVars...) + taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, &pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID(), pod, 0, logSuffix, extraLogTemplateVars...) if err != nil { return pluginsCore.PhaseInfoUndefined, err } diff --git a/go/tasks/plugins/k8s/spark/spark.go b/go/tasks/plugins/k8s/spark/spark.go index 6d664d92e..628979ee8 100755 --- a/go/tasks/plugins/k8s/spark/spark.go +++ b/go/tasks/plugins/k8s/spark/spark.go @@ -327,7 +327,7 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl PodName: sj.Status.DriverInfo.PodName, Namespace: sj.Namespace, LogName: "(Driver Logs)", - TaskExecutionIdentifier: taskExecId, + TaskExecutionIdentifier: &taskExecId, }) if err != nil { @@ -348,7 +348,7 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl PodName: sj.Status.DriverInfo.PodName, Namespace: sj.Namespace, LogName: "(User Logs)", - TaskExecutionIdentifier: taskExecId, + TaskExecutionIdentifier: &taskExecId, }) if err != nil { @@ -368,7 +368,7 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl PodName: sj.Name, Namespace: sj.Namespace, LogName: "(System Logs)", - TaskExecutionIdentifier: taskExecId, + TaskExecutionIdentifier: &taskExecId, }) if err != nil { @@ -389,7 +389,7 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl PodName: sj.Name, Namespace: sj.Namespace, LogName: "(Spark-Submit/All User Logs)", - TaskExecutionIdentifier: taskExecId, + TaskExecutionIdentifier: &taskExecId, }) if err != nil { From c486fafa3af9dd9ce9c6c323d1b3a338b87daecb Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 13 Jul 2023 10:48:34 -0700 Subject: [PATCH 12/22] tests Signed-off-by: Jeev B --- go/tasks/logs/logging_utils_test.go | 25 +++++-- .../pluginmachinery/tasklog/template_test.go | 71 +++++++++++++++++-- .../array/k8s/subtask_exec_context_test.go | 12 ++++ .../common/common_operator_test.go | 12 ++-- go/tasks/plugins/k8s/pod/plugin.go | 3 +- 5 files changed, 104 insertions(+), 19 deletions(-) diff --git a/go/tasks/logs/logging_utils_test.go b/go/tasks/logs/logging_utils_test.go index 30e4dade0..4f6670adf 100755 --- a/go/tasks/logs/logging_utils_test.go +++ b/go/tasks/logs/logging_utils_test.go @@ -17,16 +17,17 @@ const podName = "PodName" var dummyTaskExecId = &core.TaskExecutionIdentifier{ TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, - Name: "my_name", - Project: "my_project", - Domain: "my_domain", + Name: "my-name", + Project: "my-project", + Domain: "my-domain", Version: "1", }, NodeExecutionId: &core.NodeExecutionIdentifier{ + NodeId: "n0", ExecutionId: &core.WorkflowExecutionIdentifier{ - Name: "my_name", - Project: "my_project", - Domain: "my_domain", + Name: "my-name", + Project: "my-project", + Domain: "my-domain", }, }, } @@ -318,6 +319,13 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { }, MessageFormat: core.TaskLog_JSON, }, + { + DisplayName: "Internal", + TemplateURIs: []string{ + "https://my-log-server/{{ .taskExecution.node_execution_id.execution_id.project }}/{{ .taskExecution.node_execution_id.execution_id.domain }}/{{ .taskExecution.node_execution_id.execution_id.name }}/{{ .taskExecution.node_execution_id.node_id }}", + }, + MessageFormat: core.TaskLog_JSON, + }, }, }, []*core.TaskLog{ { @@ -325,5 +333,10 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { MessageFormat: core.TaskLog_JSON, Name: "StackDriver my-Suffix", }, + { + Uri: "https://my-log-server/my-project/my-domain/my-name/n0", + MessageFormat: core.TaskLog_JSON, + Name: "Internal my-Suffix", + }, }) } diff --git a/go/tasks/pluginmachinery/tasklog/template_test.go b/go/tasks/pluginmachinery/tasklog/template_test.go index 1b2bce67b..e398dddf4 100644 --- a/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/go/tasks/pluginmachinery/tasklog/template_test.go @@ -159,11 +159,12 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { input Input } tests := []struct { - name string - fields fields - args args - want Output - wantErr bool + name string + fields fields + args args + extraLogTemplateVars []TemplateVars + want Output + wantErr bool }{ { "splunk", @@ -185,6 +186,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { PodUnixFinishTime: 12345, }, }, + []TemplateVars{}, Output{ TaskLogs: []*core.TaskLog{ { @@ -216,6 +218,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { PodUnixFinishTime: 12345, }, }, + []TemplateVars{}, Output{ TaskLogs: []*core.TaskLog{ { @@ -247,6 +250,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { PodUnixFinishTime: 12345, }, }, + []TemplateVars{}, Output{ TaskLogs: []*core.TaskLog{ { @@ -258,6 +262,61 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { }, false, }, + { + "custom-with-task-execution-identifier", + fields{ + templateURI: "https://logs.flyte.corp.net/{{ .taskExecution.node_execution_id.execution_id.project }}/{{ .taskExecution.node_execution_id.execution_id.domain }}/{{ .taskExecution.node_execution_id.execution_id.name }}/{{ .taskExecution.node_execution_id.node_id }}/{{ .taskExecution.extraField }}", + messageFormat: core.TaskLog_JSON, + }, + args{ + input: Input{ + HostName: "my-host", + PodName: "my-pod", + Namespace: "my-namespace", + ContainerName: "my-container", + ContainerID: "ignore", + LogName: "main_logs", + PodRFC3339StartTime: "1970-01-01T01:02:03+01:00", + PodRFC3339FinishTime: "1970-01-01T04:25:45+01:00", + PodUnixStartTime: 123, + PodUnixFinishTime: 12345, + TaskExecutionIdentifier: &core.TaskExecutionIdentifier{ + TaskId: &core.Identifier{ + ResourceType: core.ResourceType_TASK, + Name: "my-name", + Project: "my-project", + Domain: "my-domain", + Version: "1", + }, + NodeExecutionId: &core.NodeExecutionIdentifier{ + NodeId: "n0", + ExecutionId: &core.WorkflowExecutionIdentifier{ + Name: "my-name", + Project: "my-project", + Domain: "my-domain", + }, + }, + }, + }, + }, + []TemplateVars{ + TemplateVars{ + "taskExecution": TemplateVars{ + "extraField": 1, + }, + }, + }, + Output{ + TaskLogs: []*core.TaskLog{ + { + Uri: "https://logs.flyte.corp.net/my-project/my-domain/my-name/n0/1", + MessageFormat: core.TaskLog_JSON, + Name: "main_logs", + }, + }, + }, + false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -265,7 +324,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { templateUris: []string{tt.fields.templateURI}, messageFormat: tt.fields.messageFormat, } - got, err := s.GetTaskLogs(tt.args.input) + got, err := s.GetTaskLogs(tt.args.input, tt.extraLogTemplateVars...) if (err != nil) != tt.wantErr { t.Errorf("NewTaskLog() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context_test.go b/go/tasks/plugins/array/k8s/subtask_exec_context_test.go index 77389514e..a1dbabb45 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context_test.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context_test.go @@ -5,6 +5,7 @@ import ( "fmt" "testing" + "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/tasklog" podPlugin "github.com/flyteorg/flyteplugins/go/tasks/plugins/k8s/pod" "github.com/flyteorg/flytestdlib/storage" @@ -35,4 +36,15 @@ func TestSubTaskExecutionContext(t *testing.T) { assert.Equal(t, podPlugin.ContainerTaskType, subtaskTemplate.Type) assert.Equal(t, storage.DataReference("/prefix/"), stCtx.OutputWriter().GetOutputPrefixPath()) assert.Equal(t, storage.DataReference("/raw_prefix/5/1"), stCtx.OutputWriter().GetRawOutputPrefix()) + assert.Equal(t, + tasklog.TemplateVars{ + "taskExecution": tasklog.TemplateVars{ + "executionIndex": 0, + "parentName": "notfound", + "subtaskRetryAttempt": uint64(1), + "taskRetryAttempt": uint32(0), + }, + }, + stCtx.TaskExecutionMetadata().GetTaskExecutionID().(SubTaskExecutionID).ToTemplateVars(), + ) } diff --git a/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go b/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go index 51d84c824..467abb256 100644 --- a/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go +++ b/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go @@ -328,16 +328,16 @@ func dummyTaskContext() pluginsCore.TaskExecutionContext { tID.OnGetID().Return(core.TaskExecutionIdentifier{ TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, - Name: "my_name", - Project: "my_project", - Domain: "my_domain", + Name: "my-name", + Project: "my-project", + Domain: "my-domain", Version: "1", }, NodeExecutionId: &core.NodeExecutionIdentifier{ ExecutionId: &core.WorkflowExecutionIdentifier{ - Name: "my_name", - Project: "my_project", - Domain: "my_domain", + Name: "my-name", + Project: "my-project", + Domain: "my-domain", }, }, }) diff --git a/go/tasks/plugins/k8s/pod/plugin.go b/go/tasks/plugins/k8s/pod/plugin.go index bed8d1e6c..813ba8fa7 100644 --- a/go/tasks/plugins/k8s/pod/plugin.go +++ b/go/tasks/plugins/k8s/pod/plugin.go @@ -166,8 +166,9 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin ReportedAt: &reportedAt, } + taskExecId := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() if pod.Status.Phase != v1.PodPending && pod.Status.Phase != v1.PodUnknown { - taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, &pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID(), pod, 0, logSuffix, extraLogTemplateVars...) + taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, &taskExecId, pod, 0, logSuffix, extraLogTemplateVars...) if err != nil { return pluginsCore.PhaseInfoUndefined, err } From ecaeed1d0b896a2142da942dbba96c0c4aeaf97c Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 13 Jul 2023 10:51:58 -0700 Subject: [PATCH 13/22] fix linting Signed-off-by: Jeev B --- go/tasks/logs/logging_utils.go | 2 +- go/tasks/logs/logging_utils_test.go | 20 +++++++++---------- go/tasks/plugins/k8s/dask/dask.go | 4 ++-- .../k8s/kfoperators/common/common_operator.go | 10 +++++----- go/tasks/plugins/k8s/spark/spark.go | 10 +++++----- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/go/tasks/logs/logging_utils.go b/go/tasks/logs/logging_utils.go index 87482aa0a..aefce19da 100755 --- a/go/tasks/logs/logging_utils.go +++ b/go/tasks/logs/logging_utils.go @@ -18,7 +18,7 @@ type logPlugin struct { } // Internal -func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecId *core.TaskExecutionIdentifier, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) ([]*core.TaskLog, error) { +func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID *core.TaskExecutionIdentifier, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) ([]*core.TaskLog, error) { if logPlugin == nil { return nil, nil } diff --git a/go/tasks/logs/logging_utils_test.go b/go/tasks/logs/logging_utils_test.go index 4f6670adf..6ad4265fd 100755 --- a/go/tasks/logs/logging_utils_test.go +++ b/go/tasks/logs/logging_utils_test.go @@ -14,7 +14,7 @@ import ( const podName = "PodName" -var dummyTaskExecId = &core.TaskExecutionIdentifier{ +var dummyTaskExecID = &core.TaskExecutionIdentifier{ TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, Name: "my-name", @@ -35,7 +35,7 @@ var dummyTaskExecId = &core.TaskExecutionIdentifier{ func TestGetLogsForContainerInPod_NoPlugins(t *testing.T) { logPlugin, err := InitializeLogPlugins(&LogConfig{}) assert.NoError(t, err) - l, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, nil, 0, " Suffix") + l, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, nil, 0, " Suffix") assert.NoError(t, err) assert.Nil(t, l) } @@ -47,7 +47,7 @@ func TestGetLogsForContainerInPod_NoLogs(t *testing.T) { CloudwatchLogGroup: "/kubernetes/flyte-production", }) assert.NoError(t, err) - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, nil, 0, " Suffix") + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, nil, 0, " Suffix") assert.NoError(t, err) assert.Nil(t, p) } @@ -78,7 +78,7 @@ func TestGetLogsForContainerInPod_BadIndex(t *testing.T) { } pod.Name = podName - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 1, " Suffix") + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 1, " Suffix") assert.NoError(t, err) assert.Nil(t, p) } @@ -103,7 +103,7 @@ func TestGetLogsForContainerInPod_MissingStatus(t *testing.T) { } pod.Name = podName - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 1, " Suffix") + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 1, " Suffix") assert.NoError(t, err) assert.Nil(t, p) } @@ -133,7 +133,7 @@ func TestGetLogsForContainerInPod_Cloudwatch(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix") assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -163,7 +163,7 @@ func TestGetLogsForContainerInPod_K8s(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix") assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -196,7 +196,7 @@ func TestGetLogsForContainerInPod_All(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix") assert.Nil(t, err) assert.Len(t, logs, 2) } @@ -227,7 +227,7 @@ func TestGetLogsForContainerInPod_Stackdriver(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix") assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -301,7 +301,7 @@ func assertTestSucceeded(tb testing.TB, config *LogConfig, expectedTaskLogs []*c }, } - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecId, pod, 0, " my-Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " my-Suffix") assert.Nil(tb, err) assert.Len(tb, logs, len(expectedTaskLogs)) if diff := deep.Equal(logs, expectedTaskLogs); len(diff) > 0 { diff --git a/go/tasks/plugins/k8s/dask/dask.go b/go/tasks/plugins/k8s/dask/dask.go index 2d7e9027c..f12423a64 100755 --- a/go/tasks/plugins/k8s/dask/dask.go +++ b/go/tasks/plugins/k8s/dask/dask.go @@ -326,13 +326,13 @@ func (p daskResourceHandler) GetTaskPhase(ctx context.Context, pluginContext k8s status == daskAPI.DaskJobClusterCreated if !isQueued { - taskExecId := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() + taskExecID := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() o, err := logPlugin.GetTaskLogs( tasklog.Input{ Namespace: job.ObjectMeta.Namespace, PodName: job.Status.JobRunnerPodName, LogName: "(User logs)", - TaskExecutionIdentifier: &taskExecId, + TaskExecutionIdentifier: &taskExecID, }, ) if err != nil { diff --git a/go/tasks/plugins/k8s/kfoperators/common/common_operator.go b/go/tasks/plugins/k8s/kfoperators/common/common_operator.go index ce9caa545..ac3b7f455 100644 --- a/go/tasks/plugins/k8s/kfoperators/common/common_operator.go +++ b/go/tasks/plugins/k8s/kfoperators/common/common_operator.go @@ -112,7 +112,7 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v namespace := objectMeta.Namespace taskLogs := make([]*core.TaskLog, 0, 10) - taskExecId := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() + taskExecID := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() logPlugin, err := logs.InitializeLogPlugins(logs.GetLogConfig()) @@ -141,7 +141,7 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v PodRFC3339FinishTime: RFC3999FinishTime, PodUnixStartTime: startTime, PodUnixFinishTime: finishTime, - TaskExecutionIdentifier: &taskExecId, + TaskExecutionIdentifier: &taskExecID, }, ) if masterErr != nil { @@ -159,7 +159,7 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v PodRFC3339FinishTime: RFC3999FinishTime, PodUnixStartTime: startTime, PodUnixFinishTime: finishTime, - TaskExecutionIdentifier: &taskExecId, + TaskExecutionIdentifier: &taskExecID, }) if err != nil { return nil, err @@ -176,7 +176,7 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v psReplicaLog, err := logPlugin.GetTaskLogs(tasklog.Input{ PodName: name + fmt.Sprintf("-psReplica-%d", psReplicaIndex), Namespace: namespace, - TaskExecutionIdentifier: &taskExecId, + TaskExecutionIdentifier: &taskExecID, }) if err != nil { return nil, err @@ -188,7 +188,7 @@ func GetLogs(pluginContext k8s.PluginContext, taskType string, objectMeta meta_v chiefReplicaLog, err := logPlugin.GetTaskLogs(tasklog.Input{ PodName: name + fmt.Sprintf("-chiefReplica-%d", 0), Namespace: namespace, - TaskExecutionIdentifier: &taskExecId, + TaskExecutionIdentifier: &taskExecID, }) if err != nil { return nil, err diff --git a/go/tasks/plugins/k8s/spark/spark.go b/go/tasks/plugins/k8s/spark/spark.go index 628979ee8..f0d328b2c 100755 --- a/go/tasks/plugins/k8s/spark/spark.go +++ b/go/tasks/plugins/k8s/spark/spark.go @@ -313,7 +313,7 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl sparkConfig := GetSparkConfig() taskLogs := make([]*core.TaskLog, 0, 3) - taskExecId := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() + taskExecID := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() if !isQueued { if sj.Status.DriverInfo.PodName != "" { @@ -327,7 +327,7 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl PodName: sj.Status.DriverInfo.PodName, Namespace: sj.Namespace, LogName: "(Driver Logs)", - TaskExecutionIdentifier: &taskExecId, + TaskExecutionIdentifier: &taskExecID, }) if err != nil { @@ -348,7 +348,7 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl PodName: sj.Status.DriverInfo.PodName, Namespace: sj.Namespace, LogName: "(User Logs)", - TaskExecutionIdentifier: &taskExecId, + TaskExecutionIdentifier: &taskExecID, }) if err != nil { @@ -368,7 +368,7 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl PodName: sj.Name, Namespace: sj.Namespace, LogName: "(System Logs)", - TaskExecutionIdentifier: &taskExecId, + TaskExecutionIdentifier: &taskExecID, }) if err != nil { @@ -389,7 +389,7 @@ func getEventInfoForSpark(pluginContext k8s.PluginContext, sj *sparkOp.SparkAppl PodName: sj.Name, Namespace: sj.Namespace, LogName: "(Spark-Submit/All User Logs)", - TaskExecutionIdentifier: &taskExecId, + TaskExecutionIdentifier: &taskExecID, }) if err != nil { From 1d91c32b952763385d8d32073e2421063c7221ff Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 13 Jul 2023 11:07:01 -0700 Subject: [PATCH 14/22] cleanups --- go/tasks/logs/logging_utils.go | 2 +- go/tasks/logs/logging_utils_test.go | 9 +++++---- go/tasks/pluginmachinery/tasklog/template_test.go | 14 +++++++++----- .../k8s/kfoperators/common/common_operator_test.go | 5 +++-- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/go/tasks/logs/logging_utils.go b/go/tasks/logs/logging_utils.go index aefce19da..6ab47e93b 100755 --- a/go/tasks/logs/logging_utils.go +++ b/go/tasks/logs/logging_utils.go @@ -53,7 +53,7 @@ func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, tas PodRFC3339FinishTime: time.Unix(finishTime, 0).Format(time.RFC3339), PodUnixStartTime: startTime, PodUnixFinishTime: finishTime, - TaskExecutionIdentifier: taskExecId, + TaskExecutionIdentifier: taskExecID, }, extraLogTemplateVars..., ) diff --git a/go/tasks/logs/logging_utils_test.go b/go/tasks/logs/logging_utils_test.go index 6ad4265fd..eaefcc813 100755 --- a/go/tasks/logs/logging_utils_test.go +++ b/go/tasks/logs/logging_utils_test.go @@ -17,7 +17,7 @@ const podName = "PodName" var dummyTaskExecID = &core.TaskExecutionIdentifier{ TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, - Name: "my-name", + Name: "my-task-name", Project: "my-project", Domain: "my-domain", Version: "1", @@ -25,11 +25,12 @@ var dummyTaskExecID = &core.TaskExecutionIdentifier{ NodeExecutionId: &core.NodeExecutionIdentifier{ NodeId: "n0", ExecutionId: &core.WorkflowExecutionIdentifier{ - Name: "my-name", + Name: "my-execution-name", Project: "my-project", Domain: "my-domain", }, }, + RetryAttempt: 1, } func TestGetLogsForContainerInPod_NoPlugins(t *testing.T) { @@ -322,7 +323,7 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { { DisplayName: "Internal", TemplateURIs: []string{ - "https://my-log-server/{{ .taskExecution.node_execution_id.execution_id.project }}/{{ .taskExecution.node_execution_id.execution_id.domain }}/{{ .taskExecution.node_execution_id.execution_id.name }}/{{ .taskExecution.node_execution_id.node_id }}", + "https://flyte.corp.net/console/projects/{{ .taskExecution.node_execution_id.execution_id.project }}/domains/{{ .taskExecution.node_execution_id.execution_id.domain }}/executions/{{ .taskExecution.node_execution_id.execution_id.name }}/nodeId/{{ .taskExecution.node_execution_id.node_id }}/taskId/{{ .taskExecution.task_id.name }}/attempt/{{ .taskExecution.retry_attempt }}/view/logs", }, MessageFormat: core.TaskLog_JSON, }, @@ -334,7 +335,7 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { Name: "StackDriver my-Suffix", }, { - Uri: "https://my-log-server/my-project/my-domain/my-name/n0", + Uri: "https://flyte.corp.net/console/projects/my-project/domains/my-domain/executions/my-execution-name/nodeId/n0/taskId/my-task-name/attempt/1/view/logs", MessageFormat: core.TaskLog_JSON, Name: "Internal my-Suffix", }, diff --git a/go/tasks/pluginmachinery/tasklog/template_test.go b/go/tasks/pluginmachinery/tasklog/template_test.go index e398dddf4..2aad8d410 100644 --- a/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/go/tasks/pluginmachinery/tasklog/template_test.go @@ -265,7 +265,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { { "custom-with-task-execution-identifier", fields{ - templateURI: "https://logs.flyte.corp.net/{{ .taskExecution.node_execution_id.execution_id.project }}/{{ .taskExecution.node_execution_id.execution_id.domain }}/{{ .taskExecution.node_execution_id.execution_id.name }}/{{ .taskExecution.node_execution_id.node_id }}/{{ .taskExecution.extraField }}", + templateURI: "https://flyte.corp.net/console/projects/{{ .taskExecution.node_execution_id.execution_id.project }}/domains/{{ .taskExecution.node_execution_id.execution_id.domain }}/executions/{{ .taskExecution.node_execution_id.execution_id.name }}/nodeId/{{ .taskExecution.node_execution_id.node_id }}/taskId/{{ .taskExecution.task_id.name }}/attempt/{{ .taskExecution.taskRetryAttempt }}/mappedIndex/{{ .taskExecution.executionIndex }}/mappedAttempt/{{ .taskExecution.subtaskRetryAttempt }}/view/logs", messageFormat: core.TaskLog_JSON, }, args{ @@ -283,7 +283,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { TaskExecutionIdentifier: &core.TaskExecutionIdentifier{ TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, - Name: "my-name", + Name: "my-task-name", Project: "my-project", Domain: "my-domain", Version: "1", @@ -291,25 +291,29 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { NodeExecutionId: &core.NodeExecutionIdentifier{ NodeId: "n0", ExecutionId: &core.WorkflowExecutionIdentifier{ - Name: "my-name", + Name: "my-execution-name", Project: "my-project", Domain: "my-domain", }, }, + RetryAttempt: 0, }, }, }, []TemplateVars{ TemplateVars{ "taskExecution": TemplateVars{ - "extraField": 1, + "executionIndex": 1, + "parentName": "notfound", + "subtaskRetryAttempt": 1, + "taskRetryAttempt": 0, }, }, }, Output{ TaskLogs: []*core.TaskLog{ { - Uri: "https://logs.flyte.corp.net/my-project/my-domain/my-name/n0/1", + Uri: "https://flyte.corp.net/console/projects/my-project/domains/my-domain/executions/my-execution-name/nodeId/n0/taskId/my-task-name/attempt/0/mappedIndex/1/mappedAttempt/1/view/logs", MessageFormat: core.TaskLog_JSON, Name: "main_logs", }, diff --git a/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go b/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go index 467abb256..d0b02ff9b 100644 --- a/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go +++ b/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go @@ -328,18 +328,19 @@ func dummyTaskContext() pluginsCore.TaskExecutionContext { tID.OnGetID().Return(core.TaskExecutionIdentifier{ TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, - Name: "my-name", + Name: "my-task-name", Project: "my-project", Domain: "my-domain", Version: "1", }, NodeExecutionId: &core.NodeExecutionIdentifier{ ExecutionId: &core.WorkflowExecutionIdentifier{ - Name: "my-name", + Name: "my-execution-name", Project: "my-project", Domain: "my-domain", }, }, + RetryAttempt: 0, }) taskExecutionMetadata := &mocks.TaskExecutionMetadata{} From 6e9ee1fb1440b04428be9e4314bb80ef7717486a Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 13 Jul 2023 11:15:41 -0700 Subject: [PATCH 15/22] fix --- go/tasks/plugins/k8s/pod/plugin.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/tasks/plugins/k8s/pod/plugin.go b/go/tasks/plugins/k8s/pod/plugin.go index 813ba8fa7..5e6ae6276 100644 --- a/go/tasks/plugins/k8s/pod/plugin.go +++ b/go/tasks/plugins/k8s/pod/plugin.go @@ -166,9 +166,9 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin ReportedAt: &reportedAt, } - taskExecId := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() + taskExecID := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() if pod.Status.Phase != v1.PodPending && pod.Status.Phase != v1.PodUnknown { - taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, &taskExecId, pod, 0, logSuffix, extraLogTemplateVars...) + taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, &taskExecID, pod, 0, logSuffix, extraLogTemplateVars...) if err != nil { return pluginsCore.PhaseInfoUndefined, err } From cdf5147d33ddd5f34b1455112fa54a4556428772 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 13 Jul 2023 16:56:19 -0700 Subject: [PATCH 16/22] revert to using regex replacements for performance --- go/tasks/logs/logging_utils.go | 13 +- go/tasks/logs/logging_utils_test.go | 2 +- go/tasks/pluginmachinery/tasklog/plugin.go | 16 +- go/tasks/pluginmachinery/tasklog/template.go | 236 ++++++++++++------ .../pluginmachinery/tasklog/template_test.go | 31 +-- go/tasks/plugins/array/k8s/subtask.go | 2 +- .../plugins/array/k8s/subtask_exec_context.go | 30 ++- .../array/k8s/subtask_exec_context_test.go | 10 +- go/tasks/plugins/k8s/pod/plugin.go | 2 +- 9 files changed, 219 insertions(+), 123 deletions(-) diff --git a/go/tasks/logs/logging_utils.go b/go/tasks/logs/logging_utils.go index 6ab47e93b..07c9656a7 100755 --- a/go/tasks/logs/logging_utils.go +++ b/go/tasks/logs/logging_utils.go @@ -18,7 +18,7 @@ type logPlugin struct { } // Internal -func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID *core.TaskExecutionIdentifier, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) ([]*core.TaskLog, error) { +func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID *core.TaskExecutionIdentifier, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars ...tasklog.TemplateVar) ([]*core.TaskLog, error) { if logPlugin == nil { return nil, nil } @@ -54,8 +54,8 @@ func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, tas PodUnixStartTime: startTime, PodUnixFinishTime: finishTime, TaskExecutionIdentifier: taskExecID, + ExtraTemplateVars: extraLogTemplateVars, }, - extraLogTemplateVars..., ) if err != nil { @@ -69,18 +69,13 @@ type taskLogPluginWrapper struct { logPlugins []logPlugin } -func (t taskLogPluginWrapper) GetTaskLogs(input tasklog.Input, extraTemplateVars ...tasklog.TemplateVars) (logOutput tasklog.Output, err error) { +func (t taskLogPluginWrapper) GetTaskLogs(input tasklog.Input) (logOutput tasklog.Output, err error) { logs := make([]*core.TaskLog, 0, len(t.logPlugins)) suffix := input.LogName - mergedTemplateVars := make(tasklog.TemplateVars) - if err := mergedTemplateVars.Merge(extraTemplateVars...); err != nil { - return tasklog.Output{}, err - } - for _, plugin := range t.logPlugins { input.LogName = plugin.Name + suffix - o, err := plugin.Plugin.GetTaskLogs(input, mergedTemplateVars) + o, err := plugin.Plugin.GetTaskLogs(input) if err != nil { return tasklog.Output{}, err } diff --git a/go/tasks/logs/logging_utils_test.go b/go/tasks/logs/logging_utils_test.go index eaefcc813..19eaf355d 100755 --- a/go/tasks/logs/logging_utils_test.go +++ b/go/tasks/logs/logging_utils_test.go @@ -323,7 +323,7 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { { DisplayName: "Internal", TemplateURIs: []string{ - "https://flyte.corp.net/console/projects/{{ .taskExecution.node_execution_id.execution_id.project }}/domains/{{ .taskExecution.node_execution_id.execution_id.domain }}/executions/{{ .taskExecution.node_execution_id.execution_id.name }}/nodeId/{{ .taskExecution.node_execution_id.node_id }}/taskId/{{ .taskExecution.task_id.name }}/attempt/{{ .taskExecution.retry_attempt }}/view/logs", + "https://flyte.corp.net/console/projects/{{ .executionProject }}/domains/{{ .executionDomain }}/executions/{{ .executionName }}/nodeId/{{ .nodeID }}/taskId/{{ .taskID }}/attempt/{{ .taskRetryAttempt }}/view/logs", }, MessageFormat: core.TaskLog_JSON, }, diff --git a/go/tasks/pluginmachinery/tasklog/plugin.go b/go/tasks/pluginmachinery/tasklog/plugin.go index 4ae830e33..98504943b 100644 --- a/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/go/tasks/pluginmachinery/tasklog/plugin.go @@ -1,8 +1,17 @@ package tasklog -import "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" +import ( + "regexp" -type TemplateVars map[string]interface{} + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" +) + +type TemplateVar struct { + Regex *regexp.Regexp + Value string +} + +type TemplateVars []TemplateVar // Input contains all available information about task's execution that a log plugin can use to construct task's // log links. @@ -19,6 +28,7 @@ type Input struct { PodUnixFinishTime int64 PodUID string TaskExecutionIdentifier *core.TaskExecutionIdentifier + ExtraTemplateVars TemplateVars } // Output contains all task logs a plugin generates for a given Input. @@ -29,5 +39,5 @@ type Output struct { // Plugin represents an interface for task log plugins to implement to plug generated task log links into task events. type Plugin interface { // Generates a TaskLog object given necessary computation information - GetTaskLogs(i Input, v ...TemplateVars) (logs Output, err error) + GetTaskLogs(i Input) (logs Output, err error) } diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index 373648aad..29907f79b 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -1,28 +1,73 @@ package tasklog import ( - "bytes" - "encoding/json" + "fmt" + "regexp" "strconv" "strings" - "text/template" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" - - "github.com/imdario/mergo" ) -func (t TemplateVars) Merge(others ...TemplateVars) error { - for _, o := range others { - err := mergo.Merge(&t, o) - if err != nil { - return err +func MustCreateRegex(varName string) *regexp.Regexp { + return regexp.MustCompile(fmt.Sprintf(`(?i){{\s*[\.$]%s\s*}}`, varName)) +} + +var defaultRegexes = struct { + PodName *regexp.Regexp + PodUID *regexp.Regexp + Namespace *regexp.Regexp + ContainerName *regexp.Regexp + ContainerID *regexp.Regexp + LogName *regexp.Regexp + Hostname *regexp.Regexp + PodRFC3339StartTime *regexp.Regexp + PodRFC3339FinishTime *regexp.Regexp + PodUnixStartTime *regexp.Regexp + PodUnixFinishTime *regexp.Regexp + TaskID *regexp.Regexp + TaskVersion *regexp.Regexp + TaskProject *regexp.Regexp + TaskDomain *regexp.Regexp + TaskRetryAttempt *regexp.Regexp + NodeID *regexp.Regexp + ExecutionName *regexp.Regexp + ExecutionProject *regexp.Regexp + ExecutionDomain *regexp.Regexp +}{ + MustCreateRegex("podName"), + MustCreateRegex("podUID"), + MustCreateRegex("namespace"), + MustCreateRegex("containerName"), + MustCreateRegex("containerID"), + MustCreateRegex("logName"), + MustCreateRegex("hostname"), + MustCreateRegex("podRFC3339StartTime"), + MustCreateRegex("podRFC3339FinishTime"), + MustCreateRegex("podUnixStartTime"), + MustCreateRegex("podUnixFinishTime"), + MustCreateRegex("taskID"), + MustCreateRegex("taskVersion"), + MustCreateRegex("taskProject"), + MustCreateRegex("taskDomain"), + MustCreateRegex("taskRetryAttempt"), + MustCreateRegex("nodeID"), + MustCreateRegex("executionName"), + MustCreateRegex("executionProject"), + MustCreateRegex("executionDomain"), +} + +func replaceAll(template string, vars TemplateVars) string { + for _, v := range vars { + if len(v.Value) > 0 { + template = v.Regex.ReplaceAllLiteralString(template, v.Value) } } - return nil + return template } -func (input Input) ToTemplateVars() (TemplateVars, error) { +func (input Input) ToTemplateVars() TemplateVars { + // Container IDs are prefixed with docker://, cri-o://, etc. which is stripped by fluentd before pushing to a log // stream. Therefore, we must also strip the prefix. containerID := input.ContainerID stripDelimiter := "://" @@ -30,56 +75,109 @@ func (input Input) ToTemplateVars() (TemplateVars, error) { containerID = split[1] } - var templateVars TemplateVars - var err error - - serialized, err := json.Marshal(&struct { - HostName string `json:"hostname,omitempty"` - PodName string `json:"podName,omitempty"` - Namespace string `json:"namespace,omitempty"` - ContainerName string `json:"containerName,omitempty"` - ContainerID string `json:"containerId,omitempty"` - LogName string `json:"logName,omitempty"` - PodRFC3339StartTime string `json:"podRFC3339StartTime,omitempty"` - PodRFC3339FinishTime string `json:"podRFC3339FinishTime,omitempty"` - PodUnixStartTime string `json:"podUnixStartTime,omitempty"` - PodUnixFinishTime string `json:"podUnixFinishTime,omitempty"` - PodUID string `json:"podUID,omitempty"` - TaskExecutionIdentifier *core.TaskExecutionIdentifier `json:"taskExecution,omitempty"` - }{ - input.HostName, - input.PodName, - input.Namespace, - input.ContainerName, - containerID, - input.LogName, - input.PodRFC3339StartTime, - input.PodRFC3339FinishTime, - strconv.FormatInt(input.PodUnixStartTime, 10), - strconv.FormatInt(input.PodUnixFinishTime, 10), - input.PodUID, - input.TaskExecutionIdentifier, - }) - if err != nil { - return templateVars, err + vars := TemplateVars{ + { + Regex: defaultRegexes.PodName, + Value: input.PodName, + }, + { + Regex: defaultRegexes.PodUID, + Value: input.PodUID, + }, + { + Regex: defaultRegexes.Namespace, + Value: input.Namespace, + }, + { + Regex: defaultRegexes.ContainerName, + Value: input.ContainerName, + }, + { + Regex: defaultRegexes.ContainerID, + Value: containerID, + }, + { + Regex: defaultRegexes.LogName, + Value: input.LogName, + }, + { + Regex: defaultRegexes.Hostname, + Value: input.HostName, + }, + { + Regex: defaultRegexes.PodRFC3339StartTime, + Value: input.PodRFC3339StartTime, + }, + { + Regex: defaultRegexes.PodRFC3339FinishTime, + Value: input.PodRFC3339FinishTime, + }, + { + Regex: defaultRegexes.PodUnixStartTime, + Value: strconv.FormatInt(input.PodUnixStartTime, 10), + }, + { + Regex: defaultRegexes.PodUnixFinishTime, + Value: strconv.FormatInt(input.PodUnixFinishTime, 10), + }, + } + + if input.TaskExecutionIdentifier != nil { + vars = append(vars, TemplateVar{ + Regex: defaultRegexes.TaskRetryAttempt, + Value: strconv.FormatUint(uint64(input.TaskExecutionIdentifier.RetryAttempt), 10), + }) + if input.TaskExecutionIdentifier.TaskId != nil { + vars = append( + vars, + TemplateVar{ + Regex: defaultRegexes.TaskID, + Value: input.TaskExecutionIdentifier.TaskId.Name, + }, + TemplateVar{ + Regex: defaultRegexes.TaskVersion, + Value: input.TaskExecutionIdentifier.TaskId.Version, + }, + TemplateVar{ + Regex: defaultRegexes.TaskProject, + Value: input.TaskExecutionIdentifier.TaskId.Project, + }, + TemplateVar{ + Regex: defaultRegexes.TaskDomain, + Value: input.TaskExecutionIdentifier.TaskId.Domain, + }, + ) + } + if input.TaskExecutionIdentifier.NodeExecutionId != nil { + vars = append(vars, TemplateVar{ + Regex: defaultRegexes.NodeID, + Value: input.TaskExecutionIdentifier.NodeExecutionId.NodeId, + }) + if input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId != nil { + vars = append( + vars, + TemplateVar{ + Regex: defaultRegexes.ExecutionName, + Value: input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Name, + }, + TemplateVar{ + Regex: defaultRegexes.ExecutionProject, + Value: input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Project, + }, + TemplateVar{ + Regex: defaultRegexes.ExecutionDomain, + Value: input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Domain, + }, + ) + } + } } - err = json.Unmarshal(serialized, &templateVars) - return templateVars, err + return append(vars, input.ExtraTemplateVars...) } -// A simple log plugin that supports templates in urls to build the final log link. Supported templates are: -// {{ .podName }}: Gets the pod name as it shows in k8s dashboard, -// {{ .podUID }}: Gets the pod UID, -// {{ .namespace }}: K8s namespace where the pod runs, -// {{ .containerName }}: The container name that generated the log, -// {{ .containerId }}: The container id docker/crio generated at run time, -// {{ .logName }}: A deployment specific name where to expect the logs to be. -// {{ .hostname }}: The hostname where the pod is running and where logs reside. -// {{ .PodRFC3339StartTime }}: The pod creation time in RFC3339 format -// {{ .PodRFC3339FinishTime }}: Don't have a good mechanism for this yet, but approximating with time.Now for now -// {{ .podUnixStartTime }}: The pod creation time (in unix seconds, not millis) -// {{ .podUnixFinishTime }}: Don't have a good mechanism for this yet, but approximating with time.Now for now +// A simple log plugin that supports templates in urls to build the final log link. +// See `defaultRegexes` for supported templates. type TemplateLogPlugin struct { templateUris []string messageFormat core.TaskLog_MessageFormat @@ -106,28 +204,12 @@ func (s TemplateLogPlugin) GetTaskLog(podName, podUID, namespace, containerName, return *o.TaskLogs[0], nil } -func (s TemplateLogPlugin) GetTaskLogs(input Input, extraTemplateVars ...TemplateVars) (Output, error) { - var err error - templateVars, err := input.ToTemplateVars() - if err != nil { - return Output{}, err - } - err = templateVars.Merge(extraTemplateVars...) - if err != nil { - return Output{}, err - } - +func (s TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { + templateVars := input.ToTemplateVars() taskLogs := make([]*core.TaskLog, 0, len(s.templateUris)) for _, templateURI := range s.templateUris { - var buf bytes.Buffer - t := template.Must(template.New("uri").Parse(templateURI)) - err := t.Execute(&buf, templateVars) - if err != nil { - return Output{}, err - } - taskLogs = append(taskLogs, &core.TaskLog{ - Uri: buf.String(), + Uri: replaceAll(templateURI, templateVars), Name: input.LogName, MessageFormat: s.messageFormat, }) diff --git a/go/tasks/pluginmachinery/tasklog/template_test.go b/go/tasks/pluginmachinery/tasklog/template_test.go index 2aad8d410..b923c86f9 100644 --- a/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/go/tasks/pluginmachinery/tasklog/template_test.go @@ -159,12 +159,11 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { input Input } tests := []struct { - name string - fields fields - args args - extraLogTemplateVars []TemplateVars - want Output - wantErr bool + name string + fields fields + args args + want Output + wantErr bool }{ { "splunk", @@ -186,7 +185,6 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { PodUnixFinishTime: 12345, }, }, - []TemplateVars{}, Output{ TaskLogs: []*core.TaskLog{ { @@ -218,7 +216,6 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { PodUnixFinishTime: 12345, }, }, - []TemplateVars{}, Output{ TaskLogs: []*core.TaskLog{ { @@ -250,7 +247,6 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { PodUnixFinishTime: 12345, }, }, - []TemplateVars{}, Output{ TaskLogs: []*core.TaskLog{ { @@ -265,7 +261,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { { "custom-with-task-execution-identifier", fields{ - templateURI: "https://flyte.corp.net/console/projects/{{ .taskExecution.node_execution_id.execution_id.project }}/domains/{{ .taskExecution.node_execution_id.execution_id.domain }}/executions/{{ .taskExecution.node_execution_id.execution_id.name }}/nodeId/{{ .taskExecution.node_execution_id.node_id }}/taskId/{{ .taskExecution.task_id.name }}/attempt/{{ .taskExecution.taskRetryAttempt }}/mappedIndex/{{ .taskExecution.executionIndex }}/mappedAttempt/{{ .taskExecution.subtaskRetryAttempt }}/view/logs", + templateURI: "https://flyte.corp.net/console/projects/{{ .executionProject }}/domains/{{ .executionDomain }}/executions/{{ .executionName }}/nodeId/{{ .nodeID }}/taskId/{{ .taskID }}/attempt/{{ .subtaskParentRetryAttempt }}/mappedIndex/{{ .subtaskExecutionIndex }}/mappedAttempt/{{ .subtaskRetryAttempt }}/view/logs", messageFormat: core.TaskLog_JSON, }, args{ @@ -298,15 +294,10 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { }, RetryAttempt: 0, }, - }, - }, - []TemplateVars{ - TemplateVars{ - "taskExecution": TemplateVars{ - "executionIndex": 1, - "parentName": "notfound", - "subtaskRetryAttempt": 1, - "taskRetryAttempt": 0, + ExtraTemplateVars: TemplateVars{ + {MustCreateRegex("subtaskExecutionIndex"), "1"}, + {MustCreateRegex("subtaskRetryAttempt"), "1"}, + {MustCreateRegex("subtaskParentRetryAttempt"), "0"}, }, }, }, @@ -328,7 +319,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { templateUris: []string{tt.fields.templateURI}, messageFormat: tt.fields.messageFormat, } - got, err := s.GetTaskLogs(tt.args.input, tt.extraLogTemplateVars...) + got, err := s.GetTaskLogs(tt.args.input) if (err != nil) != tt.wantErr { t.Errorf("NewTaskLog() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/go/tasks/plugins/array/k8s/subtask.go b/go/tasks/plugins/array/k8s/subtask.go index f24713fac..38cd12b70 100644 --- a/go/tasks/plugins/array/k8s/subtask.go +++ b/go/tasks/plugins/array/k8s/subtask.go @@ -304,7 +304,7 @@ func getSubtaskPhaseInfo(ctx context.Context, stCtx SubTaskExecutionContext, cfg } stID, _ := stCtx.TaskExecutionMetadata().GetTaskExecutionID().(SubTaskExecutionID) - phaseInfo, err := podPlugin.DefaultPodPlugin.GetTaskPhaseWithLogs(ctx, stCtx, pod, logPlugin, stID.GetLogSuffix(), stID.ToTemplateVars()) + phaseInfo, err := podPlugin.DefaultPodPlugin.GetTaskPhaseWithLogs(ctx, stCtx, pod, logPlugin, stID.GetLogSuffix(), stID.ToTemplateVars()...) if err != nil { logger.Warnf(ctx, "failed to check status of resource in plugin [%s], with error: %s", executorName, err.Error()) return pluginsCore.PhaseInfoUndefined, err diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context.go b/go/tasks/plugins/array/k8s/subtask_exec_context.go index 0d4fd5ba5..a34aace50 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context.go @@ -3,6 +3,7 @@ package k8s import ( "context" "fmt" + "regexp" "strconv" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" @@ -176,13 +177,32 @@ func (s SubTaskExecutionID) GetLogSuffix() string { return fmt.Sprintf(" #%d-%d-%d", s.taskRetryAttempt, s.executionIndex, s.subtaskRetryAttempt) } +var logTemplateRegexes = struct { + ExecutionIndex *regexp.Regexp + ParentName *regexp.Regexp + RetryAttempt *regexp.Regexp + ParentRetryAttempt *regexp.Regexp +}{ + tasklog.MustCreateRegex("subtaskExecutionIndex"), + tasklog.MustCreateRegex("subtaskParentName"), + tasklog.MustCreateRegex("subtaskRetryAttempt"), + tasklog.MustCreateRegex("subtaskParentRetryAttempt"), +} + func (s SubTaskExecutionID) ToTemplateVars() tasklog.TemplateVars { return tasklog.TemplateVars{ - "taskExecution": tasklog.TemplateVars{ - "executionIndex": s.executionIndex, - "parentName": s.parentName, - "subtaskRetryAttempt": s.subtaskRetryAttempt, - "taskRetryAttempt": s.taskRetryAttempt, + {logTemplateRegexes.ParentName, s.parentName}, + { + logTemplateRegexes.ExecutionIndex, + strconv.FormatUint(uint64(s.executionIndex), 10), + }, + { + logTemplateRegexes.RetryAttempt, + strconv.FormatUint(uint64(s.subtaskRetryAttempt), 10), + }, + { + logTemplateRegexes.ParentRetryAttempt, + strconv.FormatUint(uint64(s.taskRetryAttempt), 10), }, } } diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context_test.go b/go/tasks/plugins/array/k8s/subtask_exec_context_test.go index a1dbabb45..420760e26 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context_test.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context_test.go @@ -38,12 +38,10 @@ func TestSubTaskExecutionContext(t *testing.T) { assert.Equal(t, storage.DataReference("/raw_prefix/5/1"), stCtx.OutputWriter().GetRawOutputPrefix()) assert.Equal(t, tasklog.TemplateVars{ - "taskExecution": tasklog.TemplateVars{ - "executionIndex": 0, - "parentName": "notfound", - "subtaskRetryAttempt": uint64(1), - "taskRetryAttempt": uint32(0), - }, + {logTemplateRegexes.ParentName, "notfound"}, + {logTemplateRegexes.ExecutionIndex, "0"}, + {logTemplateRegexes.RetryAttempt, "1"}, + {logTemplateRegexes.ParentRetryAttempt, "0"}, }, stCtx.TaskExecutionMetadata().GetTaskExecutionID().(SubTaskExecutionID).ToTemplateVars(), ) diff --git a/go/tasks/plugins/k8s/pod/plugin.go b/go/tasks/plugins/k8s/pod/plugin.go index 5e6ae6276..efdfd3c58 100644 --- a/go/tasks/plugins/k8s/pod/plugin.go +++ b/go/tasks/plugins/k8s/pod/plugin.go @@ -146,7 +146,7 @@ func (p plugin) GetTaskPhase(ctx context.Context, pluginContext k8s.PluginContex return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)") } -func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string, extraLogTemplateVars ...tasklog.TemplateVars) (pluginsCore.PhaseInfo, error) { +func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string, extraLogTemplateVars ...tasklog.TemplateVar) (pluginsCore.PhaseInfo, error) { pluginState := k8s.PluginState{} _, err := pluginContext.PluginStateReader().Get(&pluginState) if err != nil { From e56c25fe3f77b63927689dda1b41620ec9962bda Mon Sep 17 00:00:00 2001 From: Jeev B Date: Thu, 13 Jul 2023 17:04:49 -0700 Subject: [PATCH 17/22] Readd benchmark --- go/tasks/pluginmachinery/tasklog/template.go | 139 ++++++++---------- .../pluginmachinery/tasklog/template_test.go | 7 + 2 files changed, 66 insertions(+), 80 deletions(-) diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index 29907f79b..fef2de956 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -13,7 +13,7 @@ func MustCreateRegex(varName string) *regexp.Regexp { return regexp.MustCompile(fmt.Sprintf(`(?i){{\s*[\.$]%s\s*}}`, varName)) } -var defaultRegexes = struct { +type templateRegexes struct { PodName *regexp.Regexp PodUID *regexp.Regexp Namespace *regexp.Regexp @@ -34,29 +34,35 @@ var defaultRegexes = struct { ExecutionName *regexp.Regexp ExecutionProject *regexp.Regexp ExecutionDomain *regexp.Regexp -}{ - MustCreateRegex("podName"), - MustCreateRegex("podUID"), - MustCreateRegex("namespace"), - MustCreateRegex("containerName"), - MustCreateRegex("containerID"), - MustCreateRegex("logName"), - MustCreateRegex("hostname"), - MustCreateRegex("podRFC3339StartTime"), - MustCreateRegex("podRFC3339FinishTime"), - MustCreateRegex("podUnixStartTime"), - MustCreateRegex("podUnixFinishTime"), - MustCreateRegex("taskID"), - MustCreateRegex("taskVersion"), - MustCreateRegex("taskProject"), - MustCreateRegex("taskDomain"), - MustCreateRegex("taskRetryAttempt"), - MustCreateRegex("nodeID"), - MustCreateRegex("executionName"), - MustCreateRegex("executionProject"), - MustCreateRegex("executionDomain"), } +func initDefaultRegexes() templateRegexes { + return templateRegexes{ + MustCreateRegex("podName"), + MustCreateRegex("podUID"), + MustCreateRegex("namespace"), + MustCreateRegex("containerName"), + MustCreateRegex("containerID"), + MustCreateRegex("logName"), + MustCreateRegex("hostname"), + MustCreateRegex("podRFC3339StartTime"), + MustCreateRegex("podRFC3339FinishTime"), + MustCreateRegex("podUnixStartTime"), + MustCreateRegex("podUnixFinishTime"), + MustCreateRegex("taskID"), + MustCreateRegex("taskVersion"), + MustCreateRegex("taskProject"), + MustCreateRegex("taskDomain"), + MustCreateRegex("taskRetryAttempt"), + MustCreateRegex("nodeID"), + MustCreateRegex("executionName"), + MustCreateRegex("executionProject"), + MustCreateRegex("executionDomain"), + } +} + +var defaultRegexes = initDefaultRegexes() + func replaceAll(template string, vars TemplateVars) string { for _, v := range vars { if len(v.Value) > 0 { @@ -76,97 +82,70 @@ func (input Input) ToTemplateVars() TemplateVars { } vars := TemplateVars{ + {defaultRegexes.PodName, input.PodName}, + {defaultRegexes.PodUID, input.PodUID}, + {defaultRegexes.Namespace, input.Namespace}, + {defaultRegexes.ContainerName, input.ContainerName}, + {defaultRegexes.ContainerID, containerID}, + {defaultRegexes.LogName, input.LogName}, + {defaultRegexes.Hostname, input.HostName}, + {defaultRegexes.PodRFC3339StartTime, input.PodRFC3339StartTime}, + {defaultRegexes.PodRFC3339FinishTime, input.PodRFC3339FinishTime}, { - Regex: defaultRegexes.PodName, - Value: input.PodName, - }, - { - Regex: defaultRegexes.PodUID, - Value: input.PodUID, - }, - { - Regex: defaultRegexes.Namespace, - Value: input.Namespace, - }, - { - Regex: defaultRegexes.ContainerName, - Value: input.ContainerName, - }, - { - Regex: defaultRegexes.ContainerID, - Value: containerID, - }, - { - Regex: defaultRegexes.LogName, - Value: input.LogName, - }, - { - Regex: defaultRegexes.Hostname, - Value: input.HostName, - }, - { - Regex: defaultRegexes.PodRFC3339StartTime, - Value: input.PodRFC3339StartTime, - }, - { - Regex: defaultRegexes.PodRFC3339FinishTime, - Value: input.PodRFC3339FinishTime, - }, - { - Regex: defaultRegexes.PodUnixStartTime, - Value: strconv.FormatInt(input.PodUnixStartTime, 10), + defaultRegexes.PodUnixStartTime, + strconv.FormatInt(input.PodUnixStartTime, 10), }, { - Regex: defaultRegexes.PodUnixFinishTime, - Value: strconv.FormatInt(input.PodUnixFinishTime, 10), + defaultRegexes.PodUnixFinishTime, + strconv.FormatInt(input.PodUnixFinishTime, 10), }, } if input.TaskExecutionIdentifier != nil { vars = append(vars, TemplateVar{ - Regex: defaultRegexes.TaskRetryAttempt, - Value: strconv.FormatUint(uint64(input.TaskExecutionIdentifier.RetryAttempt), 10), + defaultRegexes.TaskRetryAttempt, + strconv.FormatUint(uint64(input.TaskExecutionIdentifier.RetryAttempt), 10), }) if input.TaskExecutionIdentifier.TaskId != nil { vars = append( vars, TemplateVar{ - Regex: defaultRegexes.TaskID, - Value: input.TaskExecutionIdentifier.TaskId.Name, + defaultRegexes.TaskID, + input.TaskExecutionIdentifier.TaskId.Name, }, TemplateVar{ - Regex: defaultRegexes.TaskVersion, - Value: input.TaskExecutionIdentifier.TaskId.Version, + defaultRegexes.TaskVersion, + input.TaskExecutionIdentifier.TaskId.Version, }, TemplateVar{ - Regex: defaultRegexes.TaskProject, - Value: input.TaskExecutionIdentifier.TaskId.Project, + defaultRegexes.TaskProject, + input.TaskExecutionIdentifier.TaskId.Project, }, TemplateVar{ - Regex: defaultRegexes.TaskDomain, - Value: input.TaskExecutionIdentifier.TaskId.Domain, + defaultRegexes.TaskDomain, + input.TaskExecutionIdentifier.TaskId.Domain, }, ) } if input.TaskExecutionIdentifier.NodeExecutionId != nil { vars = append(vars, TemplateVar{ - Regex: defaultRegexes.NodeID, - Value: input.TaskExecutionIdentifier.NodeExecutionId.NodeId, + defaultRegexes.NodeID, + input.TaskExecutionIdentifier.NodeExecutionId.NodeId, }) if input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId != nil { vars = append( vars, TemplateVar{ - Regex: defaultRegexes.ExecutionName, - Value: input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Name, + defaultRegexes.ExecutionName, + input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Name, }, TemplateVar{ - Regex: defaultRegexes.ExecutionProject, - Value: input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Project, + defaultRegexes.ExecutionProject, + input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Project, }, TemplateVar{ - Regex: defaultRegexes.ExecutionDomain, - Value: input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Domain, + defaultRegexes.ExecutionDomain, + input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Domain, }, ) } diff --git a/go/tasks/pluginmachinery/tasklog/template_test.go b/go/tasks/pluginmachinery/tasklog/template_test.go index b923c86f9..b067e34ea 100644 --- a/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/go/tasks/pluginmachinery/tasklog/template_test.go @@ -30,6 +30,13 @@ func TestTemplateLog(t *testing.T) { assert.Equal(t, "https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.f-uuid-driver_pod-uid_flyteexamples-production_spark-kubernetes-driver-abc.log", tl.Uri) } +// Latest Run: Benchmark_mustInitTemplateRegexes-16 45960 26914 ns/op +func Benchmark_initDefaultRegexes(b *testing.B) { + for i := 0; i < b.N; i++ { + initDefaultRegexes() + } +} + func Test_templateLogPlugin_Regression(t *testing.T) { type fields struct { templateURI string From c41e055b34a2ab9b873506c3c141eccf00198605 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Fri, 14 Jul 2023 10:43:17 -0700 Subject: [PATCH 18/22] Split templating scheme (#373) * Use a split templating scheme * add enum for TemplateScheme --- go/tasks/logs/config.go | 2 + go/tasks/logs/logging_utils.go | 46 +++--- go/tasks/logs/logging_utils_test.go | 20 +-- go/tasks/pluginmachinery/tasklog/plugin.go | 41 +++-- go/tasks/pluginmachinery/tasklog/template.go | 153 ++++++++++-------- .../pluginmachinery/tasklog/template_test.go | 67 +++++++- .../tasklog/templatescheme_enumer.go | 113 +++++++++++++ go/tasks/plugins/array/k8s/subtask.go | 2 +- .../plugins/array/k8s/subtask_exec_context.go | 30 ++-- .../array/k8s/subtask_exec_context_test.go | 14 +- go/tasks/plugins/k8s/pod/plugin.go | 6 +- 11 files changed, 349 insertions(+), 145 deletions(-) create mode 100644 go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go diff --git a/go/tasks/logs/config.go b/go/tasks/logs/config.go index 7f9a0769b..a1af82590 100755 --- a/go/tasks/logs/config.go +++ b/go/tasks/logs/config.go @@ -3,6 +3,7 @@ package logs import ( "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" "github.com/flyteorg/flyteplugins/go/tasks/config" + "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/tasklog" ) //go:generate pflags LogConfig --default-var=DefaultConfig @@ -38,6 +39,7 @@ type TemplateLogPluginConfig struct { DisplayName string `json:"displayName" pflag:",Display name for the generated log when displayed in the console."` TemplateURIs []TemplateURI `json:"templateUris" pflag:",URI Templates for generating task log links."` MessageFormat core.TaskLog_MessageFormat `json:"messageFormat" pflag:",Log Message Format."` + Scheme tasklog.TemplateScheme `json:"scheme" pflag:",Templating scheme to use. Supported values are Pod and TaskExecution."` } var ( diff --git a/go/tasks/logs/logging_utils.go b/go/tasks/logs/logging_utils.go index 07c9656a7..741ed2d29 100755 --- a/go/tasks/logs/logging_utils.go +++ b/go/tasks/logs/logging_utils.go @@ -18,7 +18,7 @@ type logPlugin struct { } // Internal -func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID *core.TaskExecutionIdentifier, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars ...tasklog.TemplateVar) ([]*core.TaskLog, error) { +func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID *core.TaskExecutionIdentifier, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVarsByScheme *tasklog.TemplateVarsByScheme) ([]*core.TaskLog, error) { if logPlugin == nil { return nil, nil } @@ -43,18 +43,18 @@ func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, tas logs, err := logPlugin.GetTaskLogs( tasklog.Input{ - PodName: pod.Name, - PodUID: string(pod.GetUID()), - Namespace: pod.Namespace, - ContainerName: pod.Spec.Containers[index].Name, - ContainerID: pod.Status.ContainerStatuses[index].ContainerID, - LogName: nameSuffix, - PodRFC3339StartTime: time.Unix(startTime, 0).Format(time.RFC3339), - PodRFC3339FinishTime: time.Unix(finishTime, 0).Format(time.RFC3339), - PodUnixStartTime: startTime, - PodUnixFinishTime: finishTime, - TaskExecutionIdentifier: taskExecID, - ExtraTemplateVars: extraLogTemplateVars, + PodName: pod.Name, + PodUID: string(pod.GetUID()), + Namespace: pod.Namespace, + ContainerName: pod.Spec.Containers[index].Name, + ContainerID: pod.Status.ContainerStatuses[index].ContainerID, + LogName: nameSuffix, + PodRFC3339StartTime: time.Unix(startTime, 0).Format(time.RFC3339), + PodRFC3339FinishTime: time.Unix(finishTime, 0).Format(time.RFC3339), + PodUnixStartTime: startTime, + PodUnixFinishTime: finishTime, + TaskExecutionIdentifier: taskExecID, + ExtraTemplateVarsByScheme: extraLogTemplateVarsByScheme, }, ) @@ -95,33 +95,31 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { if cfg.IsKubernetesEnabled { if len(cfg.KubernetesTemplateURI) > 0 { - logPlugins = append(logPlugins, logPlugin{Name: "Kubernetes Logs", Plugin: tasklog.NewTemplateLogPlugin([]string{cfg.KubernetesTemplateURI}, core.TaskLog_JSON)}) + logPlugins = append(logPlugins, logPlugin{Name: "Kubernetes Logs", Plugin: tasklog.NewTemplateLogPlugin(tasklog.TemplateSchemePod, []string{cfg.KubernetesTemplateURI}, core.TaskLog_JSON)}) } else { - logPlugins = append(logPlugins, logPlugin{Name: "Kubernetes Logs", Plugin: tasklog.NewTemplateLogPlugin([]string{fmt.Sprintf("%s/#!/log/{{ .namespace }}/{{ .podName }}/pod?namespace={{ .namespace }}", cfg.KubernetesURL)}, core.TaskLog_JSON)}) + logPlugins = append(logPlugins, logPlugin{Name: "Kubernetes Logs", Plugin: tasklog.NewTemplateLogPlugin(tasklog.TemplateSchemePod, []string{fmt.Sprintf("%s/#!/log/{{ .namespace }}/{{ .podName }}/pod?namespace={{ .namespace }}", cfg.KubernetesURL)}, core.TaskLog_JSON)}) } } if cfg.IsCloudwatchEnabled { if len(cfg.CloudwatchTemplateURI) > 0 { - logPlugins = append(logPlugins, logPlugin{Name: "Cloudwatch Logs", Plugin: tasklog.NewTemplateLogPlugin([]string{cfg.CloudwatchTemplateURI}, core.TaskLog_JSON)}) + logPlugins = append(logPlugins, logPlugin{Name: "Cloudwatch Logs", Plugin: tasklog.NewTemplateLogPlugin(tasklog.TemplateSchemePod, []string{cfg.CloudwatchTemplateURI}, core.TaskLog_JSON)}) } else { - logPlugins = append(logPlugins, logPlugin{Name: "Cloudwatch Logs", Plugin: tasklog.NewTemplateLogPlugin( - []string{fmt.Sprintf("https://console.aws.amazon.com/cloudwatch/home?region=%s#logEventViewer:group=%s;stream=var.log.containers.{{ .podName }}_{{ .namespace }}_{{ .containerName }}-{{ .containerId }}.log", cfg.CloudwatchRegion, cfg.CloudwatchLogGroup)}, core.TaskLog_JSON)}) + logPlugins = append(logPlugins, logPlugin{Name: "Cloudwatch Logs", Plugin: tasklog.NewTemplateLogPlugin(tasklog.TemplateSchemePod, []string{fmt.Sprintf("https://console.aws.amazon.com/cloudwatch/home?region=%s#logEventViewer:group=%s;stream=var.log.containers.{{ .podName }}_{{ .namespace }}_{{ .containerName }}-{{ .containerId }}.log", cfg.CloudwatchRegion, cfg.CloudwatchLogGroup)}, core.TaskLog_JSON)}) } } if cfg.IsStackDriverEnabled { if len(cfg.StackDriverTemplateURI) > 0 { - logPlugins = append(logPlugins, logPlugin{Name: "Stackdriver Logs", Plugin: tasklog.NewTemplateLogPlugin([]string{cfg.StackDriverTemplateURI}, core.TaskLog_JSON)}) + logPlugins = append(logPlugins, logPlugin{Name: "Stackdriver Logs", Plugin: tasklog.NewTemplateLogPlugin(tasklog.TemplateSchemePod, []string{cfg.StackDriverTemplateURI}, core.TaskLog_JSON)}) } else { - logPlugins = append(logPlugins, logPlugin{Name: "Stackdriver Logs", Plugin: tasklog.NewTemplateLogPlugin( - []string{fmt.Sprintf("https://console.cloud.google.com/logs/viewer?project=%s&angularJsUrl=%%2Flogs%%2Fviewer%%3Fproject%%3D%s&resource=%s&advancedFilter=resource.labels.pod_name%%3D{{ .podName }}", cfg.GCPProjectName, cfg.GCPProjectName, cfg.StackdriverLogResourceName)}, core.TaskLog_JSON)}) + logPlugins = append(logPlugins, logPlugin{Name: "Stackdriver Logs", Plugin: tasklog.NewTemplateLogPlugin(tasklog.TemplateSchemePod, []string{fmt.Sprintf("https://console.cloud.google.com/logs/viewer?project=%s&angularJsUrl=%%2Flogs%%2Fviewer%%3Fproject%%3D%s&resource=%s&advancedFilter=resource.labels.pod_name%%3D{{ .podName }}", cfg.GCPProjectName, cfg.GCPProjectName, cfg.StackdriverLogResourceName)}, core.TaskLog_JSON)}) } } if len(cfg.Templates) > 0 { for _, cfg := range cfg.Templates { - logPlugins = append(logPlugins, logPlugin{Name: cfg.DisplayName, Plugin: tasklog.NewTemplateLogPlugin(cfg.TemplateURIs, cfg.MessageFormat)}) + logPlugins = append(logPlugins, logPlugin{Name: cfg.DisplayName, Plugin: tasklog.NewTemplateLogPlugin(cfg.Scheme, cfg.TemplateURIs, cfg.MessageFormat)}) } } @@ -129,7 +127,5 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { return nil, nil } - return taskLogPluginWrapper{ - logPlugins: logPlugins, - }, nil + return taskLogPluginWrapper{logPlugins: logPlugins}, nil } diff --git a/go/tasks/logs/logging_utils_test.go b/go/tasks/logs/logging_utils_test.go index 19eaf355d..aa23ec9a1 100755 --- a/go/tasks/logs/logging_utils_test.go +++ b/go/tasks/logs/logging_utils_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" + "github.com/flyteorg/flyteplugins/go/tasks/pluginmachinery/tasklog" "github.com/go-test/deep" v12 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,7 +37,7 @@ var dummyTaskExecID = &core.TaskExecutionIdentifier{ func TestGetLogsForContainerInPod_NoPlugins(t *testing.T) { logPlugin, err := InitializeLogPlugins(&LogConfig{}) assert.NoError(t, err) - l, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, nil, 0, " Suffix") + l, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, nil, 0, " Suffix", nil) assert.NoError(t, err) assert.Nil(t, l) } @@ -48,7 +49,7 @@ func TestGetLogsForContainerInPod_NoLogs(t *testing.T) { CloudwatchLogGroup: "/kubernetes/flyte-production", }) assert.NoError(t, err) - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, nil, 0, " Suffix") + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, nil, 0, " Suffix", nil) assert.NoError(t, err) assert.Nil(t, p) } @@ -79,7 +80,7 @@ func TestGetLogsForContainerInPod_BadIndex(t *testing.T) { } pod.Name = podName - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 1, " Suffix") + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 1, " Suffix", nil) assert.NoError(t, err) assert.Nil(t, p) } @@ -104,7 +105,7 @@ func TestGetLogsForContainerInPod_MissingStatus(t *testing.T) { } pod.Name = podName - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 1, " Suffix") + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 1, " Suffix", nil) assert.NoError(t, err) assert.Nil(t, p) } @@ -134,7 +135,7 @@ func TestGetLogsForContainerInPod_Cloudwatch(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix", nil) assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -164,7 +165,7 @@ func TestGetLogsForContainerInPod_K8s(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix", nil) assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -197,7 +198,7 @@ func TestGetLogsForContainerInPod_All(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix", nil) assert.Nil(t, err) assert.Len(t, logs, 2) } @@ -228,7 +229,7 @@ func TestGetLogsForContainerInPod_Stackdriver(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " Suffix", nil) assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -302,7 +303,7 @@ func assertTestSucceeded(tb testing.TB, config *LogConfig, expectedTaskLogs []*c }, } - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " my-Suffix") + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID, pod, 0, " my-Suffix", nil) assert.Nil(tb, err) assert.Len(tb, logs, len(expectedTaskLogs)) if diff := deep.Equal(logs, expectedTaskLogs); len(diff) > 0 { @@ -326,6 +327,7 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { "https://flyte.corp.net/console/projects/{{ .executionProject }}/domains/{{ .executionDomain }}/executions/{{ .executionName }}/nodeId/{{ .nodeID }}/taskId/{{ .taskID }}/attempt/{{ .taskRetryAttempt }}/view/logs", }, MessageFormat: core.TaskLog_JSON, + Scheme: tasklog.TemplateSchemeTaskExecution, }, }, }, []*core.TaskLog{ diff --git a/go/tasks/pluginmachinery/tasklog/plugin.go b/go/tasks/pluginmachinery/tasklog/plugin.go index 98504943b..730e5b424 100644 --- a/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/go/tasks/pluginmachinery/tasklog/plugin.go @@ -6,6 +6,15 @@ import ( "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" ) +//go:generate enumer --type=TemplateScheme --trimprefix=TemplateScheme -json -yaml + +type TemplateScheme int + +const ( + TemplateSchemePod TemplateScheme = iota + TemplateSchemeTaskExecution +) + type TemplateVar struct { Regex *regexp.Regexp Value string @@ -13,22 +22,28 @@ type TemplateVar struct { type TemplateVars []TemplateVar +type TemplateVarsByScheme struct { + Common TemplateVars + Pod TemplateVars + TaskExecution TemplateVars +} + // Input contains all available information about task's execution that a log plugin can use to construct task's // log links. type Input struct { - HostName string - PodName string - Namespace string - ContainerName string - ContainerID string - LogName string - PodRFC3339StartTime string - PodRFC3339FinishTime string - PodUnixStartTime int64 - PodUnixFinishTime int64 - PodUID string - TaskExecutionIdentifier *core.TaskExecutionIdentifier - ExtraTemplateVars TemplateVars + HostName string + PodName string + Namespace string + ContainerName string + ContainerID string + LogName string + PodRFC3339StartTime string + PodRFC3339FinishTime string + PodUnixStartTime int64 + PodUnixFinishTime int64 + PodUID string + TaskExecutionIdentifier *core.TaskExecutionIdentifier + ExtraTemplateVarsByScheme *TemplateVarsByScheme } // Output contains all task logs a plugin generates for a given Input. diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index fef2de956..ab9be67bd 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -14,12 +14,12 @@ func MustCreateRegex(varName string) *regexp.Regexp { } type templateRegexes struct { + LogName *regexp.Regexp PodName *regexp.Regexp PodUID *regexp.Regexp Namespace *regexp.Regexp ContainerName *regexp.Regexp ContainerID *regexp.Regexp - LogName *regexp.Regexp Hostname *regexp.Regexp PodRFC3339StartTime *regexp.Regexp PodRFC3339FinishTime *regexp.Regexp @@ -38,12 +38,12 @@ type templateRegexes struct { func initDefaultRegexes() templateRegexes { return templateRegexes{ + MustCreateRegex("logName"), MustCreateRegex("podName"), MustCreateRegex("podUID"), MustCreateRegex("namespace"), MustCreateRegex("containerName"), MustCreateRegex("containerID"), - MustCreateRegex("logName"), MustCreateRegex("hostname"), MustCreateRegex("podRFC3339StartTime"), MustCreateRegex("podRFC3339FinishTime"), @@ -72,92 +72,110 @@ func replaceAll(template string, vars TemplateVars) string { return template } -func (input Input) ToTemplateVars() TemplateVars { - // Container IDs are prefixed with docker://, cri-o://, etc. which is stripped by fluentd before pushing to a log - // stream. Therefore, we must also strip the prefix. - containerID := input.ContainerID - stripDelimiter := "://" - if split := strings.Split(input.ContainerID, stripDelimiter); len(split) > 1 { - containerID = split[1] - } - +func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { vars := TemplateVars{ - {defaultRegexes.PodName, input.PodName}, - {defaultRegexes.PodUID, input.PodUID}, - {defaultRegexes.Namespace, input.Namespace}, - {defaultRegexes.ContainerName, input.ContainerName}, - {defaultRegexes.ContainerID, containerID}, {defaultRegexes.LogName, input.LogName}, - {defaultRegexes.Hostname, input.HostName}, - {defaultRegexes.PodRFC3339StartTime, input.PodRFC3339StartTime}, - {defaultRegexes.PodRFC3339FinishTime, input.PodRFC3339FinishTime}, - { - defaultRegexes.PodUnixStartTime, - strconv.FormatInt(input.PodUnixStartTime, 10), - }, - { - defaultRegexes.PodUnixFinishTime, - strconv.FormatInt(input.PodUnixFinishTime, 10), - }, } - if input.TaskExecutionIdentifier != nil { - vars = append(vars, TemplateVar{ - defaultRegexes.TaskRetryAttempt, - strconv.FormatUint(uint64(input.TaskExecutionIdentifier.RetryAttempt), 10), - }) - if input.TaskExecutionIdentifier.TaskId != nil { - vars = append( - vars, - TemplateVar{ - defaultRegexes.TaskID, - input.TaskExecutionIdentifier.TaskId.Name, - }, - TemplateVar{ - defaultRegexes.TaskVersion, - input.TaskExecutionIdentifier.TaskId.Version, - }, - TemplateVar{ - defaultRegexes.TaskProject, - input.TaskExecutionIdentifier.TaskId.Project, - }, - TemplateVar{ - defaultRegexes.TaskDomain, - input.TaskExecutionIdentifier.TaskId.Domain, - }, - ) + gotExtraTemplateVars := input.ExtraTemplateVarsByScheme != nil + if gotExtraTemplateVars { + vars = append(vars, input.ExtraTemplateVarsByScheme.Common...) + } + + switch scheme { + case TemplateSchemePod: + // Container IDs are prefixed with docker://, cri-o://, etc. which is stripped by fluentd before pushing to a log + // stream. Therefore, we must also strip the prefix. + containerID := input.ContainerID + stripDelimiter := "://" + if split := strings.Split(input.ContainerID, stripDelimiter); len(split) > 1 { + containerID = split[1] } - if input.TaskExecutionIdentifier.NodeExecutionId != nil { + vars = append( + vars, + TemplateVar{defaultRegexes.PodName, input.PodName}, + TemplateVar{defaultRegexes.PodUID, input.PodUID}, + TemplateVar{defaultRegexes.Namespace, input.Namespace}, + TemplateVar{defaultRegexes.ContainerName, input.ContainerName}, + TemplateVar{defaultRegexes.ContainerID, containerID}, + TemplateVar{defaultRegexes.Hostname, input.HostName}, + TemplateVar{defaultRegexes.PodRFC3339StartTime, input.PodRFC3339StartTime}, + TemplateVar{defaultRegexes.PodRFC3339FinishTime, input.PodRFC3339FinishTime}, + TemplateVar{ + defaultRegexes.PodUnixStartTime, + strconv.FormatInt(input.PodUnixStartTime, 10), + }, + TemplateVar{ + defaultRegexes.PodUnixFinishTime, + strconv.FormatInt(input.PodUnixFinishTime, 10), + }, + ) + if gotExtraTemplateVars { + vars = append(vars, input.ExtraTemplateVarsByScheme.Pod...) + } + case TemplateSchemeTaskExecution: + if input.TaskExecutionIdentifier != nil { vars = append(vars, TemplateVar{ - defaultRegexes.NodeID, - input.TaskExecutionIdentifier.NodeExecutionId.NodeId, + defaultRegexes.TaskRetryAttempt, + strconv.FormatUint(uint64(input.TaskExecutionIdentifier.RetryAttempt), 10), }) - if input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId != nil { + if input.TaskExecutionIdentifier.TaskId != nil { vars = append( vars, TemplateVar{ - defaultRegexes.ExecutionName, - input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Name, + defaultRegexes.TaskID, + input.TaskExecutionIdentifier.TaskId.Name, }, TemplateVar{ - defaultRegexes.ExecutionProject, - input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Project, + defaultRegexes.TaskVersion, + input.TaskExecutionIdentifier.TaskId.Version, }, TemplateVar{ - defaultRegexes.ExecutionDomain, - input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Domain, + defaultRegexes.TaskProject, + input.TaskExecutionIdentifier.TaskId.Project, + }, + TemplateVar{ + defaultRegexes.TaskDomain, + input.TaskExecutionIdentifier.TaskId.Domain, }, ) } + if input.TaskExecutionIdentifier.NodeExecutionId != nil { + vars = append(vars, TemplateVar{ + defaultRegexes.NodeID, + input.TaskExecutionIdentifier.NodeExecutionId.NodeId, + }) + if input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId != nil { + vars = append( + vars, + TemplateVar{ + defaultRegexes.ExecutionName, + input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Name, + }, + TemplateVar{ + defaultRegexes.ExecutionProject, + input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Project, + }, + TemplateVar{ + defaultRegexes.ExecutionDomain, + input.TaskExecutionIdentifier.NodeExecutionId.ExecutionId.Domain, + }, + ) + } + } + } + if gotExtraTemplateVars { + vars = append(vars, input.ExtraTemplateVarsByScheme.TaskExecution...) } } - return append(vars, input.ExtraTemplateVars...) + return vars } // A simple log plugin that supports templates in urls to build the final log link. // See `defaultRegexes` for supported templates. type TemplateLogPlugin struct { + scheme TemplateScheme templateUris []string messageFormat core.TaskLog_MessageFormat } @@ -184,7 +202,7 @@ func (s TemplateLogPlugin) GetTaskLog(podName, podUID, namespace, containerName, } func (s TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { - templateVars := input.ToTemplateVars() + templateVars := input.templateVarsForScheme(s.scheme) taskLogs := make([]*core.TaskLog, 0, len(s.templateUris)) for _, templateURI := range s.templateUris { taskLogs = append(taskLogs, &core.TaskLog{ @@ -194,9 +212,7 @@ func (s TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { }) } - return Output{ - TaskLogs: taskLogs, - }, nil + return Output{TaskLogs: taskLogs}, nil } // NewTemplateLogPlugin creates a template-based log plugin with the provided template Uri and message format. Supported @@ -212,8 +228,9 @@ func (s TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { // {{ .PodRFC3339FinishTime }}: Don't have a good mechanism for this yet, but approximating with time.Now for now // {{ .podUnixStartTime }}: The pod creation time (in unix seconds, not millis) // {{ .podUnixFinishTime }}: Don't have a good mechanism for this yet, but approximating with time.Now for now -func NewTemplateLogPlugin(templateUris []string, messageFormat core.TaskLog_MessageFormat) TemplateLogPlugin { +func NewTemplateLogPlugin(scheme TemplateScheme, templateUris []string, messageFormat core.TaskLog_MessageFormat) TemplateLogPlugin { return TemplateLogPlugin{ + scheme: scheme, templateUris: templateUris, messageFormat: messageFormat, } diff --git a/go/tasks/pluginmachinery/tasklog/template_test.go b/go/tasks/pluginmachinery/tasklog/template_test.go index b067e34ea..e169c0ccb 100644 --- a/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/go/tasks/pluginmachinery/tasklog/template_test.go @@ -11,7 +11,7 @@ import ( ) func TestTemplateLog(t *testing.T) { - p := NewTemplateLogPlugin([]string{"https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.{{.podName}}_{{.podUID}}_{{.namespace}}_{{.containerName}}-{{.containerId}}.log"}, core.TaskLog_JSON) + p := NewTemplateLogPlugin(TemplateSchemePod, []string{"https://console.aws.amazon.com/cloudwatch/home?region=us-east-1#logEventViewer:group=/flyte-production/kubernetes;stream=var.log.containers.{{.podName}}_{{.podUID}}_{{.namespace}}_{{.containerName}}-{{.containerId}}.log"}, core.TaskLog_JSON) tl, err := p.GetTaskLog( "f-uuid-driver", "pod-uid", @@ -159,6 +159,7 @@ func Test_templateLogPlugin_Regression(t *testing.T) { func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { type fields struct { + scheme TemplateScheme templateURI string messageFormat core.TaskLog_MessageFormat } @@ -266,8 +267,59 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { false, }, { - "custom-with-task-execution-identifier", + "task-with-task-execution-identifier", fields{ + scheme: TemplateSchemeTaskExecution, + templateURI: "https://flyte.corp.net/console/projects/{{ .executionProject }}/domains/{{ .executionDomain }}/executions/{{ .executionName }}/nodeId/{{ .nodeID }}/taskId/{{ .taskID }}/attempt/{{ .taskRetryAttempt }}/view/logs", + messageFormat: core.TaskLog_JSON, + }, + args{ + input: Input{ + HostName: "my-host", + PodName: "my-pod", + Namespace: "my-namespace", + ContainerName: "my-container", + ContainerID: "ignore", + LogName: "main_logs", + PodRFC3339StartTime: "1970-01-01T01:02:03+01:00", + PodRFC3339FinishTime: "1970-01-01T04:25:45+01:00", + PodUnixStartTime: 123, + PodUnixFinishTime: 12345, + TaskExecutionIdentifier: &core.TaskExecutionIdentifier{ + TaskId: &core.Identifier{ + ResourceType: core.ResourceType_TASK, + Name: "my-task-name", + Project: "my-project", + Domain: "my-domain", + Version: "1", + }, + NodeExecutionId: &core.NodeExecutionIdentifier{ + NodeId: "n0", + ExecutionId: &core.WorkflowExecutionIdentifier{ + Name: "my-execution-name", + Project: "my-project", + Domain: "my-domain", + }, + }, + RetryAttempt: 0, + }, + }, + }, + Output{ + TaskLogs: []*core.TaskLog{ + { + Uri: "https://flyte.corp.net/console/projects/my-project/domains/my-domain/executions/my-execution-name/nodeId/n0/taskId/my-task-name/attempt/0/view/logs", + MessageFormat: core.TaskLog_JSON, + Name: "main_logs", + }, + }, + }, + false, + }, + { + "mapped-task-with-task-execution-identifier", + fields{ + scheme: TemplateSchemeTaskExecution, templateURI: "https://flyte.corp.net/console/projects/{{ .executionProject }}/domains/{{ .executionDomain }}/executions/{{ .executionName }}/nodeId/{{ .nodeID }}/taskId/{{ .taskID }}/attempt/{{ .subtaskParentRetryAttempt }}/mappedIndex/{{ .subtaskExecutionIndex }}/mappedAttempt/{{ .subtaskRetryAttempt }}/view/logs", messageFormat: core.TaskLog_JSON, }, @@ -301,10 +353,12 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { }, RetryAttempt: 0, }, - ExtraTemplateVars: TemplateVars{ - {MustCreateRegex("subtaskExecutionIndex"), "1"}, - {MustCreateRegex("subtaskRetryAttempt"), "1"}, - {MustCreateRegex("subtaskParentRetryAttempt"), "0"}, + ExtraTemplateVarsByScheme: &TemplateVarsByScheme{ + TaskExecution: TemplateVars{ + {MustCreateRegex("subtaskExecutionIndex"), "1"}, + {MustCreateRegex("subtaskRetryAttempt"), "1"}, + {MustCreateRegex("subtaskParentRetryAttempt"), "0"}, + }, }, }, }, @@ -323,6 +377,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := TemplateLogPlugin{ + scheme: tt.fields.scheme, templateUris: []string{tt.fields.templateURI}, messageFormat: tt.fields.messageFormat, } diff --git a/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go b/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go new file mode 100644 index 000000000..4b6f3b8a6 --- /dev/null +++ b/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go @@ -0,0 +1,113 @@ +// Code generated by "enumer --type=TemplateScheme --trimprefix=TemplateScheme -json -yaml"; DO NOT EDIT. + +package tasklog + +import ( + "encoding/json" + "fmt" + "strings" +) + +const _TemplateSchemeName = "PodTaskExecution" + +var _TemplateSchemeIndex = [...]uint8{0, 3, 16} + +const _TemplateSchemeLowerName = "podtaskexecution" + +func (i TemplateScheme) String() string { + if i < 0 || i >= TemplateScheme(len(_TemplateSchemeIndex)-1) { + return fmt.Sprintf("TemplateScheme(%d)", i) + } + return _TemplateSchemeName[_TemplateSchemeIndex[i]:_TemplateSchemeIndex[i+1]] +} + +// An "invalid array index" compiler error signifies that the constant values have changed. +// Re-run the stringer command to generate them again. +func _TemplateSchemeNoOp() { + var x [1]struct{} + _ = x[TemplateSchemePod-(0)] + _ = x[TemplateSchemeTaskExecution-(1)] +} + +var _TemplateSchemeValues = []TemplateScheme{TemplateSchemePod, TemplateSchemeTaskExecution} + +var _TemplateSchemeNameToValueMap = map[string]TemplateScheme{ + _TemplateSchemeName[0:3]: TemplateSchemePod, + _TemplateSchemeLowerName[0:3]: TemplateSchemePod, + _TemplateSchemeName[3:16]: TemplateSchemeTaskExecution, + _TemplateSchemeLowerName[3:16]: TemplateSchemeTaskExecution, +} + +var _TemplateSchemeNames = []string{ + _TemplateSchemeName[0:3], + _TemplateSchemeName[3:16], +} + +// TemplateSchemeString retrieves an enum value from the enum constants string name. +// Throws an error if the param is not part of the enum. +func TemplateSchemeString(s string) (TemplateScheme, error) { + if val, ok := _TemplateSchemeNameToValueMap[s]; ok { + return val, nil + } + + if val, ok := _TemplateSchemeNameToValueMap[strings.ToLower(s)]; ok { + return val, nil + } + return 0, fmt.Errorf("%s does not belong to TemplateScheme values", s) +} + +// TemplateSchemeValues returns all values of the enum +func TemplateSchemeValues() []TemplateScheme { + return _TemplateSchemeValues +} + +// TemplateSchemeStrings returns a slice of all String values of the enum +func TemplateSchemeStrings() []string { + strs := make([]string, len(_TemplateSchemeNames)) + copy(strs, _TemplateSchemeNames) + return strs +} + +// IsATemplateScheme returns "true" if the value is listed in the enum definition. "false" otherwise +func (i TemplateScheme) IsATemplateScheme() bool { + for _, v := range _TemplateSchemeValues { + if i == v { + return true + } + } + return false +} + +// MarshalJSON implements the json.Marshaler interface for TemplateScheme +func (i TemplateScheme) MarshalJSON() ([]byte, error) { + return json.Marshal(i.String()) +} + +// UnmarshalJSON implements the json.Unmarshaler interface for TemplateScheme +func (i *TemplateScheme) UnmarshalJSON(data []byte) error { + var s string + if err := json.Unmarshal(data, &s); err != nil { + return fmt.Errorf("TemplateScheme should be a string, got %s", data) + } + + var err error + *i, err = TemplateSchemeString(s) + return err +} + +// MarshalYAML implements a YAML Marshaler for TemplateScheme +func (i TemplateScheme) MarshalYAML() (interface{}, error) { + return i.String(), nil +} + +// UnmarshalYAML implements a YAML Unmarshaler for TemplateScheme +func (i *TemplateScheme) UnmarshalYAML(unmarshal func(interface{}) error) error { + var s string + if err := unmarshal(&s); err != nil { + return err + } + + var err error + *i, err = TemplateSchemeString(s) + return err +} diff --git a/go/tasks/plugins/array/k8s/subtask.go b/go/tasks/plugins/array/k8s/subtask.go index 38cd12b70..aefb4ac5b 100644 --- a/go/tasks/plugins/array/k8s/subtask.go +++ b/go/tasks/plugins/array/k8s/subtask.go @@ -304,7 +304,7 @@ func getSubtaskPhaseInfo(ctx context.Context, stCtx SubTaskExecutionContext, cfg } stID, _ := stCtx.TaskExecutionMetadata().GetTaskExecutionID().(SubTaskExecutionID) - phaseInfo, err := podPlugin.DefaultPodPlugin.GetTaskPhaseWithLogs(ctx, stCtx, pod, logPlugin, stID.GetLogSuffix(), stID.ToTemplateVars()...) + phaseInfo, err := podPlugin.DefaultPodPlugin.GetTaskPhaseWithLogs(ctx, stCtx, pod, logPlugin, stID.GetLogSuffix(), stID.TemplateVarsByScheme()) if err != nil { logger.Warnf(ctx, "failed to check status of resource in plugin [%s], with error: %s", executorName, err.Error()) return pluginsCore.PhaseInfoUndefined, err diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context.go b/go/tasks/plugins/array/k8s/subtask_exec_context.go index a34aace50..384e39638 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context.go @@ -189,20 +189,22 @@ var logTemplateRegexes = struct { tasklog.MustCreateRegex("subtaskParentRetryAttempt"), } -func (s SubTaskExecutionID) ToTemplateVars() tasklog.TemplateVars { - return tasklog.TemplateVars{ - {logTemplateRegexes.ParentName, s.parentName}, - { - logTemplateRegexes.ExecutionIndex, - strconv.FormatUint(uint64(s.executionIndex), 10), - }, - { - logTemplateRegexes.RetryAttempt, - strconv.FormatUint(uint64(s.subtaskRetryAttempt), 10), - }, - { - logTemplateRegexes.ParentRetryAttempt, - strconv.FormatUint(uint64(s.taskRetryAttempt), 10), +func (s SubTaskExecutionID) TemplateVarsByScheme() *tasklog.TemplateVarsByScheme { + return &tasklog.TemplateVarsByScheme{ + TaskExecution: tasklog.TemplateVars{ + {logTemplateRegexes.ParentName, s.parentName}, + { + logTemplateRegexes.ExecutionIndex, + strconv.FormatUint(uint64(s.executionIndex), 10), + }, + { + logTemplateRegexes.RetryAttempt, + strconv.FormatUint(uint64(s.subtaskRetryAttempt), 10), + }, + { + logTemplateRegexes.ParentRetryAttempt, + strconv.FormatUint(uint64(s.taskRetryAttempt), 10), + }, }, } } diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context_test.go b/go/tasks/plugins/array/k8s/subtask_exec_context_test.go index 420760e26..cf1f68669 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context_test.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context_test.go @@ -37,12 +37,14 @@ func TestSubTaskExecutionContext(t *testing.T) { assert.Equal(t, storage.DataReference("/prefix/"), stCtx.OutputWriter().GetOutputPrefixPath()) assert.Equal(t, storage.DataReference("/raw_prefix/5/1"), stCtx.OutputWriter().GetRawOutputPrefix()) assert.Equal(t, - tasklog.TemplateVars{ - {logTemplateRegexes.ParentName, "notfound"}, - {logTemplateRegexes.ExecutionIndex, "0"}, - {logTemplateRegexes.RetryAttempt, "1"}, - {logTemplateRegexes.ParentRetryAttempt, "0"}, + &tasklog.TemplateVarsByScheme{ + TaskExecution: tasklog.TemplateVars{ + {logTemplateRegexes.ParentName, "notfound"}, + {logTemplateRegexes.ExecutionIndex, "0"}, + {logTemplateRegexes.RetryAttempt, "1"}, + {logTemplateRegexes.ParentRetryAttempt, "0"}, + }, }, - stCtx.TaskExecutionMetadata().GetTaskExecutionID().(SubTaskExecutionID).ToTemplateVars(), + stCtx.TaskExecutionMetadata().GetTaskExecutionID().(SubTaskExecutionID).TemplateVarsByScheme(), ) } diff --git a/go/tasks/plugins/k8s/pod/plugin.go b/go/tasks/plugins/k8s/pod/plugin.go index efdfd3c58..2ab446ee0 100644 --- a/go/tasks/plugins/k8s/pod/plugin.go +++ b/go/tasks/plugins/k8s/pod/plugin.go @@ -143,10 +143,10 @@ func (p plugin) GetTaskPhase(ctx context.Context, pluginContext k8s.PluginContex return pluginsCore.PhaseInfoUndefined, err } - return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)") + return p.GetTaskPhaseWithLogs(ctx, pluginContext, r, logPlugin, " (User)", nil) } -func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string, extraLogTemplateVars ...tasklog.TemplateVar) (pluginsCore.PhaseInfo, error) { +func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string, extraLogTemplateVarsByScheme *tasklog.TemplateVarsByScheme) (pluginsCore.PhaseInfo, error) { pluginState := k8s.PluginState{} _, err := pluginContext.PluginStateReader().Get(&pluginState) if err != nil { @@ -168,7 +168,7 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin taskExecID := pluginContext.TaskExecutionMetadata().GetTaskExecutionID().GetID() if pod.Status.Phase != v1.PodPending && pod.Status.Phase != v1.PodUnknown { - taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, &taskExecID, pod, 0, logSuffix, extraLogTemplateVars...) + taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, &taskExecID, pod, 0, logSuffix, extraLogTemplateVarsByScheme) if err != nil { return pluginsCore.PhaseInfoUndefined, err } From 065643fb706cf7dc78eb0c40752604985a654ca2 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Fri, 14 Jul 2023 12:19:02 -0700 Subject: [PATCH 19/22] cleanup comment --- go/tasks/pluginmachinery/tasklog/template.go | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/go/tasks/pluginmachinery/tasklog/template.go b/go/tasks/pluginmachinery/tasklog/template.go index ab9be67bd..13f298489 100644 --- a/go/tasks/pluginmachinery/tasklog/template.go +++ b/go/tasks/pluginmachinery/tasklog/template.go @@ -215,19 +215,8 @@ func (s TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { return Output{TaskLogs: taskLogs}, nil } -// NewTemplateLogPlugin creates a template-based log plugin with the provided template Uri and message format. Supported -// templates are: -// {{ .podName }}: Gets the pod name as it shows in k8s dashboard, -// {{ .podUID }}: Gets the pod UID, -// {{ .namespace }}: K8s namespace where the pod runs, -// {{ .containerName }}: The container name that generated the log, -// {{ .containerId }}: The container id docker/crio generated at run time, -// {{ .logName }}: A deployment specific name where to expect the logs to be. -// {{ .hostname }}: The hostname where the pod is running and where logs reside. -// {{ .PodRFC3339StartTime }}: The pod creation time in RFC3339 format -// {{ .PodRFC3339FinishTime }}: Don't have a good mechanism for this yet, but approximating with time.Now for now -// {{ .podUnixStartTime }}: The pod creation time (in unix seconds, not millis) -// {{ .podUnixFinishTime }}: Don't have a good mechanism for this yet, but approximating with time.Now for now +// NewTemplateLogPlugin creates a template-based log plugin with the provided template Uri and message format. +// See `defaultRegexes` for supported templates. func NewTemplateLogPlugin(scheme TemplateScheme, templateUris []string, messageFormat core.TaskLog_MessageFormat) TemplateLogPlugin { return TemplateLogPlugin{ scheme: scheme, From 791f21db6d2abe28240f65d86397da87b0dac37c Mon Sep 17 00:00:00 2001 From: Jeev B Date: Fri, 14 Jul 2023 12:28:42 -0700 Subject: [PATCH 20/22] fix linting issues --- go/tasks/plugins/array/k8s/subtask_exec_context.go | 14 +++++++------- .../plugins/array/k8s/subtask_exec_context_test.go | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context.go b/go/tasks/plugins/array/k8s/subtask_exec_context.go index 384e39638..646118698 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context.go @@ -192,18 +192,18 @@ var logTemplateRegexes = struct { func (s SubTaskExecutionID) TemplateVarsByScheme() *tasklog.TemplateVarsByScheme { return &tasklog.TemplateVarsByScheme{ TaskExecution: tasklog.TemplateVars{ - {logTemplateRegexes.ParentName, s.parentName}, + {Regex: logTemplateRegexes.ParentName, Value: s.parentName}, { - logTemplateRegexes.ExecutionIndex, - strconv.FormatUint(uint64(s.executionIndex), 10), + Regex: logTemplateRegexes.ExecutionIndex, + Value: strconv.FormatUint(uint64(s.executionIndex), 10), }, { - logTemplateRegexes.RetryAttempt, - strconv.FormatUint(uint64(s.subtaskRetryAttempt), 10), + Regex: logTemplateRegexes.RetryAttempt, + Value: strconv.FormatUint(s.subtaskRetryAttempt, 10), }, { - logTemplateRegexes.ParentRetryAttempt, - strconv.FormatUint(uint64(s.taskRetryAttempt), 10), + Regex: logTemplateRegexes.ParentRetryAttempt, + Value: strconv.FormatUint(uint64(s.taskRetryAttempt), 10), }, }, } diff --git a/go/tasks/plugins/array/k8s/subtask_exec_context_test.go b/go/tasks/plugins/array/k8s/subtask_exec_context_test.go index cf1f68669..04b03b582 100644 --- a/go/tasks/plugins/array/k8s/subtask_exec_context_test.go +++ b/go/tasks/plugins/array/k8s/subtask_exec_context_test.go @@ -39,10 +39,10 @@ func TestSubTaskExecutionContext(t *testing.T) { assert.Equal(t, &tasklog.TemplateVarsByScheme{ TaskExecution: tasklog.TemplateVars{ - {logTemplateRegexes.ParentName, "notfound"}, - {logTemplateRegexes.ExecutionIndex, "0"}, - {logTemplateRegexes.RetryAttempt, "1"}, - {logTemplateRegexes.ParentRetryAttempt, "0"}, + {Regex: logTemplateRegexes.ParentName, Value: "notfound"}, + {Regex: logTemplateRegexes.ExecutionIndex, Value: "0"}, + {Regex: logTemplateRegexes.RetryAttempt, Value: "1"}, + {Regex: logTemplateRegexes.ParentRetryAttempt, Value: "0"}, }, }, stCtx.TaskExecutionMetadata().GetTaskExecutionID().(SubTaskExecutionID).TemplateVarsByScheme(), From c8fdc345def2e965315451ada8fc970a052e1606 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Fri, 14 Jul 2023 13:26:34 -0700 Subject: [PATCH 21/22] add unit tests for templateVarsForScheme --- .../pluginmachinery/tasklog/template_test.go | 199 ++++++++++++++++++ 1 file changed, 199 insertions(+) diff --git a/go/tasks/pluginmachinery/tasklog/template_test.go b/go/tasks/pluginmachinery/tasklog/template_test.go index e169c0ccb..dc8b5799b 100644 --- a/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/go/tasks/pluginmachinery/tasklog/template_test.go @@ -2,6 +2,7 @@ package tasklog import ( "reflect" + "regexp" "testing" @@ -37,6 +38,204 @@ func Benchmark_initDefaultRegexes(b *testing.B) { } } +func Test_Input_templateVarsForScheme(t *testing.T) { + testRegexes := struct { + Foo *regexp.Regexp + Bar *regexp.Regexp + Baz *regexp.Regexp + Ham *regexp.Regexp + Spam *regexp.Regexp + }{ + MustCreateRegex("foo"), + MustCreateRegex("bar"), + MustCreateRegex("baz"), + MustCreateRegex("ham"), + MustCreateRegex("spam"), + } + podBase := Input{ + HostName: "my-host", + PodName: "my-pod", + PodUID: "my-pod-uid", + Namespace: "my-namespace", + ContainerName: "my-container", + ContainerID: "docker://containerID", + LogName: "main_logs", + PodRFC3339StartTime: "1970-01-01T01:02:03+01:00", + PodRFC3339FinishTime: "1970-01-01T04:25:45+01:00", + PodUnixStartTime: 123, + PodUnixFinishTime: 12345, + } + taskExecutionBase := Input{ + LogName: "main_logs", + TaskExecutionIdentifier: &core.TaskExecutionIdentifier{ + TaskId: &core.Identifier{ + ResourceType: core.ResourceType_TASK, + Name: "my-task-name", + Project: "my-task-project", + Domain: "my-task-domain", + Version: "1", + }, + NodeExecutionId: &core.NodeExecutionIdentifier{ + NodeId: "n0", + ExecutionId: &core.WorkflowExecutionIdentifier{ + Name: "my-execution-name", + Project: "my-execution-project", + Domain: "my-execution-domain", + }, + }, + RetryAttempt: 0, + }, + } + + tests := []struct { + name string + scheme TemplateScheme + baseVars Input + extraVars *TemplateVarsByScheme + exact TemplateVars + contains TemplateVars + notContains TemplateVars + }{ + { + "pod happy path", + TemplateSchemePod, + podBase, + nil, + TemplateVars{ + {defaultRegexes.LogName, "main_logs"}, + {defaultRegexes.PodName, "my-pod"}, + {defaultRegexes.PodUID, "my-pod-uid"}, + {defaultRegexes.Namespace, "my-namespace"}, + {defaultRegexes.ContainerName, "my-container"}, + {defaultRegexes.ContainerID, "containerID"}, + {defaultRegexes.Hostname, "my-host"}, + {defaultRegexes.PodRFC3339StartTime, "1970-01-01T01:02:03+01:00"}, + {defaultRegexes.PodRFC3339FinishTime, "1970-01-01T04:25:45+01:00"}, + {defaultRegexes.PodUnixStartTime, "123"}, + {defaultRegexes.PodUnixFinishTime, "12345"}, + }, + nil, + nil, + }, + { + "pod with extra vars", + TemplateSchemePod, + podBase, + &TemplateVarsByScheme{ + Common: TemplateVars{ + {testRegexes.Foo, "foo"}, + }, + Pod: TemplateVars{ + {testRegexes.Bar, "bar"}, + {testRegexes.Baz, "baz"}, + }, + }, + nil, + TemplateVars{ + {testRegexes.Foo, "foo"}, + {testRegexes.Bar, "bar"}, + {testRegexes.Baz, "baz"}, + }, + nil, + }, + { + "pod with unused extra vars", + TemplateSchemePod, + podBase, + &TemplateVarsByScheme{ + TaskExecution: TemplateVars{ + {testRegexes.Bar, "bar"}, + {testRegexes.Baz, "baz"}, + }, + }, + nil, + nil, + TemplateVars{ + {testRegexes.Bar, "bar"}, + {testRegexes.Baz, "baz"}, + }, + }, + { + "task execution happy path", + TemplateSchemeTaskExecution, + taskExecutionBase, + nil, + TemplateVars{ + {defaultRegexes.LogName, "main_logs"}, + {defaultRegexes.TaskRetryAttempt, "0"}, + {defaultRegexes.TaskID, "my-task-name"}, + {defaultRegexes.TaskVersion, "1"}, + {defaultRegexes.TaskProject, "my-task-project"}, + {defaultRegexes.TaskDomain, "my-task-domain"}, + {defaultRegexes.NodeID, "n0"}, + {defaultRegexes.ExecutionName, "my-execution-name"}, + {defaultRegexes.ExecutionProject, "my-execution-project"}, + {defaultRegexes.ExecutionDomain, "my-execution-domain"}, + }, + nil, + nil, + }, + { + "task execution with extra vars", + TemplateSchemeTaskExecution, + taskExecutionBase, + &TemplateVarsByScheme{ + Common: TemplateVars{ + {testRegexes.Foo, "foo"}, + }, + TaskExecution: TemplateVars{ + {testRegexes.Bar, "bar"}, + {testRegexes.Baz, "baz"}, + }, + }, + nil, + TemplateVars{ + {testRegexes.Foo, "foo"}, + {testRegexes.Bar, "bar"}, + {testRegexes.Baz, "baz"}, + }, + nil, + }, + { + "task execution with unused extra vars", + TemplateSchemeTaskExecution, + taskExecutionBase, + &TemplateVarsByScheme{ + Pod: TemplateVars{ + {testRegexes.Bar, "bar"}, + {testRegexes.Baz, "baz"}, + }, + }, + nil, + nil, + TemplateVars{ + {testRegexes.Bar, "bar"}, + {testRegexes.Baz, "baz"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + base := tt.baseVars + base.ExtraTemplateVarsByScheme = tt.extraVars + got := base.templateVarsForScheme(tt.scheme) + if tt.exact != nil { + assert.Equal(t, got, tt.exact) + } + if tt.contains != nil { + for _, c := range tt.contains { + assert.Contains(t, got, c) + } + } + if tt.notContains != nil { + for _, c := range tt.notContains { + assert.NotContains(t, got, c) + } + } + }) + } +} + func Test_templateLogPlugin_Regression(t *testing.T) { type fields struct { templateURI string From 06b5e9401e98e860c9cb7481d0efb496df133b88 Mon Sep 17 00:00:00 2001 From: Jeev B Date: Fri, 14 Jul 2023 13:32:16 -0700 Subject: [PATCH 22/22] update for consistency --- go/tasks/logs/logging_utils_test.go | 10 +++++----- .../pluginmachinery/tasklog/template_test.go | 20 +++++++++---------- .../common/common_operator_test.go | 8 ++++---- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/go/tasks/logs/logging_utils_test.go b/go/tasks/logs/logging_utils_test.go index aa23ec9a1..ab44ed213 100755 --- a/go/tasks/logs/logging_utils_test.go +++ b/go/tasks/logs/logging_utils_test.go @@ -19,16 +19,16 @@ var dummyTaskExecID = &core.TaskExecutionIdentifier{ TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, Name: "my-task-name", - Project: "my-project", - Domain: "my-domain", + Project: "my-task-project", + Domain: "my-task-domain", Version: "1", }, NodeExecutionId: &core.NodeExecutionIdentifier{ NodeId: "n0", ExecutionId: &core.WorkflowExecutionIdentifier{ Name: "my-execution-name", - Project: "my-project", - Domain: "my-domain", + Project: "my-execution-project", + Domain: "my-execution-domain", }, }, RetryAttempt: 1, @@ -337,7 +337,7 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { Name: "StackDriver my-Suffix", }, { - Uri: "https://flyte.corp.net/console/projects/my-project/domains/my-domain/executions/my-execution-name/nodeId/n0/taskId/my-task-name/attempt/1/view/logs", + Uri: "https://flyte.corp.net/console/projects/my-execution-project/domains/my-execution-domain/executions/my-execution-name/nodeId/n0/taskId/my-task-name/attempt/1/view/logs", MessageFormat: core.TaskLog_JSON, Name: "Internal my-Suffix", }, diff --git a/go/tasks/pluginmachinery/tasklog/template_test.go b/go/tasks/pluginmachinery/tasklog/template_test.go index dc8b5799b..72d12c4be 100644 --- a/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/go/tasks/pluginmachinery/tasklog/template_test.go @@ -488,16 +488,16 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, Name: "my-task-name", - Project: "my-project", - Domain: "my-domain", + Project: "my-task-project", + Domain: "my-task-domain", Version: "1", }, NodeExecutionId: &core.NodeExecutionIdentifier{ NodeId: "n0", ExecutionId: &core.WorkflowExecutionIdentifier{ Name: "my-execution-name", - Project: "my-project", - Domain: "my-domain", + Project: "my-execution-project", + Domain: "my-execution-domain", }, }, RetryAttempt: 0, @@ -507,7 +507,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { Output{ TaskLogs: []*core.TaskLog{ { - Uri: "https://flyte.corp.net/console/projects/my-project/domains/my-domain/executions/my-execution-name/nodeId/n0/taskId/my-task-name/attempt/0/view/logs", + Uri: "https://flyte.corp.net/console/projects/my-execution-project/domains/my-execution-domain/executions/my-execution-name/nodeId/n0/taskId/my-task-name/attempt/0/view/logs", MessageFormat: core.TaskLog_JSON, Name: "main_logs", }, @@ -538,16 +538,16 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, Name: "my-task-name", - Project: "my-project", - Domain: "my-domain", + Project: "my-task-project", + Domain: "my-task-domain", Version: "1", }, NodeExecutionId: &core.NodeExecutionIdentifier{ NodeId: "n0", ExecutionId: &core.WorkflowExecutionIdentifier{ Name: "my-execution-name", - Project: "my-project", - Domain: "my-domain", + Project: "my-execution-project", + Domain: "my-execution-domain", }, }, RetryAttempt: 0, @@ -564,7 +564,7 @@ func TestTemplateLogPlugin_NewTaskLog(t *testing.T) { Output{ TaskLogs: []*core.TaskLog{ { - Uri: "https://flyte.corp.net/console/projects/my-project/domains/my-domain/executions/my-execution-name/nodeId/n0/taskId/my-task-name/attempt/0/mappedIndex/1/mappedAttempt/1/view/logs", + Uri: "https://flyte.corp.net/console/projects/my-execution-project/domains/my-execution-domain/executions/my-execution-name/nodeId/n0/taskId/my-task-name/attempt/0/mappedIndex/1/mappedAttempt/1/view/logs", MessageFormat: core.TaskLog_JSON, Name: "main_logs", }, diff --git a/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go b/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go index d0b02ff9b..cd92557f3 100644 --- a/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go +++ b/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go @@ -329,15 +329,15 @@ func dummyTaskContext() pluginsCore.TaskExecutionContext { TaskId: &core.Identifier{ ResourceType: core.ResourceType_TASK, Name: "my-task-name", - Project: "my-project", - Domain: "my-domain", + Project: "my-task-project", + Domain: "my-task-domain", Version: "1", }, NodeExecutionId: &core.NodeExecutionIdentifier{ ExecutionId: &core.WorkflowExecutionIdentifier{ Name: "my-execution-name", - Project: "my-project", - Domain: "my-domain", + Project: "my-execution-project", + Domain: "my-execution-domain", }, }, RetryAttempt: 0,