From 785599ba6286d60d4396e2c00df2a6b7d6cac884 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Fri, 29 Dec 2023 10:00:43 -0300 Subject: [PATCH 01/19] Revert "Add Flyin propeller config (#4610)" This reverts commit 35016759a3848b27d604d872e9a79bfd89e4ab53. Signed-off-by: Eduardo Apolinario --- flyteplugins/go/tasks/logs/config.go | 3 - flyteplugins/go/tasks/logs/logconfig_flags.go | 2 - .../go/tasks/logs/logconfig_flags_test.go | 28 ---- flyteplugins/go/tasks/logs/logging_utils.go | 11 +- .../go/tasks/logs/logging_utils_test.go | 54 ++----- .../tasks/pluginmachinery/tasklog/plugin.go | 3 - .../tasks/pluginmachinery/tasklog/template.go | 27 ---- .../pluginmachinery/tasklog/template_test.go | 132 ------------------ .../tasklog/templatescheme_enumer.go | 11 +- .../go/tasks/plugins/k8s/pod/plugin.go | 7 +- 10 files changed, 20 insertions(+), 258 deletions(-) diff --git a/flyteplugins/go/tasks/logs/config.go b/flyteplugins/go/tasks/logs/config.go index b802844a4a..ca5a6012a8 100644 --- a/flyteplugins/go/tasks/logs/config.go +++ b/flyteplugins/go/tasks/logs/config.go @@ -28,9 +28,6 @@ type LogConfig struct { StackdriverLogResourceName string `json:"stackdriver-logresourcename" pflag:",Name of the logresource in stackdriver"` StackDriverTemplateURI tasklog.TemplateURI `json:"stackdriver-template-uri" pflag:",Template Uri to use when building stackdriver log links"` - IsFlyinEnabled bool `json:"flyin-enabled" pflag:",Enable Log-links to flyin logs"` - FlyinTemplateURI tasklog.TemplateURI `json:"flyin-template-uri" pflag:",Template Uri to use when building flyin log links"` - Templates []tasklog.TemplateLogPlugin `json:"templates" pflag:"-,"` } diff --git a/flyteplugins/go/tasks/logs/logconfig_flags.go b/flyteplugins/go/tasks/logs/logconfig_flags.go index de8ba022dc..00c08a8a58 100755 --- a/flyteplugins/go/tasks/logs/logconfig_flags.go +++ b/flyteplugins/go/tasks/logs/logconfig_flags.go @@ -61,7 +61,5 @@ func (cfg LogConfig) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "gcp-project"), DefaultConfig.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stackdriver-logresourcename"), DefaultConfig.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stackdriver-template-uri"), DefaultConfig.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "flyin-enabled"), DefaultConfig.IsFlyinEnabled, "Enable Log-links to flyin logs") - cmdFlags.String(fmt.Sprintf("%v%v", prefix, "flyin-template-uri"), DefaultConfig.FlyinTemplateURI, "Template Uri to use when building flyin log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/logs/logconfig_flags_test.go b/flyteplugins/go/tasks/logs/logconfig_flags_test.go index dfbee43c69..8bb775df1f 100755 --- a/flyteplugins/go/tasks/logs/logconfig_flags_test.go +++ b/flyteplugins/go/tasks/logs/logconfig_flags_test.go @@ -253,32 +253,4 @@ func TestLogConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_flyin-enabled", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("flyin-enabled", testValue) - if vBool, err := cmdFlags.GetBool("flyin-enabled"); err == nil { - testDecodeJson_LogConfig(t, fmt.Sprintf("%v", vBool), &actual.IsFlyinEnabled) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) - t.Run("Test_flyin-template-uri", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("flyin-template-uri", testValue) - if vString, err := cmdFlags.GetString("flyin-template-uri"); err == nil { - testDecodeJson_LogConfig(t, fmt.Sprintf("%v", vString), &actual.FlyinTemplateURI) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) } diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index 20f3522e27..4978109458 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -14,7 +14,7 @@ import ( ) // Internal -func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID pluginsCore.TaskExecutionID, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVarsByScheme *tasklog.TemplateVarsByScheme, taskTemplate *core.TaskTemplate) ([]*core.TaskLog, error) { +func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID pluginsCore.TaskExecutionID, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVarsByScheme *tasklog.TemplateVarsByScheme) ([]*core.TaskLog, error) { if logPlugin == nil { return nil, nil } @@ -51,7 +51,6 @@ func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, tas PodUnixFinishTime: finishTime, TaskExecutionID: taskExecID, ExtraTemplateVarsByScheme: extraLogTemplateVarsByScheme, - TaskTemplate: taskTemplate, }, ) @@ -109,14 +108,6 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { } } - if cfg.IsFlyinEnabled { - if len(cfg.FlyinTemplateURI) > 0 { - plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Flyin Logs", Scheme: tasklog.TemplateSchemeFlyin, TemplateURIs: []tasklog.TemplateURI{cfg.FlyinTemplateURI}, MessageFormat: core.TaskLog_JSON}) - } else { - plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Flyin Logs", Scheme: tasklog.TemplateSchemeFlyin, TemplateURIs: []tasklog.TemplateURI{fmt.Sprintf("https://flyin.%s/logs/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", cfg.GCPProjectName)}, MessageFormat: core.TaskLog_JSON}) - } - } - plugins = append(plugins, cfg.Templates...) return templateLogPluginCollection{plugins: plugins}, nil } diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index 46eb682201..91048eff16 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -44,7 +44,7 @@ func dummyTaskExecID() pluginCore.TaskExecutionID { func TestGetLogsForContainerInPod_NoPlugins(t *testing.T) { logPlugin, err := InitializeLogPlugins(&LogConfig{}) assert.NoError(t, err) - l, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), nil, 0, " Suffix", nil, nil) + l, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), nil, 0, " Suffix", nil) assert.NoError(t, err) assert.Nil(t, l) } @@ -56,7 +56,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", nil, nil) + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), nil, 0, " Suffix", nil) assert.NoError(t, err) assert.Nil(t, p) } @@ -87,7 +87,7 @@ func TestGetLogsForContainerInPod_BadIndex(t *testing.T) { } pod.Name = podName - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 1, " Suffix", nil, nil) + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 1, " Suffix", nil) assert.NoError(t, err) assert.Nil(t, p) } @@ -112,7 +112,7 @@ func TestGetLogsForContainerInPod_MissingStatus(t *testing.T) { } pod.Name = podName - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 1, " Suffix", nil, nil) + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 1, " Suffix", nil) assert.NoError(t, err) assert.Nil(t, p) } @@ -142,7 +142,7 @@ func TestGetLogsForContainerInPod_Cloudwatch(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil, nil) + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil) assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -172,7 +172,7 @@ func TestGetLogsForContainerInPod_K8s(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil, nil) + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil) assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -205,7 +205,7 @@ func TestGetLogsForContainerInPod_All(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil, nil) + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil) assert.Nil(t, err) assert.Len(t, logs, 2) } @@ -236,7 +236,7 @@ func TestGetLogsForContainerInPod_Stackdriver(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil, nil) + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil) assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -252,7 +252,7 @@ func TestGetLogsForContainerInPod_LegacyTemplate(t *testing.T) { IsStackDriverEnabled: true, StackDriverTemplateURI: "https://sd-my-log-server/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", - }, nil, []*core.TaskLog{ + }, []*core.TaskLog{ { Uri: "https://k8s-my-log-server/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, @@ -275,7 +275,7 @@ func TestGetLogsForContainerInPod_LegacyTemplate(t *testing.T) { assertTestSucceeded(t, &LogConfig{ IsStackDriverEnabled: true, StackDriverTemplateURI: "https://sd-my-log-server/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", - }, nil, []*core.TaskLog{ + }, []*core.TaskLog{ { Uri: "https://sd-my-log-server/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, @@ -285,7 +285,7 @@ func TestGetLogsForContainerInPod_LegacyTemplate(t *testing.T) { }) } -func assertTestSucceeded(tb testing.TB, config *LogConfig, taskTemplate *core.TaskTemplate, expectedTaskLogs []*core.TaskLog) { +func assertTestSucceeded(tb testing.TB, config *LogConfig, expectedTaskLogs []*core.TaskLog) { logPlugin, err := InitializeLogPlugins(config) assert.NoError(tb, err) @@ -310,7 +310,7 @@ func assertTestSucceeded(tb testing.TB, config *LogConfig, taskTemplate *core.Ta }, } - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " my-Suffix", nil, taskTemplate) + 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 { @@ -337,7 +337,7 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { Scheme: tasklog.TemplateSchemeTaskExecution, }, }, - }, nil, []*core.TaskLog{ + }, []*core.TaskLog{ { Uri: "https://my-log-server/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, @@ -350,31 +350,3 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { }, }) } - -func TestGetLogsForContainerInPod_Flyin(t *testing.T) { - assertTestSucceeded(t, - &LogConfig{ - IsKubernetesEnabled: true, - KubernetesTemplateURI: "https://k8s.com", - IsFlyinEnabled: true, - FlyinTemplateURI: "https://flyin.mydomain.com:{{ .port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", - }, - &core.TaskTemplate{ - Config: map[string]string{ - "link_type": "vscode", - "port": "65535", - }, - }, - []*core.TaskLog{ - { - Uri: "https://k8s.com", - MessageFormat: core.TaskLog_JSON, - Name: "Kubernetes Logs my-Suffix", - }, - { - Uri: "https://flyin.mydomain.com:65535/my-namespace/my-pod/ContainerName/ContainerID", - MessageFormat: core.TaskLog_JSON, - Name: "Flyin Logs my-Suffix", - }, - }) -} diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go index da2357a6d9..c43da02e58 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go @@ -14,7 +14,6 @@ type TemplateScheme int const ( TemplateSchemePod TemplateScheme = iota TemplateSchemeTaskExecution - TemplateSchemeFlyin ) // TemplateURI is a URI that accepts templates. See: go/tasks/pluginmachinery/tasklog/template.go for available templates. @@ -31,7 +30,6 @@ type TemplateVarsByScheme struct { Common TemplateVars Pod TemplateVars TaskExecution TemplateVars - Flyin TemplateVars } // Input contains all available information about task's execution that a log plugin can use to construct task's @@ -50,7 +48,6 @@ type Input struct { PodUID string TaskExecutionID pluginsCore.TaskExecutionID ExtraTemplateVarsByScheme *TemplateVarsByScheme - TaskTemplate *core.TaskTemplate } // Output contains all task logs a plugin generates for a given Input. diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index ea5c5f373c..750b1972df 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -35,7 +35,6 @@ type templateRegexes struct { ExecutionProject *regexp.Regexp ExecutionDomain *regexp.Regexp GeneratedName *regexp.Regexp - Port *regexp.Regexp } func initDefaultRegexes() templateRegexes { @@ -61,7 +60,6 @@ func initDefaultRegexes() templateRegexes { MustCreateRegex("executionProject"), MustCreateRegex("executionDomain"), MustCreateRegex("generatedName"), - MustCreateRegex("port"), } } @@ -87,16 +85,6 @@ func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { } switch scheme { - case TemplateSchemeFlyin: - port := input.TaskTemplate.GetConfig()["port"] - if port == "" { - port = "8080" - } - vars = append( - vars, - TemplateVar{defaultRegexes.Port, port}, - ) - fallthrough 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. @@ -193,22 +181,7 @@ func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { templateVars := input.templateVarsForScheme(p.Scheme) taskLogs := make([]*core.TaskLog, 0, len(p.TemplateURIs)) - - // Grab metadata from task template and check if key "link_type" is set to "vscode". - // If so, add a vscode link to the task logs. - isFlyin := false - if input.TaskTemplate != nil && input.TaskTemplate.GetConfig() != nil { - config := input.TaskTemplate.GetConfig() - if config != nil && config["link_type"] == "vscode" { - isFlyin = true - } - } for _, templateURI := range p.TemplateURIs { - // Skip Flyin logs if plugin is enabled but no metadata is defined in input's task template. - // This is to prevent Flyin logs from being generated for tasks that don't have a Flyin metadata section. - if p.DisplayName == "Flyin Logs" && !isFlyin { - continue - } taskLogs = append(taskLogs, &core.TaskLog{ Uri: replaceAll(templateURI, templateVars), Name: p.DisplayName + input.LogName, diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go index ad6eef25a3..f279707a3b 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go @@ -74,24 +74,6 @@ func Test_Input_templateVarsForScheme(t *testing.T) { LogName: "main_logs", TaskExecutionID: dummyTaskExecID(), } - flyinBase := 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, - TaskTemplate: &core.TaskTemplate{ - Config: map[string]string{ - "port": "1234", - }, - }, - } tests := []struct { name string @@ -220,50 +202,6 @@ func Test_Input_templateVarsForScheme(t *testing.T) { {testRegexes.Baz, "baz"}, }, }, - { - "flyin happy path", - TemplateSchemeFlyin, - flyinBase, - nil, - nil, - TemplateVars{ - {defaultRegexes.Port, "1234"}, - }, - nil, - }, - { - "flyin and pod happy path", - TemplateSchemeFlyin, - flyinBase, - nil, - TemplateVars{ - {defaultRegexes.LogName, "main_logs"}, - {defaultRegexes.Port, "1234"}, - {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 port not affected", - TemplateSchemePod, - podBase, - nil, - nil, - nil, - TemplateVars{ - {defaultRegexes.Port, "1234"}, - }, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -536,76 +474,6 @@ func TestTemplateLogPlugin(t *testing.T) { }, }, }, - { - "flyin", - TemplateLogPlugin{ - Scheme: TemplateSchemeFlyin, - TemplateURIs: []TemplateURI{"vscode://flyin:{{ .port }}/{{ .podName }}"}, - MessageFormat: core.TaskLog_JSON, - }, - args{ - input: Input{ - PodName: "my-pod-name", - TaskTemplate: &core.TaskTemplate{ - Config: map[string]string{ - "link_type": "vscode", - "port": "1234", - }, - }, - }, - }, - Output{ - TaskLogs: []*core.TaskLog{ - { - Uri: "vscode://flyin:1234/my-pod-name", - MessageFormat: core.TaskLog_JSON, - }, - }, - }, - }, - { - "flyin - default port", - TemplateLogPlugin{ - Scheme: TemplateSchemeFlyin, - TemplateURIs: []TemplateURI{"vscode://flyin:{{ .port }}/{{ .podName }}"}, - MessageFormat: core.TaskLog_JSON, - }, - args{ - input: Input{ - PodName: "my-pod-name", - TaskTemplate: &core.TaskTemplate{ - Config: map[string]string{ - "link_type": "vscode", - }, - }, - }, - }, - Output{ - TaskLogs: []*core.TaskLog{ - { - Uri: "vscode://flyin:8080/my-pod-name", - MessageFormat: core.TaskLog_JSON, - }, - }, - }, - }, - { - "flyin - no link_type in task template", - TemplateLogPlugin{ - Scheme: TemplateSchemeFlyin, - TemplateURIs: []TemplateURI{"vscode://flyin:{{ .port }}/{{ .podName }}"}, - MessageFormat: core.TaskLog_JSON, - DisplayName: "Flyin Logs", - }, - args{ - input: Input{ - PodName: "my-pod-name", - }, - }, - Output{ - TaskLogs: []*core.TaskLog{}, - }, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go index c1f4d668c0..70f15faf01 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go @@ -7,9 +7,9 @@ import ( "fmt" ) -const _TemplateSchemeName = "PodTaskExecutionFlyin" +const _TemplateSchemeName = "PodTaskExecution" -var _TemplateSchemeIndex = [...]uint8{0, 3, 16, 21} +var _TemplateSchemeIndex = [...]uint8{0, 3, 16} func (i TemplateScheme) String() string { if i < 0 || i >= TemplateScheme(len(_TemplateSchemeIndex)-1) { @@ -18,12 +18,11 @@ func (i TemplateScheme) String() string { return _TemplateSchemeName[_TemplateSchemeIndex[i]:_TemplateSchemeIndex[i+1]] } -var _TemplateSchemeValues = []TemplateScheme{0, 1, 2} +var _TemplateSchemeValues = []TemplateScheme{0, 1} var _TemplateSchemeNameToValueMap = map[string]TemplateScheme{ - _TemplateSchemeName[0:3]: 0, - _TemplateSchemeName[3:16]: 1, - _TemplateSchemeName[16:21]: 2, + _TemplateSchemeName[0:3]: 0, + _TemplateSchemeName[3:16]: 1, } // TemplateSchemeString retrieves an enum value from the enum constants string name. diff --git a/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go b/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go index eae0ac98b7..b266a6f5e8 100644 --- a/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go +++ b/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go @@ -153,11 +153,6 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin return pluginsCore.PhaseInfoUndefined, err } - taskTemplate, err := pluginContext.TaskReader().Read(ctx) - if err != nil { - return pluginsCore.PhaseInfoUndefined, err - } - pod := r.(*v1.Pod) transitionOccurredAt := flytek8s.GetLastTransitionOccurredAt(pod).Time @@ -173,7 +168,7 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin taskExecID := pluginContext.TaskExecutionMetadata().GetTaskExecutionID() if pod.Status.Phase != v1.PodPending && pod.Status.Phase != v1.PodUnknown { - taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, taskExecID, pod, 0, logSuffix, extraLogTemplateVarsByScheme, taskTemplate) + taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, taskExecID, pod, 0, logSuffix, extraLogTemplateVarsByScheme) if err != nil { return pluginsCore.PhaseInfoUndefined, err } From 40bcac03de08011745196591a6dfe4bb63c2e4dc Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Fri, 29 Dec 2023 12:00:21 -0300 Subject: [PATCH 02/19] wip Signed-off-by: Eduardo Apolinario --- ...uild github.com_flyteorg_flyte_cmd.run.xml | 16 ++++ flyte-single-binary-local.yaml | 3 + flyteplugins/go/tasks/logs/config.go | 8 ++ flyteplugins/go/tasks/logs/logconfig_flags.go | 1 + .../go/tasks/logs/logconfig_flags_test.go | 14 +++ flyteplugins/go/tasks/logs/logging_utils.go | 15 +++- .../pluginmachinery/core/phase_enumer.go | 7 +- .../tasks/pluginmachinery/tasklog/plugin.go | 3 + .../tasks/pluginmachinery/tasklog/template.go | 44 ++++++++++ .../tasklog/templatescheme_enumer.go | 11 +-- .../webapi/mocks/sync_plugin.go | 88 +++++++++++++++++++ .../tasks/plugins/array/core/phase_enumer.go | 7 +- .../tasks/plugins/array/k8s/config_flags.go | 1 + .../plugins/array/k8s/config_flags_test.go | 14 +++ .../go/tasks/plugins/k8s/pod/plugin.go | 7 +- .../tasks/plugins/k8s/spark/config_flags.go | 4 + .../plugins/k8s/spark/config_flags_test.go | 56 ++++++++++++ 17 files changed, 284 insertions(+), 15 deletions(-) create mode 100644 .run/go build github.com_flyteorg_flyte_cmd.run.xml create mode 100644 flyteplugins/go/tasks/pluginmachinery/webapi/mocks/sync_plugin.go diff --git a/.run/go build github.com_flyteorg_flyte_cmd.run.xml b/.run/go build github.com_flyteorg_flyte_cmd.run.xml new file mode 100644 index 0000000000..a980caaee5 --- /dev/null +++ b/.run/go build github.com_flyteorg_flyte_cmd.run.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/flyte-single-binary-local.yaml b/flyte-single-binary-local.yaml index ca63458b03..4eecc5d41d 100644 --- a/flyte-single-binary-local.yaml +++ b/flyte-single-binary-local.yaml @@ -66,6 +66,9 @@ plugins: kubernetes-template-uri: http://localhost:30080/kubernetes-dashboard/#/log/{{.namespace }}/{{ .podName }}/pod?namespace={{ .namespace }} cloudwatch-enabled: false stackdriver-enabled: false + dynamic-log-links-enabled: true + dynamic-log-links: + flyin: http://flyin.vs:{{ .taskConfig.port }}/ database: postgres: diff --git a/flyteplugins/go/tasks/logs/config.go b/flyteplugins/go/tasks/logs/config.go index ca5a6012a8..d17bcc660e 100644 --- a/flyteplugins/go/tasks/logs/config.go +++ b/flyteplugins/go/tasks/logs/config.go @@ -7,6 +7,11 @@ import ( //go:generate pflags LogConfig --default-var=DefaultConfig + +type DynamicLogLinks struct { + Flyin tasklog.TemplateURI `json:"flyin" pflag:"-,Template Uri to use when building flyin log links"` +} + // LogConfig encapsulates plugins' log configs type LogConfig struct { IsCloudwatchEnabled bool `json:"cloudwatch-enabled" pflag:",Enable Cloudwatch Logging"` @@ -28,6 +33,9 @@ type LogConfig struct { StackdriverLogResourceName string `json:"stackdriver-logresourcename" pflag:",Name of the logresource in stackdriver"` StackDriverTemplateURI tasklog.TemplateURI `json:"stackdriver-template-uri" pflag:",Template Uri to use when building stackdriver log links"` + IsDynamicLogLinksEnabled bool `json:"dynamic-log-links-enabled" pflag:",Enable dynamic log links"` + DynamicLogLinks DynamicLogLinks `json:"dynamic-log-links" pflag:",Map of dynamic log links"` + Templates []tasklog.TemplateLogPlugin `json:"templates" pflag:"-,"` } diff --git a/flyteplugins/go/tasks/logs/logconfig_flags.go b/flyteplugins/go/tasks/logs/logconfig_flags.go index 00c08a8a58..8f0e560100 100755 --- a/flyteplugins/go/tasks/logs/logconfig_flags.go +++ b/flyteplugins/go/tasks/logs/logconfig_flags.go @@ -61,5 +61,6 @@ func (cfg LogConfig) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "gcp-project"), DefaultConfig.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stackdriver-logresourcename"), DefaultConfig.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stackdriver-template-uri"), DefaultConfig.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") + cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "dynamic-log-links-enabled"), DefaultConfig.IsDynamicLogLinksEnabled, "Enable dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/logs/logconfig_flags_test.go b/flyteplugins/go/tasks/logs/logconfig_flags_test.go index 8bb775df1f..004ff8dfd3 100755 --- a/flyteplugins/go/tasks/logs/logconfig_flags_test.go +++ b/flyteplugins/go/tasks/logs/logconfig_flags_test.go @@ -253,4 +253,18 @@ func TestLogConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_dynamic-log-links-enabled", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("dynamic-log-links-enabled", testValue) + if vBool, err := cmdFlags.GetBool("dynamic-log-links-enabled"); err == nil { + testDecodeJson_LogConfig(t, fmt.Sprintf("%v", vBool), &actual.IsDynamicLogLinksEnabled) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) } diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index 4978109458..41706c99fe 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -14,7 +14,7 @@ import ( ) // Internal -func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID pluginsCore.TaskExecutionID, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVarsByScheme *tasklog.TemplateVarsByScheme) ([]*core.TaskLog, error) { +func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID pluginsCore.TaskExecutionID, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVarsByScheme *tasklog.TemplateVarsByScheme, taskTemplate *core.TaskTemplate) ([]*core.TaskLog, error) { if logPlugin == nil { return nil, nil } @@ -51,6 +51,7 @@ func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, tas PodUnixFinishTime: finishTime, TaskExecutionID: taskExecID, ExtraTemplateVarsByScheme: extraLogTemplateVarsByScheme, + TaskTemplate: taskTemplate, }, ) @@ -62,7 +63,8 @@ func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, tas } type templateLogPluginCollection struct { - plugins []tasklog.TemplateLogPlugin + plugins []tasklog.TemplateLogPlugin + dynamicPlugins []tasklog.TemplateLogPlugin } func (t templateLogPluginCollection) GetTaskLogs(input tasklog.Input) (tasklog.Output, error) { @@ -83,6 +85,7 @@ func (t templateLogPluginCollection) GetTaskLogs(input tasklog.Input) (tasklog.O func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { // Use a list to maintain order. var plugins []tasklog.TemplateLogPlugin + var dynamicPlugins []tasklog.TemplateLogPlugin if cfg.IsKubernetesEnabled { if len(cfg.KubernetesTemplateURI) > 0 { @@ -108,6 +111,12 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { } } + if cfg.IsDynamicLogLinksEnabled { + if len(cfg.DynamicLogLinks.Flyin) > 0 { + dynamicPlugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Flyin Logs", Scheme: tasklog.TemplateSchemeDynamic, TemplateURIs: []tasklog.TemplateURI{cfg.DynamicLogLinks.Flyin}, MessageFormat: core.TaskLog_JSON}) + } + } + plugins = append(plugins, cfg.Templates...) - return templateLogPluginCollection{plugins: plugins}, nil + return templateLogPluginCollection{plugins: plugins, dynamicPlugins: dynamicPlugins}, nil } diff --git a/flyteplugins/go/tasks/pluginmachinery/core/phase_enumer.go b/flyteplugins/go/tasks/pluginmachinery/core/phase_enumer.go index bd26c4d560..caa4d86250 100644 --- a/flyteplugins/go/tasks/pluginmachinery/core/phase_enumer.go +++ b/flyteplugins/go/tasks/pluginmachinery/core/phase_enumer.go @@ -6,9 +6,9 @@ import ( "fmt" ) -const _PhaseName = "PhaseUndefinedPhaseNotReadyPhaseWaitingForResourcesPhaseQueuedPhaseInitializingPhaseRunningPhaseSuccessPhaseRetryableFailurePhasePermanentFailurePhaseWaitingForCache" +const _PhaseName = "PhaseUndefinedPhaseNotReadyPhaseWaitingForResourcesPhaseQueuedPhaseInitializingPhaseRunningPhaseSuccessPhaseRetryableFailurePhasePermanentFailurePhaseWaitingForCachePhaseAborted" -var _PhaseIndex = [...]uint8{0, 14, 27, 51, 62, 79, 91, 103, 124, 145, 165} +var _PhaseIndex = [...]uint8{0, 14, 27, 51, 62, 79, 91, 103, 124, 145, 165, 177} func (i Phase) String() string { if i < 0 || i >= Phase(len(_PhaseIndex)-1) { @@ -17,7 +17,7 @@ func (i Phase) String() string { return _PhaseName[_PhaseIndex[i]:_PhaseIndex[i+1]] } -var _PhaseValues = []Phase{0, 1, 2, 3, 4, 5, 6, 7, 8, 9} +var _PhaseValues = []Phase{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10} var _PhaseNameToValueMap = map[string]Phase{ _PhaseName[0:14]: 0, @@ -30,6 +30,7 @@ var _PhaseNameToValueMap = map[string]Phase{ _PhaseName[103:124]: 7, _PhaseName[124:145]: 8, _PhaseName[145:165]: 9, + _PhaseName[165:177]: 10, } // PhaseString retrieves an enum value from the enum constants string name. diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go index c43da02e58..d7d21b1e7d 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go @@ -14,6 +14,7 @@ type TemplateScheme int const ( TemplateSchemePod TemplateScheme = iota TemplateSchemeTaskExecution + TemplateSchemeDynamic ) // TemplateURI is a URI that accepts templates. See: go/tasks/pluginmachinery/tasklog/template.go for available templates. @@ -48,6 +49,7 @@ type Input struct { PodUID string TaskExecutionID pluginsCore.TaskExecutionID ExtraTemplateVarsByScheme *TemplateVarsByScheme + TaskTemplate *core.TaskTemplate } // Output contains all task logs a plugin generates for a given Input. @@ -64,6 +66,7 @@ type Plugin interface { type TemplateLogPlugin 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."` + DynamicTemplateURIs []TemplateURI `json:"dynamicTemplateUris" pflag:",Dynamic URI Templates for generating task log links."` MessageFormat core.TaskLog_MessageFormat `json:"messageFormat" pflag:",Log Message Format."` Scheme TemplateScheme `json:"scheme" pflag:",Templating scheme to use. Supported values are Pod and TaskExecution."` } diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index 750b1972df..04980dc3aa 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -10,9 +10,15 @@ import ( ) func MustCreateRegex(varName string) *regexp.Regexp { + // TODO: Is this regex correct? Why are we allowing $? Shouldn't it be {{\s*\.%s\s*}}? + // Also, case insensitive matching is enabled. Is this correct? return regexp.MustCompile(fmt.Sprintf(`(?i){{\s*[\.$]%s\s*}}`, varName)) } +func MustCreateDynamicLogRegex(varName string) *regexp.Regexp { + return regexp.MustCompile(fmt.Sprintf(`(?i){{\s*.taskConfig[\.$]%s\s*}}`, varName)) +} + type templateRegexes struct { LogName *regexp.Regexp PodName *regexp.Regexp @@ -35,6 +41,7 @@ type templateRegexes struct { ExecutionProject *regexp.Regexp ExecutionDomain *regexp.Regexp GeneratedName *regexp.Regexp + Port *regexp.Regexp } func initDefaultRegexes() templateRegexes { @@ -60,6 +67,7 @@ func initDefaultRegexes() templateRegexes { MustCreateRegex("executionProject"), MustCreateRegex("executionDomain"), MustCreateRegex("generatedName"), + MustCreateDynamicLogRegex("port"), } } @@ -85,6 +93,16 @@ func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { } switch scheme { + case TemplateSchemeDynamic: + port := input.TaskTemplate.GetConfig()["port"] + if port == "" { + port = "8080" + } + vars = append( + vars, + TemplateVar{defaultRegexes.Port, port}, + ) + fallthrough 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. @@ -178,6 +196,21 @@ func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { return vars } +func hasDynamicLogLinks(taskTemplate *core.TaskTemplate) bool { + if taskTemplate == nil { + return false + } + config := taskTemplate.GetConfig() + if config == nil { + return false + } + // TODO: establish a better protocol to handle dynamic log links. + // One idea is to have a comma-separated list of dynamic log link types + // in the task template. + // NB: we are going to grandfather in the flyin way of doing things for now. + return config["link_type"] == "vscode" +} + func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { templateVars := input.templateVarsForScheme(p.Scheme) taskLogs := make([]*core.TaskLog, 0, len(p.TemplateURIs)) @@ -189,5 +222,16 @@ func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { }) } + if hasDynamicLogLinks(input.TaskTemplate) { + // TODO: Only emit dynamic log links enabled in the task template. + for _, templateURI := range p.DynamicTemplateURIs { + taskLogs = append(taskLogs, &core.TaskLog{ + Uri: replaceAll(templateURI, templateVars), + Name: p.DisplayName + input.LogName, + MessageFormat: p.MessageFormat, + }) + } + } + return Output{TaskLogs: taskLogs}, nil } diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go index 70f15faf01..d942a34344 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go @@ -7,9 +7,9 @@ import ( "fmt" ) -const _TemplateSchemeName = "PodTaskExecution" +const _TemplateSchemeName = "PodTaskExecutionDynamic" -var _TemplateSchemeIndex = [...]uint8{0, 3, 16} +var _TemplateSchemeIndex = [...]uint8{0, 3, 16, 23} func (i TemplateScheme) String() string { if i < 0 || i >= TemplateScheme(len(_TemplateSchemeIndex)-1) { @@ -18,11 +18,12 @@ func (i TemplateScheme) String() string { return _TemplateSchemeName[_TemplateSchemeIndex[i]:_TemplateSchemeIndex[i+1]] } -var _TemplateSchemeValues = []TemplateScheme{0, 1} +var _TemplateSchemeValues = []TemplateScheme{0, 1, 2} var _TemplateSchemeNameToValueMap = map[string]TemplateScheme{ - _TemplateSchemeName[0:3]: 0, - _TemplateSchemeName[3:16]: 1, + _TemplateSchemeName[0:3]: 0, + _TemplateSchemeName[3:16]: 1, + _TemplateSchemeName[16:23]: 2, } // TemplateSchemeString retrieves an enum value from the enum constants string name. diff --git a/flyteplugins/go/tasks/pluginmachinery/webapi/mocks/sync_plugin.go b/flyteplugins/go/tasks/pluginmachinery/webapi/mocks/sync_plugin.go new file mode 100644 index 0000000000..cfe5d38090 --- /dev/null +++ b/flyteplugins/go/tasks/pluginmachinery/webapi/mocks/sync_plugin.go @@ -0,0 +1,88 @@ +// Code generated by mockery v1.0.1. DO NOT EDIT. + +package mocks + +import ( + context "context" + + core "github.com/flyteorg/flyte/flyteplugins/go/tasks/pluginmachinery/core" + mock "github.com/stretchr/testify/mock" + + webapi "github.com/flyteorg/flyte/flyteplugins/go/tasks/pluginmachinery/webapi" +) + +// SyncPlugin is an autogenerated mock type for the SyncPlugin type +type SyncPlugin struct { + mock.Mock +} + +type SyncPlugin_Do struct { + *mock.Call +} + +func (_m SyncPlugin_Do) Return(phase core.PhaseInfo, err error) *SyncPlugin_Do { + return &SyncPlugin_Do{Call: _m.Call.Return(phase, err)} +} + +func (_m *SyncPlugin) OnDo(ctx context.Context, tCtx webapi.TaskExecutionContext) *SyncPlugin_Do { + c_call := _m.On("Do", ctx, tCtx) + return &SyncPlugin_Do{Call: c_call} +} + +func (_m *SyncPlugin) OnDoMatch(matchers ...interface{}) *SyncPlugin_Do { + c_call := _m.On("Do", matchers...) + return &SyncPlugin_Do{Call: c_call} +} + +// Do provides a mock function with given fields: ctx, tCtx +func (_m *SyncPlugin) Do(ctx context.Context, tCtx webapi.TaskExecutionContext) (core.PhaseInfo, error) { + ret := _m.Called(ctx, tCtx) + + var r0 core.PhaseInfo + if rf, ok := ret.Get(0).(func(context.Context, webapi.TaskExecutionContext) core.PhaseInfo); ok { + r0 = rf(ctx, tCtx) + } else { + r0 = ret.Get(0).(core.PhaseInfo) + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, webapi.TaskExecutionContext) error); ok { + r1 = rf(ctx, tCtx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +type SyncPlugin_GetConfig struct { + *mock.Call +} + +func (_m SyncPlugin_GetConfig) Return(_a0 webapi.PluginConfig) *SyncPlugin_GetConfig { + return &SyncPlugin_GetConfig{Call: _m.Call.Return(_a0)} +} + +func (_m *SyncPlugin) OnGetConfig() *SyncPlugin_GetConfig { + c_call := _m.On("GetConfig") + return &SyncPlugin_GetConfig{Call: c_call} +} + +func (_m *SyncPlugin) OnGetConfigMatch(matchers ...interface{}) *SyncPlugin_GetConfig { + c_call := _m.On("GetConfig", matchers...) + return &SyncPlugin_GetConfig{Call: c_call} +} + +// GetConfig provides a mock function with given fields: +func (_m *SyncPlugin) GetConfig() webapi.PluginConfig { + ret := _m.Called() + + var r0 webapi.PluginConfig + if rf, ok := ret.Get(0).(func() webapi.PluginConfig); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(webapi.PluginConfig) + } + + return r0 +} diff --git a/flyteplugins/go/tasks/plugins/array/core/phase_enumer.go b/flyteplugins/go/tasks/plugins/array/core/phase_enumer.go index c659c7bcfe..a5cc4569bc 100644 --- a/flyteplugins/go/tasks/plugins/array/core/phase_enumer.go +++ b/flyteplugins/go/tasks/plugins/array/core/phase_enumer.go @@ -6,9 +6,9 @@ import ( "fmt" ) -const _PhaseName = "PhaseStartPhasePreLaunchPhaseLaunchPhaseWaitingForResourcesPhaseCheckingSubTaskExecutionsPhaseAssembleFinalOutputPhaseWriteToDiscoveryPhaseWriteToDiscoveryThenFailPhaseSuccessPhaseAssembleFinalErrorPhaseRetryableFailurePhasePermanentFailure" +const _PhaseName = "PhaseStartPhasePreLaunchPhaseLaunchPhaseWaitingForResourcesPhaseCheckingSubTaskExecutionsPhaseAssembleFinalOutputPhaseWriteToDiscoveryPhaseWriteToDiscoveryThenFailPhaseSuccessPhaseAssembleFinalErrorPhaseRetryableFailurePhasePermanentFailurePhaseAbortSubTasks" -var _PhaseIndex = [...]uint8{0, 10, 24, 35, 59, 89, 113, 134, 163, 175, 198, 219, 240} +var _PhaseIndex = [...]uint16{0, 10, 24, 35, 59, 89, 113, 134, 163, 175, 198, 219, 240, 258} func (i Phase) String() string { if i >= Phase(len(_PhaseIndex)-1) { @@ -17,7 +17,7 @@ func (i Phase) String() string { return _PhaseName[_PhaseIndex[i]:_PhaseIndex[i+1]] } -var _PhaseValues = []Phase{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11} +var _PhaseValues = []Phase{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12} var _PhaseNameToValueMap = map[string]Phase{ _PhaseName[0:10]: 0, @@ -32,6 +32,7 @@ var _PhaseNameToValueMap = map[string]Phase{ _PhaseName[175:198]: 9, _PhaseName[198:219]: 10, _PhaseName[219:240]: 11, + _PhaseName[240:258]: 12, } // PhaseString retrieves an enum value from the enum constants string name. diff --git a/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go b/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go index 1497f64bdc..22edf6e444 100755 --- a/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go @@ -70,5 +70,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.gcp-project"), defaultConfig.LogConfig.Config.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.stackdriver-logresourcename"), defaultConfig.LogConfig.Config.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.stackdriver-template-uri"), defaultConfig.LogConfig.Config.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") + cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.config.dynamic-log-links-enabled"), defaultConfig.LogConfig.Config.IsDynamicLogLinksEnabled, "Enable dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go b/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go index 3737c45c3b..fa40f5f9ec 100755 --- a/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go @@ -379,4 +379,18 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_logs.config.dynamic-log-links-enabled", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("logs.config.dynamic-log-links-enabled", testValue) + if vBool, err := cmdFlags.GetBool("logs.config.dynamic-log-links-enabled"); err == nil { + testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.LogConfig.Config.IsDynamicLogLinksEnabled) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) } diff --git a/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go b/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go index b266a6f5e8..eae0ac98b7 100644 --- a/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go +++ b/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go @@ -153,6 +153,11 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin return pluginsCore.PhaseInfoUndefined, err } + taskTemplate, err := pluginContext.TaskReader().Read(ctx) + if err != nil { + return pluginsCore.PhaseInfoUndefined, err + } + pod := r.(*v1.Pod) transitionOccurredAt := flytek8s.GetLastTransitionOccurredAt(pod).Time @@ -168,7 +173,7 @@ func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.Plugin taskExecID := pluginContext.TaskExecutionMetadata().GetTaskExecutionID() if pod.Status.Phase != v1.PodPending && pod.Status.Phase != v1.PodUnknown { - taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, taskExecID, pod, 0, logSuffix, extraLogTemplateVarsByScheme) + taskLogs, err := logs.GetLogsForContainerInPod(ctx, logPlugin, taskExecID, pod, 0, logSuffix, extraLogTemplateVarsByScheme, taskTemplate) if err != nil { return pluginsCore.PhaseInfoUndefined, err } diff --git a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go index d5d6945f71..d82cdd96ad 100755 --- a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go +++ b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go @@ -62,6 +62,7 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.gcp-project"), defaultConfig.LogConfig.Mixed.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.stackdriver-logresourcename"), defaultConfig.LogConfig.Mixed.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.stackdriver-template-uri"), defaultConfig.LogConfig.Mixed.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") + cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.mixed.dynamic-log-links-enabled"), defaultConfig.LogConfig.Mixed.IsDynamicLogLinksEnabled, "Enable dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-enabled"), defaultConfig.LogConfig.User.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-region"), defaultConfig.LogConfig.User.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-log-group"), defaultConfig.LogConfig.User.CloudwatchLogGroup, "Log group to which streams are associated.") @@ -73,6 +74,7 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.gcp-project"), defaultConfig.LogConfig.User.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.stackdriver-logresourcename"), defaultConfig.LogConfig.User.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.stackdriver-template-uri"), defaultConfig.LogConfig.User.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") + cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.user.dynamic-log-links-enabled"), defaultConfig.LogConfig.User.IsDynamicLogLinksEnabled, "Enable dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-enabled"), defaultConfig.LogConfig.System.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-region"), defaultConfig.LogConfig.System.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-log-group"), defaultConfig.LogConfig.System.CloudwatchLogGroup, "Log group to which streams are associated.") @@ -84,6 +86,7 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.gcp-project"), defaultConfig.LogConfig.System.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.stackdriver-logresourcename"), defaultConfig.LogConfig.System.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.stackdriver-template-uri"), defaultConfig.LogConfig.System.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") + cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.system.dynamic-log-links-enabled"), defaultConfig.LogConfig.System.IsDynamicLogLinksEnabled, "Enable dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-enabled"), defaultConfig.LogConfig.AllUser.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-region"), defaultConfig.LogConfig.AllUser.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-log-group"), defaultConfig.LogConfig.AllUser.CloudwatchLogGroup, "Log group to which streams are associated.") @@ -95,5 +98,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.gcp-project"), defaultConfig.LogConfig.AllUser.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.stackdriver-logresourcename"), defaultConfig.LogConfig.AllUser.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.stackdriver-template-uri"), defaultConfig.LogConfig.AllUser.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") + cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.all-user.dynamic-log-links-enabled"), defaultConfig.LogConfig.AllUser.IsDynamicLogLinksEnabled, "Enable dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go index ea8659a48a..ee45707d19 100755 --- a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go @@ -267,6 +267,20 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_logs.mixed.dynamic-log-links-enabled", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("logs.mixed.dynamic-log-links-enabled", testValue) + if vBool, err := cmdFlags.GetBool("logs.mixed.dynamic-log-links-enabled"); err == nil { + testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.LogConfig.Mixed.IsDynamicLogLinksEnabled) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) t.Run("Test_logs.user.cloudwatch-enabled", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -421,6 +435,20 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_logs.user.dynamic-log-links-enabled", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("logs.user.dynamic-log-links-enabled", testValue) + if vBool, err := cmdFlags.GetBool("logs.user.dynamic-log-links-enabled"); err == nil { + testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.LogConfig.User.IsDynamicLogLinksEnabled) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) t.Run("Test_logs.system.cloudwatch-enabled", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -575,6 +603,20 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_logs.system.dynamic-log-links-enabled", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("logs.system.dynamic-log-links-enabled", testValue) + if vBool, err := cmdFlags.GetBool("logs.system.dynamic-log-links-enabled"); err == nil { + testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.LogConfig.System.IsDynamicLogLinksEnabled) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) t.Run("Test_logs.all-user.cloudwatch-enabled", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -729,4 +771,18 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_logs.all-user.dynamic-log-links-enabled", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("logs.all-user.dynamic-log-links-enabled", testValue) + if vBool, err := cmdFlags.GetBool("logs.all-user.dynamic-log-links-enabled"); err == nil { + testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.LogConfig.AllUser.IsDynamicLogLinksEnabled) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) } From 930564eae054d32cb22555356be28533090492e4 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Mon, 22 Jan 2024 15:07:25 -0800 Subject: [PATCH 03/19] Add tests Signed-off-by: Eduardo Apolinario --- flyteplugins/go/tasks/logs/config.go | 5 +- flyteplugins/go/tasks/logs/logging_utils.go | 4 +- .../go/tasks/logs/logging_utils_test.go | 147 ++++++++++++++++-- .../tasks/pluginmachinery/tasklog/plugin.go | 12 +- .../pluginmachinery/tasklog/template_test.go | 133 ++++++++++++++++ 5 files changed, 277 insertions(+), 24 deletions(-) diff --git a/flyteplugins/go/tasks/logs/config.go b/flyteplugins/go/tasks/logs/config.go index d17bcc660e..7422108a25 100644 --- a/flyteplugins/go/tasks/logs/config.go +++ b/flyteplugins/go/tasks/logs/config.go @@ -7,7 +7,6 @@ import ( //go:generate pflags LogConfig --default-var=DefaultConfig - type DynamicLogLinks struct { Flyin tasklog.TemplateURI `json:"flyin" pflag:"-,Template Uri to use when building flyin log links"` } @@ -33,8 +32,8 @@ type LogConfig struct { StackdriverLogResourceName string `json:"stackdriver-logresourcename" pflag:",Name of the logresource in stackdriver"` StackDriverTemplateURI tasklog.TemplateURI `json:"stackdriver-template-uri" pflag:",Template Uri to use when building stackdriver log links"` - IsDynamicLogLinksEnabled bool `json:"dynamic-log-links-enabled" pflag:",Enable dynamic log links"` - DynamicLogLinks DynamicLogLinks `json:"dynamic-log-links" pflag:",Map of dynamic log links"` + IsDynamicLogLinksEnabled bool `json:"dynamic-log-links-enabled" pflag:",Enable dynamic log links"` + DynamicLogLinks DynamicLogLinks `json:"dynamic-log-links" pflag:",Map of dynamic log links"` Templates []tasklog.TemplateLogPlugin `json:"templates" pflag:"-,"` } diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index 41706c99fe..9587813852 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -70,7 +70,7 @@ type templateLogPluginCollection struct { func (t templateLogPluginCollection) GetTaskLogs(input tasklog.Input) (tasklog.Output, error) { var taskLogs []*core.TaskLog - for _, plugin := range t.plugins { + for _, plugin := range append(t.plugins, t.dynamicPlugins...) { o, err := plugin.GetTaskLogs(input) if err != nil { return tasklog.Output{}, err @@ -113,7 +113,7 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { if cfg.IsDynamicLogLinksEnabled { if len(cfg.DynamicLogLinks.Flyin) > 0 { - dynamicPlugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Flyin Logs", Scheme: tasklog.TemplateSchemeDynamic, TemplateURIs: []tasklog.TemplateURI{cfg.DynamicLogLinks.Flyin}, MessageFormat: core.TaskLog_JSON}) + dynamicPlugins = append(dynamicPlugins, tasklog.TemplateLogPlugin{DisplayName: "Flyin Logs", Scheme: tasklog.TemplateSchemeDynamic, DynamicTemplateURIs: []tasklog.TemplateURI{cfg.DynamicLogLinks.Flyin}, MessageFormat: core.TaskLog_JSON}) } } diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index 91048eff16..afc967b7a9 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -44,7 +44,7 @@ func dummyTaskExecID() pluginCore.TaskExecutionID { func TestGetLogsForContainerInPod_NoPlugins(t *testing.T) { logPlugin, err := InitializeLogPlugins(&LogConfig{}) assert.NoError(t, err) - l, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), nil, 0, " Suffix", nil) + l, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), nil, 0, " Suffix", nil, nil) assert.NoError(t, err) assert.Nil(t, l) } @@ -56,7 +56,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", nil) + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), nil, 0, " Suffix", nil, nil) assert.NoError(t, err) assert.Nil(t, p) } @@ -87,7 +87,7 @@ func TestGetLogsForContainerInPod_BadIndex(t *testing.T) { } pod.Name = podName - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 1, " Suffix", nil) + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 1, " Suffix", nil, nil) assert.NoError(t, err) assert.Nil(t, p) } @@ -112,7 +112,7 @@ func TestGetLogsForContainerInPod_MissingStatus(t *testing.T) { } pod.Name = podName - p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 1, " Suffix", nil) + p, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 1, " Suffix", nil, nil) assert.NoError(t, err) assert.Nil(t, p) } @@ -142,7 +142,7 @@ func TestGetLogsForContainerInPod_Cloudwatch(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil) + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil, nil) assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -172,7 +172,7 @@ func TestGetLogsForContainerInPod_K8s(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil) + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil, nil) assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -205,7 +205,7 @@ func TestGetLogsForContainerInPod_All(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil) + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil, nil) assert.Nil(t, err) assert.Len(t, logs, 2) } @@ -236,7 +236,7 @@ func TestGetLogsForContainerInPod_Stackdriver(t *testing.T) { } pod.Name = podName - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil) + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " Suffix", nil, nil) assert.Nil(t, err) assert.Len(t, logs, 1) } @@ -252,7 +252,7 @@ func TestGetLogsForContainerInPod_LegacyTemplate(t *testing.T) { IsStackDriverEnabled: true, StackDriverTemplateURI: "https://sd-my-log-server/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", - }, []*core.TaskLog{ + }, nil, []*core.TaskLog{ { Uri: "https://k8s-my-log-server/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, @@ -275,7 +275,7 @@ func TestGetLogsForContainerInPod_LegacyTemplate(t *testing.T) { assertTestSucceeded(t, &LogConfig{ IsStackDriverEnabled: true, StackDriverTemplateURI: "https://sd-my-log-server/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", - }, []*core.TaskLog{ + }, nil, []*core.TaskLog{ { Uri: "https://sd-my-log-server/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, @@ -285,7 +285,7 @@ func TestGetLogsForContainerInPod_LegacyTemplate(t *testing.T) { }) } -func assertTestSucceeded(tb testing.TB, config *LogConfig, expectedTaskLogs []*core.TaskLog) { +func assertTestSucceeded(tb testing.TB, config *LogConfig, taskTemplate *core.TaskTemplate, expectedTaskLogs []*core.TaskLog) { logPlugin, err := InitializeLogPlugins(config) assert.NoError(tb, err) @@ -310,7 +310,7 @@ func assertTestSucceeded(tb testing.TB, config *LogConfig, expectedTaskLogs []*c }, } - logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " my-Suffix", nil) + logs, err := GetLogsForContainerInPod(context.TODO(), logPlugin, dummyTaskExecID(), pod, 0, " my-Suffix", nil, taskTemplate) assert.Nil(tb, err) assert.Len(tb, logs, len(expectedTaskLogs)) if diff := deep.Equal(logs, expectedTaskLogs); len(diff) > 0 { @@ -337,7 +337,7 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { Scheme: tasklog.TemplateSchemeTaskExecution, }, }, - }, []*core.TaskLog{ + }, nil, []*core.TaskLog{ { Uri: "https://my-log-server/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, @@ -350,3 +350,124 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { }, }) } + +func TestGetLogsForContainerInPod_Flyin(t *testing.T) { + tests := []struct { + name string + config *LogConfig + template *core.TaskTemplate + expectedTaskLogs []*core.TaskLog + }{ + { + "Flyin enabled but no task template", + &LogConfig{ + IsDynamicLogLinksEnabled: true, + DynamicLogLinks: DynamicLogLinks{ + Flyin: "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + }, + }, + nil, + nil, + }, + { + "Flyin enabled but config not found in task template", + &LogConfig{ + IsDynamicLogLinksEnabled: true, + DynamicLogLinks: DynamicLogLinks{ + Flyin: "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + }, + }, + &core.TaskTemplate{}, + nil, + }, + { + "Flyin enabled but no port in task template", + &LogConfig{ + IsDynamicLogLinksEnabled: true, + DynamicLogLinks: DynamicLogLinks{ + Flyin: "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + }, + }, + &core.TaskTemplate{ + Config: map[string]string{ + "link_type": "vscode", + }, + }, + []*core.TaskLog{ + { + Uri: "https://flyin.mydomain.com:8080/my-namespace/my-pod/ContainerName/ContainerID", + MessageFormat: core.TaskLog_JSON, + Name: "Flyin Logs my-Suffix", + }, + }, + }, + { + "Flyin disabled but config present in TaskTemplate", + &LogConfig{ + IsDynamicLogLinksEnabled: false, + }, + &core.TaskTemplate{ + Config: map[string]string{ + "link_type": "vscode", + "port": "65535", + }, + }, + nil, + }, + { + "Flyin disabled and K8s enabled and flyin config present in TaskTemplate", + &LogConfig{ + IsKubernetesEnabled: true, + KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + IsDynamicLogLinksEnabled: false, + }, + &core.TaskTemplate{ + Config: map[string]string{ + "link_type": "vscode", + "port": "65535", + }, + }, + []*core.TaskLog{ + { + Uri: "https://k8s.com/my-namespace/my-pod/ContainerName/ContainerID", + MessageFormat: core.TaskLog_JSON, + Name: "Kubernetes Logs my-Suffix", + }, + }, + }, + { + "Flyin and K8s enabled", + &LogConfig{ + IsKubernetesEnabled: true, + KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + IsDynamicLogLinksEnabled: true, + DynamicLogLinks: DynamicLogLinks{ + Flyin: "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + }, + }, + &core.TaskTemplate{ + Config: map[string]string{ + "link_type": "vscode", + "port": "65535", + }, + }, + []*core.TaskLog{ + { + Uri: "https://k8s.com/my-namespace/my-pod/ContainerName/ContainerID", + MessageFormat: core.TaskLog_JSON, + Name: "Kubernetes Logs my-Suffix", + }, + { + Uri: "https://flyin.mydomain.com:65535/my-namespace/my-pod/ContainerName/ContainerID", + MessageFormat: core.TaskLog_JSON, + Name: "Flyin Logs my-Suffix", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assertTestSucceeded(t, tt.config, tt.template, tt.expectedTaskLogs) + }) + } +} diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go index d7d21b1e7d..4cb7fe209c 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go @@ -49,7 +49,7 @@ type Input struct { PodUID string TaskExecutionID pluginsCore.TaskExecutionID ExtraTemplateVarsByScheme *TemplateVarsByScheme - TaskTemplate *core.TaskTemplate + TaskTemplate *core.TaskTemplate } // Output contains all task logs a plugin generates for a given Input. @@ -64,9 +64,9 @@ type Plugin interface { } type TemplateLogPlugin 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."` - DynamicTemplateURIs []TemplateURI `json:"dynamicTemplateUris" pflag:",Dynamic URI Templates for generating task log links."` - MessageFormat core.TaskLog_MessageFormat `json:"messageFormat" pflag:",Log Message Format."` - Scheme TemplateScheme `json:"scheme" pflag:",Templating scheme to use. Supported values are Pod and TaskExecution."` + 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."` + DynamicTemplateURIs []TemplateURI `json:"dynamicTemplateUris" pflag:",Dynamic URI Templates for generating task log links."` + MessageFormat core.TaskLog_MessageFormat `json:"messageFormat" pflag:",Log Message Format."` + Scheme TemplateScheme `json:"scheme" pflag:",Templating scheme to use. Supported values are Pod and TaskExecution."` } diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go index f279707a3b..abcf96dbe8 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go @@ -75,6 +75,25 @@ func Test_Input_templateVarsForScheme(t *testing.T) { TaskExecutionID: dummyTaskExecID(), } + flyinBase := 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, + TaskTemplate: &core.TaskTemplate{ + Config: map[string]string{ + "port": "1234", + }, + }, + } + tests := []struct { name string scheme TemplateScheme @@ -202,6 +221,50 @@ func Test_Input_templateVarsForScheme(t *testing.T) { {testRegexes.Baz, "baz"}, }, }, + { + "flyin happy path", + TemplateSchemeDynamic, + flyinBase, + nil, + nil, + TemplateVars{ + {defaultRegexes.Port, "1234"}, + }, + nil, + }, + { + "flyin and pod happy path", + TemplateSchemeDynamic, + flyinBase, + nil, + TemplateVars{ + {defaultRegexes.LogName, "main_logs"}, + {defaultRegexes.Port, "1234"}, + {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 port not affected", + TemplateSchemePod, + podBase, + nil, + nil, + nil, + TemplateVars{ + {defaultRegexes.Port, "1234"}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -474,6 +537,76 @@ func TestTemplateLogPlugin(t *testing.T) { }, }, }, + { + "flyin", + TemplateLogPlugin{ + Scheme: TemplateSchemeDynamic, + DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, + MessageFormat: core.TaskLog_JSON, + }, + args{ + input: Input{ + PodName: "my-pod-name", + TaskTemplate: &core.TaskTemplate{ + Config: map[string]string{ + "link_type": "vscode", + "port": "1234", + }, + }, + }, + }, + Output{ + TaskLogs: []*core.TaskLog{ + { + Uri: "vscode://flyin:1234/my-pod-name", + MessageFormat: core.TaskLog_JSON, + }, + }, + }, + }, + { + "flyin - default port", + TemplateLogPlugin{ + Scheme: TemplateSchemeDynamic, + TemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, + MessageFormat: core.TaskLog_JSON, + }, + args{ + input: Input{ + PodName: "my-pod-name", + TaskTemplate: &core.TaskTemplate{ + Config: map[string]string{ + "link_type": "vscode", + }, + }, + }, + }, + Output{ + TaskLogs: []*core.TaskLog{ + { + Uri: "vscode://flyin:8080/my-pod-name", + MessageFormat: core.TaskLog_JSON, + }, + }, + }, + }, + { + "flyin - no link_type in task template", + TemplateLogPlugin{ + Scheme: TemplateSchemeDynamic, + DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, + MessageFormat: core.TaskLog_JSON, + DisplayName: "Flyin Logs", + }, + args{ + input: Input{ + PodName: "my-pod-name", + }, + }, + Output{ + TaskLogs: []*core.TaskLog{}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 93f584525e7d59d32a43c69ddbdeafe440d9ef1a Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Tue, 23 Jan 2024 11:17:05 -0800 Subject: [PATCH 04/19] Introduce Kind to DynamicTemplateURI and update tests Signed-off-by: Eduardo Apolinario --- flyte-single-binary-local.yaml | 3 -- flyteplugins/go/tasks/logs/logging_utils.go | 2 +- .../tasks/pluginmachinery/tasklog/plugin.go | 7 ++- .../tasks/pluginmachinery/tasklog/template.go | 43 ++++++++++++------- .../pluginmachinery/tasklog/template_test.go | 4 +- 5 files changed, 36 insertions(+), 23 deletions(-) diff --git a/flyte-single-binary-local.yaml b/flyte-single-binary-local.yaml index 4eecc5d41d..ca63458b03 100644 --- a/flyte-single-binary-local.yaml +++ b/flyte-single-binary-local.yaml @@ -66,9 +66,6 @@ plugins: kubernetes-template-uri: http://localhost:30080/kubernetes-dashboard/#/log/{{.namespace }}/{{ .podName }}/pod?namespace={{ .namespace }} cloudwatch-enabled: false stackdriver-enabled: false - dynamic-log-links-enabled: true - dynamic-log-links: - flyin: http://flyin.vs:{{ .taskConfig.port }}/ database: postgres: diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index 9587813852..4036086224 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -113,7 +113,7 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { if cfg.IsDynamicLogLinksEnabled { if len(cfg.DynamicLogLinks.Flyin) > 0 { - dynamicPlugins = append(dynamicPlugins, tasklog.TemplateLogPlugin{DisplayName: "Flyin Logs", Scheme: tasklog.TemplateSchemeDynamic, DynamicTemplateURIs: []tasklog.TemplateURI{cfg.DynamicLogLinks.Flyin}, MessageFormat: core.TaskLog_JSON}) + dynamicPlugins = append(dynamicPlugins, tasklog.TemplateLogPlugin{DisplayName: "Flyin Logs", Scheme: tasklog.TemplateSchemeDynamic, DynamicTemplateURIs: []tasklog.DynamicTemplateURI{{TemplateURI: cfg.DynamicLogLinks.Flyin, Kind: "flyin"}}, MessageFormat: core.TaskLog_JSON}) } } diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go index 4cb7fe209c..e2e108aa8a 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go @@ -20,6 +20,11 @@ const ( // TemplateURI is a URI that accepts templates. See: go/tasks/pluginmachinery/tasklog/template.go for available templates. type TemplateURI = string +type DynamicTemplateURI struct { + TemplateURI TemplateURI + Kind string // Kind describes the kind of the dynamic template. Currently only "flyin" is supported. +} + type TemplateVar struct { Regex *regexp.Regexp Value string @@ -66,7 +71,7 @@ type Plugin interface { type TemplateLogPlugin 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."` - DynamicTemplateURIs []TemplateURI `json:"dynamicTemplateUris" pflag:",Dynamic URI Templates for generating task log links."` + DynamicTemplateURIs []DynamicTemplateURI `json:"dynamicTemplateUris" pflag:",Dynamic URI Templates for generating task log links."` MessageFormat core.TaskLog_MessageFormat `json:"messageFormat" pflag:",Log Message Format."` Scheme TemplateScheme `json:"scheme" pflag:",Templating scheme to use. Supported values are Pod and TaskExecution."` } diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index 04980dc3aa..981448c696 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -196,19 +196,29 @@ func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { return vars } -func hasDynamicLogLinks(taskTemplate *core.TaskTemplate) bool { +func getDynamicLogLinkTypes(taskTemplate *core.TaskTemplate) []string { if taskTemplate == nil { - return false + return nil } config := taskTemplate.GetConfig() if config == nil { - return false + return nil } - // TODO: establish a better protocol to handle dynamic log links. - // One idea is to have a comma-separated list of dynamic log link types - // in the task template. - // NB: we are going to grandfather in the flyin way of doing things for now. - return config["link_type"] == "vscode" + linkType := config["link_type"] + if linkType == "" { + return nil + } + dynamicLogLinkTypes := []string{} + for _, linkType := range strings.Split(linkType, ",") { + // NB: we are going to grandfather in flyin. + if linkType == "vscode" { + dynamicLogLinkTypes = append(dynamicLogLinkTypes, "flyin") + } else { + dynamicLogLinkTypes = append(dynamicLogLinkTypes, linkType) + } + } + + return dynamicLogLinkTypes } func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { @@ -222,14 +232,15 @@ func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { }) } - if hasDynamicLogLinks(input.TaskTemplate) { - // TODO: Only emit dynamic log links enabled in the task template. - for _, templateURI := range p.DynamicTemplateURIs { - taskLogs = append(taskLogs, &core.TaskLog{ - Uri: replaceAll(templateURI, templateVars), - Name: p.DisplayName + input.LogName, - MessageFormat: p.MessageFormat, - }) + for _, dynamicLogLinkType := range getDynamicLogLinkTypes(input.TaskTemplate) { + for _, dynamicTemplateURI := range p.DynamicTemplateURIs { + if dynamicTemplateURI.Kind == dynamicLogLinkType { + taskLogs = append(taskLogs, &core.TaskLog{ + Uri: replaceAll(dynamicTemplateURI.TemplateURI, templateVars), + Name: p.DisplayName + input.LogName, + MessageFormat: p.MessageFormat, + }) + } } } diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go index abcf96dbe8..a5b89e62a1 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go @@ -541,7 +541,7 @@ func TestTemplateLogPlugin(t *testing.T) { "flyin", TemplateLogPlugin{ Scheme: TemplateSchemeDynamic, - DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, + DynamicTemplateURIs: []DynamicTemplateURI{{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}", "flyin"}}, MessageFormat: core.TaskLog_JSON, }, args{ @@ -594,7 +594,7 @@ func TestTemplateLogPlugin(t *testing.T) { "flyin - no link_type in task template", TemplateLogPlugin{ Scheme: TemplateSchemeDynamic, - DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, + DynamicTemplateURIs: []DynamicTemplateURI{{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}", "flyin"}}, MessageFormat: core.TaskLog_JSON, DisplayName: "Flyin Logs", }, From f168124998773f5a3b5c7caab3152040c2768e58 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Wed, 24 Jan 2024 12:32:01 -0800 Subject: [PATCH 05/19] Use a map in the plugins config section Signed-off-by: Eduardo Apolinario --- ...uild github.com_flyteorg_flyte_cmd.run.xml | 16 ------ flyteplugins/go/tasks/logs/config.go | 6 +- flyteplugins/go/tasks/logs/logconfig_flags.go | 1 + .../go/tasks/logs/logconfig_flags_test.go | 14 +++++ flyteplugins/go/tasks/logs/logging_utils.go | 12 +++- .../go/tasks/logs/logging_utils_test.go | 20 +++---- .../tasks/plugins/array/k8s/config_flags.go | 1 + .../plugins/array/k8s/config_flags_test.go | 14 +++++ .../tasks/plugins/k8s/spark/config_flags.go | 4 ++ .../plugins/k8s/spark/config_flags_test.go | 56 +++++++++++++++++++ 10 files changed, 111 insertions(+), 33 deletions(-) delete mode 100644 .run/go build github.com_flyteorg_flyte_cmd.run.xml diff --git a/.run/go build github.com_flyteorg_flyte_cmd.run.xml b/.run/go build github.com_flyteorg_flyte_cmd.run.xml deleted file mode 100644 index a980caaee5..0000000000 --- a/.run/go build github.com_flyteorg_flyte_cmd.run.xml +++ /dev/null @@ -1,16 +0,0 @@ - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/flyteplugins/go/tasks/logs/config.go b/flyteplugins/go/tasks/logs/config.go index 7422108a25..29e2072baa 100644 --- a/flyteplugins/go/tasks/logs/config.go +++ b/flyteplugins/go/tasks/logs/config.go @@ -7,10 +7,6 @@ import ( //go:generate pflags LogConfig --default-var=DefaultConfig -type DynamicLogLinks struct { - Flyin tasklog.TemplateURI `json:"flyin" pflag:"-,Template Uri to use when building flyin log links"` -} - // LogConfig encapsulates plugins' log configs type LogConfig struct { IsCloudwatchEnabled bool `json:"cloudwatch-enabled" pflag:",Enable Cloudwatch Logging"` @@ -33,7 +29,7 @@ type LogConfig struct { StackDriverTemplateURI tasklog.TemplateURI `json:"stackdriver-template-uri" pflag:",Template Uri to use when building stackdriver log links"` IsDynamicLogLinksEnabled bool `json:"dynamic-log-links-enabled" pflag:",Enable dynamic log links"` - DynamicLogLinks DynamicLogLinks `json:"dynamic-log-links" pflag:",Map of dynamic log links"` + DynamicLogLinks map[string]tasklog.TemplateURI `json:"dynamic-log-links" pflag:",Map of dynamic log links"` Templates []tasklog.TemplateLogPlugin `json:"templates" pflag:"-,"` } diff --git a/flyteplugins/go/tasks/logs/logconfig_flags.go b/flyteplugins/go/tasks/logs/logconfig_flags.go index 8f0e560100..5d714926a4 100755 --- a/flyteplugins/go/tasks/logs/logconfig_flags.go +++ b/flyteplugins/go/tasks/logs/logconfig_flags.go @@ -62,5 +62,6 @@ func (cfg LogConfig) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stackdriver-logresourcename"), DefaultConfig.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stackdriver-template-uri"), DefaultConfig.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "dynamic-log-links-enabled"), DefaultConfig.IsDynamicLogLinksEnabled, "Enable dynamic log links") + cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "dynamic-log-links"), DefaultConfig.DynamicLogLinks, "Map of dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/logs/logconfig_flags_test.go b/flyteplugins/go/tasks/logs/logconfig_flags_test.go index 004ff8dfd3..af52427196 100755 --- a/flyteplugins/go/tasks/logs/logconfig_flags_test.go +++ b/flyteplugins/go/tasks/logs/logconfig_flags_test.go @@ -267,4 +267,18 @@ func TestLogConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_dynamic-log-links", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "a=1,b=2" + + cmdFlags.Set("dynamic-log-links", testValue) + if vStringToString, err := cmdFlags.GetStringToString("dynamic-log-links"); err == nil { + testDecodeRaw_LogConfig(t, vStringToString, &actual.DynamicLogLinks) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) } diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index 4036086224..1097e61b65 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -112,8 +112,16 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { } if cfg.IsDynamicLogLinksEnabled { - if len(cfg.DynamicLogLinks.Flyin) > 0 { - dynamicPlugins = append(dynamicPlugins, tasklog.TemplateLogPlugin{DisplayName: "Flyin Logs", Scheme: tasklog.TemplateSchemeDynamic, DynamicTemplateURIs: []tasklog.DynamicTemplateURI{{TemplateURI: cfg.DynamicLogLinks.Flyin, Kind: "flyin"}}, MessageFormat: core.TaskLog_JSON}) + for logLinkType, dynamicLogLink := range cfg.DynamicLogLinks { + dynamicPlugins = append( + dynamicPlugins, + tasklog.TemplateLogPlugin{ + DisplayName: fmt.Sprintf("%s logs", logLinkType), + Scheme: tasklog.TemplateSchemeDynamic, + DynamicTemplateURIs: []tasklog.DynamicTemplateURI{ + {TemplateURI: dynamicLogLink, Kind: logLinkType}}, + MessageFormat: core.TaskLog_JSON, + }) } } diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index afc967b7a9..fe33f46e55 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -362,8 +362,8 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { "Flyin enabled but no task template", &LogConfig{ IsDynamicLogLinksEnabled: true, - DynamicLogLinks: DynamicLogLinks{ - Flyin: "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + DynamicLogLinks: map[string]tasklog.TemplateURI{ + "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, nil, @@ -373,8 +373,8 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { "Flyin enabled but config not found in task template", &LogConfig{ IsDynamicLogLinksEnabled: true, - DynamicLogLinks: DynamicLogLinks{ - Flyin: "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + DynamicLogLinks: map[string]tasklog.TemplateURI{ + "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, &core.TaskTemplate{}, @@ -384,8 +384,8 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { "Flyin enabled but no port in task template", &LogConfig{ IsDynamicLogLinksEnabled: true, - DynamicLogLinks: DynamicLogLinks{ - Flyin: "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + DynamicLogLinks: map[string]tasklog.TemplateURI{ + "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, &core.TaskTemplate{ @@ -397,7 +397,7 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { { Uri: "https://flyin.mydomain.com:8080/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, - Name: "Flyin Logs my-Suffix", + Name: "flyin logs my-Suffix", }, }, }, @@ -441,8 +441,8 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { IsKubernetesEnabled: true, KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", IsDynamicLogLinksEnabled: true, - DynamicLogLinks: DynamicLogLinks{ - Flyin: "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + DynamicLogLinks: map[string]tasklog.TemplateURI{ + "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, &core.TaskTemplate{ @@ -460,7 +460,7 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { { Uri: "https://flyin.mydomain.com:65535/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, - Name: "Flyin Logs my-Suffix", + Name: "flyin logs my-Suffix", }, }, }, diff --git a/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go b/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go index 22edf6e444..c5158ec923 100755 --- a/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go @@ -71,5 +71,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.stackdriver-logresourcename"), defaultConfig.LogConfig.Config.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.stackdriver-template-uri"), defaultConfig.LogConfig.Config.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.config.dynamic-log-links-enabled"), defaultConfig.LogConfig.Config.IsDynamicLogLinksEnabled, "Enable dynamic log links") + cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.config.dynamic-log-links"), defaultConfig.LogConfig.Config.DynamicLogLinks, "Map of dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go b/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go index fa40f5f9ec..c72ca10822 100755 --- a/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go @@ -393,4 +393,18 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_logs.config.dynamic-log-links", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "a=1,b=2" + + cmdFlags.Set("logs.config.dynamic-log-links", testValue) + if vStringToString, err := cmdFlags.GetStringToString("logs.config.dynamic-log-links"); err == nil { + testDecodeRaw_Config(t, vStringToString, &actual.LogConfig.Config.DynamicLogLinks) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) } diff --git a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go index d82cdd96ad..19331aae8c 100755 --- a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go +++ b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go @@ -63,6 +63,7 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.stackdriver-logresourcename"), defaultConfig.LogConfig.Mixed.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.stackdriver-template-uri"), defaultConfig.LogConfig.Mixed.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.mixed.dynamic-log-links-enabled"), defaultConfig.LogConfig.Mixed.IsDynamicLogLinksEnabled, "Enable dynamic log links") + cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.mixed.dynamic-log-links"), defaultConfig.LogConfig.Mixed.DynamicLogLinks, "Map of dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-enabled"), defaultConfig.LogConfig.User.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-region"), defaultConfig.LogConfig.User.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-log-group"), defaultConfig.LogConfig.User.CloudwatchLogGroup, "Log group to which streams are associated.") @@ -75,6 +76,7 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.stackdriver-logresourcename"), defaultConfig.LogConfig.User.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.stackdriver-template-uri"), defaultConfig.LogConfig.User.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.user.dynamic-log-links-enabled"), defaultConfig.LogConfig.User.IsDynamicLogLinksEnabled, "Enable dynamic log links") + cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.user.dynamic-log-links"), defaultConfig.LogConfig.User.DynamicLogLinks, "Map of dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-enabled"), defaultConfig.LogConfig.System.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-region"), defaultConfig.LogConfig.System.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-log-group"), defaultConfig.LogConfig.System.CloudwatchLogGroup, "Log group to which streams are associated.") @@ -87,6 +89,7 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.stackdriver-logresourcename"), defaultConfig.LogConfig.System.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.stackdriver-template-uri"), defaultConfig.LogConfig.System.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.system.dynamic-log-links-enabled"), defaultConfig.LogConfig.System.IsDynamicLogLinksEnabled, "Enable dynamic log links") + cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.system.dynamic-log-links"), defaultConfig.LogConfig.System.DynamicLogLinks, "Map of dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-enabled"), defaultConfig.LogConfig.AllUser.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-region"), defaultConfig.LogConfig.AllUser.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-log-group"), defaultConfig.LogConfig.AllUser.CloudwatchLogGroup, "Log group to which streams are associated.") @@ -99,5 +102,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.stackdriver-logresourcename"), defaultConfig.LogConfig.AllUser.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.stackdriver-template-uri"), defaultConfig.LogConfig.AllUser.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.all-user.dynamic-log-links-enabled"), defaultConfig.LogConfig.AllUser.IsDynamicLogLinksEnabled, "Enable dynamic log links") + cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.all-user.dynamic-log-links"), defaultConfig.LogConfig.AllUser.DynamicLogLinks, "Map of dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go index ee45707d19..44f5f47d6b 100755 --- a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go @@ -281,6 +281,20 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_logs.mixed.dynamic-log-links", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "a=1,b=2" + + cmdFlags.Set("logs.mixed.dynamic-log-links", testValue) + if vStringToString, err := cmdFlags.GetStringToString("logs.mixed.dynamic-log-links"); err == nil { + testDecodeRaw_Config(t, vStringToString, &actual.LogConfig.Mixed.DynamicLogLinks) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) t.Run("Test_logs.user.cloudwatch-enabled", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -449,6 +463,20 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_logs.user.dynamic-log-links", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "a=1,b=2" + + cmdFlags.Set("logs.user.dynamic-log-links", testValue) + if vStringToString, err := cmdFlags.GetStringToString("logs.user.dynamic-log-links"); err == nil { + testDecodeRaw_Config(t, vStringToString, &actual.LogConfig.User.DynamicLogLinks) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) t.Run("Test_logs.system.cloudwatch-enabled", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -617,6 +645,20 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_logs.system.dynamic-log-links", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "a=1,b=2" + + cmdFlags.Set("logs.system.dynamic-log-links", testValue) + if vStringToString, err := cmdFlags.GetStringToString("logs.system.dynamic-log-links"); err == nil { + testDecodeRaw_Config(t, vStringToString, &actual.LogConfig.System.DynamicLogLinks) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) t.Run("Test_logs.all-user.cloudwatch-enabled", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -785,4 +827,18 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_logs.all-user.dynamic-log-links", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "a=1,b=2" + + cmdFlags.Set("logs.all-user.dynamic-log-links", testValue) + if vStringToString, err := cmdFlags.GetStringToString("logs.all-user.dynamic-log-links"); err == nil { + testDecodeRaw_Config(t, vStringToString, &actual.LogConfig.AllUser.DynamicLogLinks) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) } From d66f1a083f08246c4fb8dcbbac86b7888e126f87 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Wed, 24 Jan 2024 12:43:16 -0800 Subject: [PATCH 06/19] Lint Signed-off-by: Eduardo Apolinario --- flyteplugins/go/tasks/logs/config.go | 2 +- flyteplugins/go/tasks/logs/logging_utils.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/flyteplugins/go/tasks/logs/config.go b/flyteplugins/go/tasks/logs/config.go index 29e2072baa..061d50c83c 100644 --- a/flyteplugins/go/tasks/logs/config.go +++ b/flyteplugins/go/tasks/logs/config.go @@ -28,7 +28,7 @@ type LogConfig struct { StackdriverLogResourceName string `json:"stackdriver-logresourcename" pflag:",Name of the logresource in stackdriver"` StackDriverTemplateURI tasklog.TemplateURI `json:"stackdriver-template-uri" pflag:",Template Uri to use when building stackdriver log links"` - IsDynamicLogLinksEnabled bool `json:"dynamic-log-links-enabled" pflag:",Enable dynamic log links"` + IsDynamicLogLinksEnabled bool `json:"dynamic-log-links-enabled" pflag:",Enable dynamic log links"` DynamicLogLinks map[string]tasklog.TemplateURI `json:"dynamic-log-links" pflag:",Map of dynamic log links"` Templates []tasklog.TemplateLogPlugin `json:"templates" pflag:"-,"` diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index 1097e61b65..97c22408a7 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -117,7 +117,7 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { dynamicPlugins, tasklog.TemplateLogPlugin{ DisplayName: fmt.Sprintf("%s logs", logLinkType), - Scheme: tasklog.TemplateSchemeDynamic, + Scheme: tasklog.TemplateSchemeDynamic, DynamicTemplateURIs: []tasklog.DynamicTemplateURI{ {TemplateURI: dynamicLogLink, Kind: logLinkType}}, MessageFormat: core.TaskLog_JSON, From 59dd968ee43a4beeb249c6ed78a9b45fe6008045 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Fri, 26 Jan 2024 10:37:00 -0800 Subject: [PATCH 07/19] Remove boolean flag to enable dynamic log links Signed-off-by: Eduardo Apolinario --- flyteplugins/go/tasks/logs/config.go | 1 - flyteplugins/go/tasks/logs/logconfig_flags.go | 1 - .../go/tasks/logs/logconfig_flags_test.go | 14 ----- flyteplugins/go/tasks/logs/logging_utils.go | 22 ++++---- .../go/tasks/logs/logging_utils_test.go | 6 -- .../tasks/plugins/array/k8s/config_flags.go | 1 - .../plugins/array/k8s/config_flags_test.go | 14 ----- .../tasks/plugins/k8s/spark/config_flags.go | 4 -- .../plugins/k8s/spark/config_flags_test.go | 56 ------------------- 9 files changed, 10 insertions(+), 109 deletions(-) diff --git a/flyteplugins/go/tasks/logs/config.go b/flyteplugins/go/tasks/logs/config.go index 061d50c83c..69f2ca23cb 100644 --- a/flyteplugins/go/tasks/logs/config.go +++ b/flyteplugins/go/tasks/logs/config.go @@ -28,7 +28,6 @@ type LogConfig struct { StackdriverLogResourceName string `json:"stackdriver-logresourcename" pflag:",Name of the logresource in stackdriver"` StackDriverTemplateURI tasklog.TemplateURI `json:"stackdriver-template-uri" pflag:",Template Uri to use when building stackdriver log links"` - IsDynamicLogLinksEnabled bool `json:"dynamic-log-links-enabled" pflag:",Enable dynamic log links"` DynamicLogLinks map[string]tasklog.TemplateURI `json:"dynamic-log-links" pflag:",Map of dynamic log links"` Templates []tasklog.TemplateLogPlugin `json:"templates" pflag:"-,"` diff --git a/flyteplugins/go/tasks/logs/logconfig_flags.go b/flyteplugins/go/tasks/logs/logconfig_flags.go index 5d714926a4..5cf5b8f88c 100755 --- a/flyteplugins/go/tasks/logs/logconfig_flags.go +++ b/flyteplugins/go/tasks/logs/logconfig_flags.go @@ -61,7 +61,6 @@ func (cfg LogConfig) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "gcp-project"), DefaultConfig.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stackdriver-logresourcename"), DefaultConfig.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stackdriver-template-uri"), DefaultConfig.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "dynamic-log-links-enabled"), DefaultConfig.IsDynamicLogLinksEnabled, "Enable dynamic log links") cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "dynamic-log-links"), DefaultConfig.DynamicLogLinks, "Map of dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/logs/logconfig_flags_test.go b/flyteplugins/go/tasks/logs/logconfig_flags_test.go index af52427196..6f42d36d38 100755 --- a/flyteplugins/go/tasks/logs/logconfig_flags_test.go +++ b/flyteplugins/go/tasks/logs/logconfig_flags_test.go @@ -253,20 +253,6 @@ func TestLogConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_dynamic-log-links-enabled", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("dynamic-log-links-enabled", testValue) - if vBool, err := cmdFlags.GetBool("dynamic-log-links-enabled"); err == nil { - testDecodeJson_LogConfig(t, fmt.Sprintf("%v", vBool), &actual.IsDynamicLogLinksEnabled) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) t.Run("Test_dynamic-log-links", func(t *testing.T) { t.Run("Override", func(t *testing.T) { diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index 97c22408a7..c18eb79c2c 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -111,18 +111,16 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { } } - if cfg.IsDynamicLogLinksEnabled { - for logLinkType, dynamicLogLink := range cfg.DynamicLogLinks { - dynamicPlugins = append( - dynamicPlugins, - tasklog.TemplateLogPlugin{ - DisplayName: fmt.Sprintf("%s logs", logLinkType), - Scheme: tasklog.TemplateSchemeDynamic, - DynamicTemplateURIs: []tasklog.DynamicTemplateURI{ - {TemplateURI: dynamicLogLink, Kind: logLinkType}}, - MessageFormat: core.TaskLog_JSON, - }) - } + for logLinkType, dynamicLogLink := range cfg.DynamicLogLinks { + dynamicPlugins = append( + dynamicPlugins, + tasklog.TemplateLogPlugin{ + DisplayName: fmt.Sprintf("%s logs", logLinkType), + Scheme: tasklog.TemplateSchemeDynamic, + DynamicTemplateURIs: []tasklog.DynamicTemplateURI{ + {TemplateURI: dynamicLogLink, Kind: logLinkType}}, + MessageFormat: core.TaskLog_JSON, + }) } plugins = append(plugins, cfg.Templates...) diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index fe33f46e55..00f58c01a0 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -361,7 +361,6 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { { "Flyin enabled but no task template", &LogConfig{ - IsDynamicLogLinksEnabled: true, DynamicLogLinks: map[string]tasklog.TemplateURI{ "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, @@ -372,7 +371,6 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { { "Flyin enabled but config not found in task template", &LogConfig{ - IsDynamicLogLinksEnabled: true, DynamicLogLinks: map[string]tasklog.TemplateURI{ "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, @@ -383,7 +381,6 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { { "Flyin enabled but no port in task template", &LogConfig{ - IsDynamicLogLinksEnabled: true, DynamicLogLinks: map[string]tasklog.TemplateURI{ "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, @@ -404,7 +401,6 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { { "Flyin disabled but config present in TaskTemplate", &LogConfig{ - IsDynamicLogLinksEnabled: false, }, &core.TaskTemplate{ Config: map[string]string{ @@ -419,7 +415,6 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { &LogConfig{ IsKubernetesEnabled: true, KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", - IsDynamicLogLinksEnabled: false, }, &core.TaskTemplate{ Config: map[string]string{ @@ -440,7 +435,6 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { &LogConfig{ IsKubernetesEnabled: true, KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", - IsDynamicLogLinksEnabled: true, DynamicLogLinks: map[string]tasklog.TemplateURI{ "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, diff --git a/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go b/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go index c5158ec923..a593c5879c 100755 --- a/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go @@ -70,7 +70,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.gcp-project"), defaultConfig.LogConfig.Config.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.stackdriver-logresourcename"), defaultConfig.LogConfig.Config.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.stackdriver-template-uri"), defaultConfig.LogConfig.Config.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.config.dynamic-log-links-enabled"), defaultConfig.LogConfig.Config.IsDynamicLogLinksEnabled, "Enable dynamic log links") cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.config.dynamic-log-links"), defaultConfig.LogConfig.Config.DynamicLogLinks, "Map of dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go b/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go index c72ca10822..8dbce02379 100755 --- a/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go @@ -379,20 +379,6 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_logs.config.dynamic-log-links-enabled", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("logs.config.dynamic-log-links-enabled", testValue) - if vBool, err := cmdFlags.GetBool("logs.config.dynamic-log-links-enabled"); err == nil { - testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.LogConfig.Config.IsDynamicLogLinksEnabled) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) t.Run("Test_logs.config.dynamic-log-links", func(t *testing.T) { t.Run("Override", func(t *testing.T) { diff --git a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go index 19331aae8c..b8807386e3 100755 --- a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go +++ b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go @@ -62,7 +62,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.gcp-project"), defaultConfig.LogConfig.Mixed.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.stackdriver-logresourcename"), defaultConfig.LogConfig.Mixed.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.stackdriver-template-uri"), defaultConfig.LogConfig.Mixed.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.mixed.dynamic-log-links-enabled"), defaultConfig.LogConfig.Mixed.IsDynamicLogLinksEnabled, "Enable dynamic log links") cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.mixed.dynamic-log-links"), defaultConfig.LogConfig.Mixed.DynamicLogLinks, "Map of dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-enabled"), defaultConfig.LogConfig.User.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-region"), defaultConfig.LogConfig.User.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") @@ -75,7 +74,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.gcp-project"), defaultConfig.LogConfig.User.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.stackdriver-logresourcename"), defaultConfig.LogConfig.User.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.stackdriver-template-uri"), defaultConfig.LogConfig.User.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.user.dynamic-log-links-enabled"), defaultConfig.LogConfig.User.IsDynamicLogLinksEnabled, "Enable dynamic log links") cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.user.dynamic-log-links"), defaultConfig.LogConfig.User.DynamicLogLinks, "Map of dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-enabled"), defaultConfig.LogConfig.System.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-region"), defaultConfig.LogConfig.System.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") @@ -88,7 +86,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.gcp-project"), defaultConfig.LogConfig.System.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.stackdriver-logresourcename"), defaultConfig.LogConfig.System.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.stackdriver-template-uri"), defaultConfig.LogConfig.System.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.system.dynamic-log-links-enabled"), defaultConfig.LogConfig.System.IsDynamicLogLinksEnabled, "Enable dynamic log links") cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.system.dynamic-log-links"), defaultConfig.LogConfig.System.DynamicLogLinks, "Map of dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-enabled"), defaultConfig.LogConfig.AllUser.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-region"), defaultConfig.LogConfig.AllUser.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") @@ -101,7 +98,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.gcp-project"), defaultConfig.LogConfig.AllUser.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.stackdriver-logresourcename"), defaultConfig.LogConfig.AllUser.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.stackdriver-template-uri"), defaultConfig.LogConfig.AllUser.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.all-user.dynamic-log-links-enabled"), defaultConfig.LogConfig.AllUser.IsDynamicLogLinksEnabled, "Enable dynamic log links") cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.all-user.dynamic-log-links"), defaultConfig.LogConfig.AllUser.DynamicLogLinks, "Map of dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go index 44f5f47d6b..c8e16e6d7d 100755 --- a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go @@ -267,20 +267,6 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_logs.mixed.dynamic-log-links-enabled", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("logs.mixed.dynamic-log-links-enabled", testValue) - if vBool, err := cmdFlags.GetBool("logs.mixed.dynamic-log-links-enabled"); err == nil { - testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.LogConfig.Mixed.IsDynamicLogLinksEnabled) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) t.Run("Test_logs.mixed.dynamic-log-links", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -449,20 +435,6 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_logs.user.dynamic-log-links-enabled", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("logs.user.dynamic-log-links-enabled", testValue) - if vBool, err := cmdFlags.GetBool("logs.user.dynamic-log-links-enabled"); err == nil { - testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.LogConfig.User.IsDynamicLogLinksEnabled) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) t.Run("Test_logs.user.dynamic-log-links", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -631,20 +603,6 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_logs.system.dynamic-log-links-enabled", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("logs.system.dynamic-log-links-enabled", testValue) - if vBool, err := cmdFlags.GetBool("logs.system.dynamic-log-links-enabled"); err == nil { - testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.LogConfig.System.IsDynamicLogLinksEnabled) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) t.Run("Test_logs.system.dynamic-log-links", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -813,20 +771,6 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_logs.all-user.dynamic-log-links-enabled", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "1" - - cmdFlags.Set("logs.all-user.dynamic-log-links-enabled", testValue) - if vBool, err := cmdFlags.GetBool("logs.all-user.dynamic-log-links-enabled"); err == nil { - testDecodeJson_Config(t, fmt.Sprintf("%v", vBool), &actual.LogConfig.AllUser.IsDynamicLogLinksEnabled) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) t.Run("Test_logs.all-user.dynamic-log-links", func(t *testing.T) { t.Run("Override", func(t *testing.T) { From 0a42ca1fcac739d26c1e17746469ebac24f7b194 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Fri, 26 Jan 2024 11:13:24 -0800 Subject: [PATCH 08/19] Add name to TemplateLogPlugin and use that to identify dynamic log links Signed-off-by: Eduardo Apolinario --- flyteplugins/go/tasks/logs/config.go | 2 +- flyteplugins/go/tasks/logs/logging_utils.go | 6 +++-- .../go/tasks/logs/logging_utils_test.go | 23 +++++++++---------- .../tasks/pluginmachinery/tasklog/plugin.go | 8 ++----- .../tasks/pluginmachinery/tasklog/template.go | 16 +++---------- .../pluginmachinery/tasklog/template_test.go | 13 +++++++---- .../common/common_operator_test.go | 5 ++-- 7 files changed, 32 insertions(+), 41 deletions(-) diff --git a/flyteplugins/go/tasks/logs/config.go b/flyteplugins/go/tasks/logs/config.go index 69f2ca23cb..b644dd12c4 100644 --- a/flyteplugins/go/tasks/logs/config.go +++ b/flyteplugins/go/tasks/logs/config.go @@ -28,7 +28,7 @@ type LogConfig struct { StackdriverLogResourceName string `json:"stackdriver-logresourcename" pflag:",Name of the logresource in stackdriver"` StackDriverTemplateURI tasklog.TemplateURI `json:"stackdriver-template-uri" pflag:",Template Uri to use when building stackdriver log links"` - DynamicLogLinks map[string]tasklog.TemplateURI `json:"dynamic-log-links" pflag:",Map of dynamic log links"` + DynamicLogLinks map[string]tasklog.TemplateURI `json:"dynamic-log-links" pflag:",Map of dynamic log links"` Templates []tasklog.TemplateLogPlugin `json:"templates" pflag:"-,"` } diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index c18eb79c2c..44ae459f34 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -115,10 +115,12 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { dynamicPlugins = append( dynamicPlugins, tasklog.TemplateLogPlugin{ + Name: logLinkType, DisplayName: fmt.Sprintf("%s logs", logLinkType), Scheme: tasklog.TemplateSchemeDynamic, - DynamicTemplateURIs: []tasklog.DynamicTemplateURI{ - {TemplateURI: dynamicLogLink, Kind: logLinkType}}, + DynamicTemplateURIs: []tasklog.TemplateURI{ + dynamicLogLink, + }, MessageFormat: core.TaskLog_JSON, }) } diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index 00f58c01a0..02251c3836 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -362,7 +362,7 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { "Flyin enabled but no task template", &LogConfig{ DynamicLogLinks: map[string]tasklog.TemplateURI{ - "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + "vscode": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, nil, @@ -372,7 +372,7 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { "Flyin enabled but config not found in task template", &LogConfig{ DynamicLogLinks: map[string]tasklog.TemplateURI{ - "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + "vscode": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, &core.TaskTemplate{}, @@ -382,7 +382,7 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { "Flyin enabled but no port in task template", &LogConfig{ DynamicLogLinks: map[string]tasklog.TemplateURI{ - "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + "vscode": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, &core.TaskTemplate{ @@ -394,14 +394,13 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { { Uri: "https://flyin.mydomain.com:8080/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, - Name: "flyin logs my-Suffix", + Name: "vscode logs my-Suffix", }, }, }, { "Flyin disabled but config present in TaskTemplate", - &LogConfig{ - }, + &LogConfig{}, &core.TaskTemplate{ Config: map[string]string{ "link_type": "vscode", @@ -413,8 +412,8 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { { "Flyin disabled and K8s enabled and flyin config present in TaskTemplate", &LogConfig{ - IsKubernetesEnabled: true, - KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + IsKubernetesEnabled: true, + KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, &core.TaskTemplate{ Config: map[string]string{ @@ -433,10 +432,10 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { { "Flyin and K8s enabled", &LogConfig{ - IsKubernetesEnabled: true, - KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + IsKubernetesEnabled: true, + KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", DynamicLogLinks: map[string]tasklog.TemplateURI{ - "flyin": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + "vscode": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, &core.TaskTemplate{ @@ -454,7 +453,7 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { { Uri: "https://flyin.mydomain.com:65535/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, - Name: "flyin logs my-Suffix", + Name: "vscode logs my-Suffix", }, }, }, diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go index e2e108aa8a..c0427c11c5 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go @@ -20,11 +20,6 @@ const ( // TemplateURI is a URI that accepts templates. See: go/tasks/pluginmachinery/tasklog/template.go for available templates. type TemplateURI = string -type DynamicTemplateURI struct { - TemplateURI TemplateURI - Kind string // Kind describes the kind of the dynamic template. Currently only "flyin" is supported. -} - type TemplateVar struct { Regex *regexp.Regexp Value string @@ -69,9 +64,10 @@ type Plugin interface { } type TemplateLogPlugin struct { + Name string `json:"name" pflag:",Name of the plugin."` 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."` - DynamicTemplateURIs []DynamicTemplateURI `json:"dynamicTemplateUris" pflag:",Dynamic URI Templates for generating task log links."` + DynamicTemplateURIs []TemplateURI `json:"dynamictemplateUris" pflag:",URI Templates for generating dynamic task log links."` MessageFormat core.TaskLog_MessageFormat `json:"messageFormat" pflag:",Log Message Format."` Scheme TemplateScheme `json:"scheme" pflag:",Templating scheme to use. Supported values are Pod and TaskExecution."` } diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index 3575daaa24..8faa933dae 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -212,17 +212,7 @@ func getDynamicLogLinkTypes(taskTemplate *core.TaskTemplate) []string { if linkType == "" { return nil } - dynamicLogLinkTypes := []string{} - for _, linkType := range strings.Split(linkType, ",") { - // NB: we are going to grandfather in flyin. - if linkType == "vscode" { - dynamicLogLinkTypes = append(dynamicLogLinkTypes, "flyin") - } else { - dynamicLogLinkTypes = append(dynamicLogLinkTypes, linkType) - } - } - - return dynamicLogLinkTypes + return strings.Split(linkType, ",") } func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { @@ -238,9 +228,9 @@ func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { for _, dynamicLogLinkType := range getDynamicLogLinkTypes(input.TaskTemplate) { for _, dynamicTemplateURI := range p.DynamicTemplateURIs { - if dynamicTemplateURI.Kind == dynamicLogLinkType { + if p.Name == dynamicLogLinkType { taskLogs = append(taskLogs, &core.TaskLog{ - Uri: replaceAll(dynamicTemplateURI.TemplateURI, templateVars), + Uri: replaceAll(dynamicTemplateURI, templateVars), Name: p.DisplayName + input.LogName, MessageFormat: p.MessageFormat, }) diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go index 289752c2e8..fffae5a036 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go @@ -548,8 +548,9 @@ func TestTemplateLogPlugin(t *testing.T) { { "flyin", TemplateLogPlugin{ + Name: "vscode", Scheme: TemplateSchemeDynamic, - DynamicTemplateURIs: []DynamicTemplateURI{{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}", "flyin"}}, + DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, MessageFormat: core.TaskLog_JSON, }, args{ @@ -575,9 +576,10 @@ func TestTemplateLogPlugin(t *testing.T) { { "flyin - default port", TemplateLogPlugin{ - Scheme: TemplateSchemeDynamic, - TemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, - MessageFormat: core.TaskLog_JSON, + Name: "vscode", + Scheme: TemplateSchemeDynamic, + DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, + MessageFormat: core.TaskLog_JSON, }, args{ input: Input{ @@ -601,8 +603,9 @@ func TestTemplateLogPlugin(t *testing.T) { { "flyin - no link_type in task template", TemplateLogPlugin{ + Name: "vscode", Scheme: TemplateSchemeDynamic, - DynamicTemplateURIs: []DynamicTemplateURI{{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}", "flyin"}}, + DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, MessageFormat: core.TaskLog_JSON, DisplayName: "Flyin Logs", }, diff --git a/flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go b/flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go index bf7d537416..64538f2d61 100644 --- a/flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go @@ -211,8 +211,9 @@ func TestGetLogsTemplateUri(t *testing.T) { taskCtx := dummyTaskContext() pytorchJobObjectMeta := meta_v1.ObjectMeta{ - Name: "test", - Namespace: "pytorch-namespace", + Name: "test", + Namespace: "pytorch-" + + "namespace", CreationTimestamp: meta_v1.Time{ Time: time.Date(2022, time.January, 1, 12, 0, 0, 0, time.UTC), }, From 4b6dbc3dacef50daad18ce71538815a8fdb48bfb Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Fri, 26 Jan 2024 15:52:25 -0800 Subject: [PATCH 09/19] Remove scheme switch Signed-off-by: Eduardo Apolinario --- .../go/tasks/logs/logging_utils_test.go | 20 ---- .../tasks/pluginmachinery/tasklog/template.go | 58 +++++---- .../pluginmachinery/tasklog/template_test.go | 111 ++++++++---------- .../plugins/array/k8s/management_test.go | 1 + .../go/tasks/plugins/k8s/dask/dask_test.go | 1 + .../common/common_operator_test.go | 2 + .../plugins/k8s/kfoperators/mpi/mpi_test.go | 1 + .../k8s/kfoperators/pytorch/pytorch_test.go | 1 + .../kfoperators/tensorflow/tensorflow_test.go | 1 + .../go/tasks/plugins/k8s/pod/sidecar_test.go | 1 + .../go/tasks/plugins/k8s/spark/spark_test.go | 1 + 11 files changed, 86 insertions(+), 112 deletions(-) diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index 02251c3836..a0475a6169 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -378,26 +378,6 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { &core.TaskTemplate{}, nil, }, - { - "Flyin enabled but no port in task template", - &LogConfig{ - DynamicLogLinks: map[string]tasklog.TemplateURI{ - "vscode": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", - }, - }, - &core.TaskTemplate{ - Config: map[string]string{ - "link_type": "vscode", - }, - }, - []*core.TaskLog{ - { - Uri: "https://flyin.mydomain.com:8080/my-namespace/my-pod/ContainerName/ContainerID", - MessageFormat: core.TaskLog_JSON, - Name: "vscode logs my-Suffix", - }, - }, - }, { "Flyin disabled but config present in TaskTemplate", &LogConfig{}, diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index 8faa933dae..2bdb3f40fb 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -92,38 +92,34 @@ func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { vars = append(vars, input.ExtraTemplateVarsByScheme.Common...) } - switch scheme { - case TemplateSchemeDynamic: - port := input.TaskTemplate.GetConfig()["port"] - if port == "" { - port = "8080" - } + port := input.TaskTemplate.GetConfig()["port"] + if port != "" { vars = append( vars, TemplateVar{defaultRegexes.Port, port}, ) - fallthrough - 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] - } - 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}, - ) - if gotExtraTemplateVars { - vars = append(vars, input.ExtraTemplateVarsByScheme.Pod...) - } - case TemplateSchemeTaskExecution: + } + + // 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] + } + 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}, + ) + if gotExtraTemplateVars { + vars = append(vars, input.ExtraTemplateVarsByScheme.Pod...) + } + if input.TaskExecutionID != nil { taskExecutionIdentifier := input.TaskExecutionID.GetID() vars = append( vars, @@ -178,9 +174,9 @@ func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { }, ) } - if gotExtraTemplateVars { - vars = append(vars, input.ExtraTemplateVarsByScheme.TaskExecution...) - } + } + if gotExtraTemplateVars { + vars = append(vars, input.ExtraTemplateVarsByScheme.TaskExecution...) } vars = append( diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go index fffae5a036..f62e65250c 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go @@ -149,23 +149,6 @@ func Test_Input_templateVarsForScheme(t *testing.T) { }, 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, @@ -173,6 +156,12 @@ func Test_Input_templateVarsForScheme(t *testing.T) { nil, TemplateVars{ {defaultRegexes.LogName, "main_logs"}, + {defaultRegexes.PodName, ""}, + {defaultRegexes.PodUID, ""}, + {defaultRegexes.Namespace, ""}, + {defaultRegexes.ContainerName, ""}, + {defaultRegexes.ContainerID, ""}, + {defaultRegexes.Hostname, ""}, {defaultRegexes.NodeID, "n0-0-n0"}, {defaultRegexes.GeneratedName, "generated-name"}, {defaultRegexes.TaskRetryAttempt, "1"}, @@ -212,23 +201,23 @@ func Test_Input_templateVarsForScheme(t *testing.T) { }, 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"}, - }, - }, + // { + // "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"}, + // }, + // }, { "flyin happy path", TemplateSchemeDynamic, @@ -573,33 +562,33 @@ func TestTemplateLogPlugin(t *testing.T) { }, }, }, - { - "flyin - default port", - TemplateLogPlugin{ - Name: "vscode", - Scheme: TemplateSchemeDynamic, - DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, - MessageFormat: core.TaskLog_JSON, - }, - args{ - input: Input{ - PodName: "my-pod-name", - TaskTemplate: &core.TaskTemplate{ - Config: map[string]string{ - "link_type": "vscode", - }, - }, - }, - }, - Output{ - TaskLogs: []*core.TaskLog{ - { - Uri: "vscode://flyin:8080/my-pod-name", - MessageFormat: core.TaskLog_JSON, - }, - }, - }, - }, + // { + // "flyin - default port", + // TemplateLogPlugin{ + // Name: "vscode", + // Scheme: TemplateSchemeDynamic, + // DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, + // MessageFormat: core.TaskLog_JSON, + // }, + // args{ + // input: Input{ + // PodName: "my-pod-name", + // TaskTemplate: &core.TaskTemplate{ + // Config: map[string]string{ + // "link_type": "vscode", + // }, + // }, + // }, + // }, + // Output{ + // TaskLogs: []*core.TaskLog{ + // { + // Uri: "vscode://flyin:8080/my-pod-name", + // MessageFormat: core.TaskLog_JSON, + // }, + // }, + // }, + // }, { "flyin - no link_type in task template", TemplateLogPlugin{ diff --git a/flyteplugins/go/tasks/plugins/array/k8s/management_test.go b/flyteplugins/go/tasks/plugins/array/k8s/management_test.go index ab26028b09..9404bdfb72 100644 --- a/flyteplugins/go/tasks/plugins/array/k8s/management_test.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/management_test.go @@ -67,6 +67,7 @@ func getMockTaskExecutionContext(ctx context.Context, parallelism int) *mocks.Ta tID := &mocks.TaskExecutionID{} tID.OnGetGeneratedName().Return("notfound") + tID.On("GetUniqueNodeID").Return("an-unique-id") tID.OnGetID().Return(core2.TaskExecutionIdentifier{ TaskId: &core2.Identifier{ ResourceType: core2.ResourceType_TASK, diff --git a/flyteplugins/go/tasks/plugins/k8s/dask/dask_test.go b/flyteplugins/go/tasks/plugins/k8s/dask/dask_test.go index 576740af93..e7d43a2256 100644 --- a/flyteplugins/go/tasks/plugins/k8s/dask/dask_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/dask/dask_test.go @@ -179,6 +179,7 @@ func dummyDaskTaskContext(taskTemplate *core.TaskTemplate, resources *v1.Resourc }, }) tID.On("GetGeneratedName").Return(testTaskID) + tID.On("GetUniqueNodeID").Return("an-unique-id") taskExecutionMetadata := &mocks.TaskExecutionMetadata{} taskExecutionMetadata.OnGetTaskExecutionID().Return(tID) diff --git a/flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go b/flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go index 64538f2d61..d0e154835c 100644 --- a/flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/kfoperators/common/common_operator_test.go @@ -318,6 +318,8 @@ func dummyTaskContext() pluginsCore.TaskExecutionContext { }, RetryAttempt: 0, }) + tID.OnGetGeneratedName().Return("some-acceptable-name") + tID.On("GetUniqueNodeID").Return("an-unique-id") taskExecutionMetadata := &mocks.TaskExecutionMetadata{} taskExecutionMetadata.OnGetTaskExecutionID().Return(tID) diff --git a/flyteplugins/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go b/flyteplugins/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go index d009c7c887..29fe47a446 100644 --- a/flyteplugins/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go @@ -148,6 +148,7 @@ func dummyMPITaskContext(taskTemplate *core.TaskTemplate, resources *corev1.Reso }, }) tID.OnGetGeneratedName().Return("some-acceptable-name") + tID.On("GetUniqueNodeID").Return("an-unique-id") overrides := &mocks.TaskOverrides{} overrides.OnGetResources().Return(resources) diff --git a/flyteplugins/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch_test.go b/flyteplugins/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch_test.go index e0606b1020..0700644578 100644 --- a/flyteplugins/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/kfoperators/pytorch/pytorch_test.go @@ -154,6 +154,7 @@ func dummyPytorchTaskContext(taskTemplate *core.TaskTemplate, resources *corev1. }, }) tID.OnGetGeneratedName().Return("some-acceptable-name") + tID.On("GetUniqueNodeID").Return("an-unique-id") overrides := &mocks.TaskOverrides{} overrides.OnGetResources().Return(resources) diff --git a/flyteplugins/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow_test.go b/flyteplugins/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow_test.go index c3252183fc..2402bd4c5a 100644 --- a/flyteplugins/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow_test.go @@ -149,6 +149,7 @@ func dummyTensorFlowTaskContext(taskTemplate *core.TaskTemplate, resources *core }, }) tID.OnGetGeneratedName().Return("some-acceptable-name") + tID.On("GetUniqueNodeID").Return("an-unique-id") overrides := &mocks.TaskOverrides{} overrides.OnGetResources().Return(resources) diff --git a/flyteplugins/go/tasks/plugins/k8s/pod/sidecar_test.go b/flyteplugins/go/tasks/plugins/k8s/pod/sidecar_test.go index 0c728780d9..4977695a1a 100644 --- a/flyteplugins/go/tasks/plugins/k8s/pod/sidecar_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/pod/sidecar_test.go @@ -84,6 +84,7 @@ func dummySidecarTaskMetadata(resources *v1.ResourceRequirements, extendedResour }, }) tID.On("GetGeneratedName").Return("my_project:my_domain:my_name") + tID.On("GetUniqueNodeID").Return("an-unique-id") taskMetadata.On("GetTaskExecutionID").Return(tID) to := &pluginsCoreMock.TaskOverrides{} diff --git a/flyteplugins/go/tasks/plugins/k8s/spark/spark_test.go b/flyteplugins/go/tasks/plugins/k8s/spark/spark_test.go index b07ab0ef33..264f9514e9 100644 --- a/flyteplugins/go/tasks/plugins/k8s/spark/spark_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/spark/spark_test.go @@ -379,6 +379,7 @@ func dummySparkTaskContext(taskTemplate *core.TaskTemplate, interruptible bool) }, }) tID.On("GetGeneratedName").Return("some-acceptable-name") + tID.On("GetUniqueNodeID").Return("an-unique-id") overrides := &mocks.TaskOverrides{} overrides.On("GetResources").Return(&corev1.ResourceRequirements{}) From 8049e31ec883e643f0657339ff4b1ccc1968430a Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Fri, 26 Jan 2024 16:31:51 -0800 Subject: [PATCH 10/19] Remove scheme from log links --- flyteplugins/go/tasks/logs/logging_utils.go | 41 +++++---- .../go/tasks/logs/logging_utils_test.go | 1 - .../tasks/pluginmachinery/tasklog/plugin.go | 48 ++++------- .../tasks/pluginmachinery/tasklog/template.go | 18 ++-- .../pluginmachinery/tasklog/template_test.go | 72 ++++++---------- .../tasklog/templatescheme_enumer.go | 85 ------------------- .../plugins/array/k8s/subtask_exec_context.go | 30 +++---- .../array/k8s/subtask_exec_context_test.go | 12 ++- .../go/tasks/plugins/k8s/pod/plugin.go | 2 +- flyteplugins/go/tasks/plugins/k8s/ray/ray.go | 14 +-- .../go/tasks/plugins/k8s/ray/ray_test.go | 2 - 11 files changed, 96 insertions(+), 229 deletions(-) delete mode 100644 flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index 44ae459f34..4a34419808 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -14,7 +14,7 @@ import ( ) // Internal -func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID pluginsCore.TaskExecutionID, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVarsByScheme *tasklog.TemplateVarsByScheme, taskTemplate *core.TaskTemplate) ([]*core.TaskLog, error) { +func GetLogsForContainerInPod(ctx context.Context, logPlugin tasklog.Plugin, taskExecID pluginsCore.TaskExecutionID, pod *v1.Pod, index uint32, nameSuffix string, extraLogTemplateVars []tasklog.TemplateVar, taskTemplate *core.TaskTemplate) ([]*core.TaskLog, error) { if logPlugin == nil { return nil, nil } @@ -39,19 +39,19 @@ 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, - TaskExecutionID: taskExecID, - ExtraTemplateVarsByScheme: extraLogTemplateVarsByScheme, - TaskTemplate: taskTemplate, + 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, + TaskExecutionID: taskExecID, + ExtraTemplateVars: extraLogTemplateVars, + TaskTemplate: taskTemplate, }, ) @@ -89,25 +89,25 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { if cfg.IsKubernetesEnabled { if len(cfg.KubernetesTemplateURI) > 0 { - plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Kubernetes Logs", Scheme: tasklog.TemplateSchemePod, TemplateURIs: []tasklog.TemplateURI{cfg.KubernetesTemplateURI}, MessageFormat: core.TaskLog_JSON}) + plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Kubernetes Logs", TemplateURIs: []tasklog.TemplateURI{cfg.KubernetesTemplateURI}, MessageFormat: core.TaskLog_JSON}) } else { - plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Kubernetes Logs", Scheme: tasklog.TemplateSchemePod, TemplateURIs: []tasklog.TemplateURI{fmt.Sprintf("%s/#!/log/{{ .namespace }}/{{ .podName }}/pod?namespace={{ .namespace }}", cfg.KubernetesURL)}, MessageFormat: core.TaskLog_JSON}) + plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Kubernetes Logs", TemplateURIs: []tasklog.TemplateURI{fmt.Sprintf("%s/#!/log/{{ .namespace }}/{{ .podName }}/pod?namespace={{ .namespace }}", cfg.KubernetesURL)}, MessageFormat: core.TaskLog_JSON}) } } if cfg.IsCloudwatchEnabled { if len(cfg.CloudwatchTemplateURI) > 0 { - plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Cloudwatch Logs", Scheme: tasklog.TemplateSchemePod, TemplateURIs: []tasklog.TemplateURI{cfg.CloudwatchTemplateURI}, MessageFormat: core.TaskLog_JSON}) + plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Cloudwatch Logs", TemplateURIs: []tasklog.TemplateURI{cfg.CloudwatchTemplateURI}, MessageFormat: core.TaskLog_JSON}) } else { - plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Cloudwatch Logs", Scheme: tasklog.TemplateSchemePod, TemplateURIs: []tasklog.TemplateURI{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)}, MessageFormat: core.TaskLog_JSON}) + plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Cloudwatch Logs", TemplateURIs: []tasklog.TemplateURI{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)}, MessageFormat: core.TaskLog_JSON}) } } if cfg.IsStackDriverEnabled { if len(cfg.StackDriverTemplateURI) > 0 { - plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Stackdriver Logs", Scheme: tasklog.TemplateSchemePod, TemplateURIs: []tasklog.TemplateURI{cfg.StackDriverTemplateURI}, MessageFormat: core.TaskLog_JSON}) + plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Stackdriver Logs", TemplateURIs: []tasklog.TemplateURI{cfg.StackDriverTemplateURI}, MessageFormat: core.TaskLog_JSON}) } else { - plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Stackdriver Logs", Scheme: tasklog.TemplateSchemePod, TemplateURIs: []tasklog.TemplateURI{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)}, MessageFormat: core.TaskLog_JSON}) + plugins = append(plugins, tasklog.TemplateLogPlugin{DisplayName: "Stackdriver Logs", TemplateURIs: []tasklog.TemplateURI{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)}, MessageFormat: core.TaskLog_JSON}) } } @@ -117,7 +117,6 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { tasklog.TemplateLogPlugin{ Name: logLinkType, DisplayName: fmt.Sprintf("%s logs", logLinkType), - Scheme: tasklog.TemplateSchemeDynamic, DynamicTemplateURIs: []tasklog.TemplateURI{ dynamicLogLink, }, diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index a0475a6169..6b7aaf5685 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -334,7 +334,6 @@ 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, }, }, }, nil, []*core.TaskLog{ diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go index c0427c11c5..c995892d97 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go @@ -7,16 +7,6 @@ import ( pluginsCore "github.com/flyteorg/flyte/flyteplugins/go/tasks/pluginmachinery/core" ) -//go:generate enumer --type=TemplateScheme --trimprefix=TemplateScheme -json -yaml - -type TemplateScheme int - -const ( - TemplateSchemePod TemplateScheme = iota - TemplateSchemeTaskExecution - TemplateSchemeDynamic -) - // TemplateURI is a URI that accepts templates. See: go/tasks/pluginmachinery/tasklog/template.go for available templates. type TemplateURI = string @@ -25,31 +15,23 @@ type TemplateVar struct { Value string } -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 - TaskExecutionID pluginsCore.TaskExecutionID - ExtraTemplateVarsByScheme *TemplateVarsByScheme - TaskTemplate *core.TaskTemplate + HostName string + PodName string + Namespace string + ContainerName string + ContainerID string + LogName string + PodRFC3339StartTime string + PodRFC3339FinishTime string + PodUnixStartTime int64 + PodUnixFinishTime int64 + PodUID string + TaskExecutionID pluginsCore.TaskExecutionID + ExtraTemplateVars []TemplateVar + TaskTemplate *core.TaskTemplate } // Output contains all task logs a plugin generates for a given Input. @@ -64,10 +46,10 @@ type Plugin interface { } type TemplateLogPlugin struct { + // TODO: these don't need pflags anymore? Name string `json:"name" pflag:",Name of the plugin."` 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."` DynamicTemplateURIs []TemplateURI `json:"dynamictemplateUris" pflag:",URI Templates for generating dynamic task log links."` MessageFormat core.TaskLog_MessageFormat `json:"messageFormat" pflag:",Log Message Format."` - Scheme TemplateScheme `json:"scheme" pflag:",Templating scheme to use. Supported values are Pod and TaskExecution."` } diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index 2bdb3f40fb..aa386ef6a9 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -73,7 +73,7 @@ func initDefaultRegexes() templateRegexes { var defaultRegexes = initDefaultRegexes() -func replaceAll(template string, vars TemplateVars) string { +func replaceAll(template string, vars []TemplateVar) string { for _, v := range vars { if len(v.Value) > 0 { template = v.Regex.ReplaceAllLiteralString(template, v.Value) @@ -82,14 +82,14 @@ func replaceAll(template string, vars TemplateVars) string { return template } -func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { - vars := TemplateVars{ - {defaultRegexes.LogName, input.LogName}, +func (input Input) templateVarsForScheme() []TemplateVar { + vars := []TemplateVar{ + TemplateVar{defaultRegexes.LogName, input.LogName}, } - gotExtraTemplateVars := input.ExtraTemplateVarsByScheme != nil + gotExtraTemplateVars := input.ExtraTemplateVars != nil if gotExtraTemplateVars { - vars = append(vars, input.ExtraTemplateVarsByScheme.Common...) + vars = append(vars, input.ExtraTemplateVars...) } port := input.TaskTemplate.GetConfig()["port"] @@ -117,7 +117,7 @@ func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { TemplateVar{defaultRegexes.Hostname, input.HostName}, ) if gotExtraTemplateVars { - vars = append(vars, input.ExtraTemplateVarsByScheme.Pod...) + vars = append(vars, input.ExtraTemplateVars...) } if input.TaskExecutionID != nil { taskExecutionIdentifier := input.TaskExecutionID.GetID() @@ -176,7 +176,7 @@ func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars { } } if gotExtraTemplateVars { - vars = append(vars, input.ExtraTemplateVarsByScheme.TaskExecution...) + vars = append(vars, input.ExtraTemplateVars...) } vars = append( @@ -212,7 +212,7 @@ func getDynamicLogLinkTypes(taskTemplate *core.TaskTemplate) []string { } func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { - templateVars := input.templateVarsForScheme(p.Scheme) + templateVars := input.templateVarsForScheme() taskLogs := make([]*core.TaskLog, 0, len(p.TemplateURIs)) for _, templateURI := range p.TemplateURIs { taskLogs = append(taskLogs, &core.TaskLog{ diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go index f62e65250c..e7001003dc 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go @@ -100,19 +100,17 @@ func Test_Input_templateVarsForScheme(t *testing.T) { tests := []struct { name string - scheme TemplateScheme baseVars Input - extraVars *TemplateVarsByScheme - exact TemplateVars - contains TemplateVars - notContains TemplateVars + extraVars []TemplateVar + exact []TemplateVar + contains []TemplateVar + notContains []TemplateVar }{ { "pod happy path", - TemplateSchemePod, podBase, nil, - TemplateVars{ + []TemplateVar{ {defaultRegexes.LogName, "main_logs"}, {defaultRegexes.PodName, "my-pod"}, {defaultRegexes.PodUID, "my-pod-uid"}, @@ -130,19 +128,14 @@ func Test_Input_templateVarsForScheme(t *testing.T) { }, { "pod with extra vars", - TemplateSchemePod, podBase, - &TemplateVarsByScheme{ - Common: TemplateVars{ - {testRegexes.Foo, "foo"}, - }, - Pod: TemplateVars{ - {testRegexes.Bar, "bar"}, - {testRegexes.Baz, "baz"}, - }, + []TemplateVar{ + {testRegexes.Foo, "foo"}, + {testRegexes.Bar, "bar"}, + {testRegexes.Baz, "baz"}, }, nil, - TemplateVars{ + []TemplateVar{ {testRegexes.Foo, "foo"}, {testRegexes.Bar, "bar"}, {testRegexes.Baz, "baz"}, @@ -151,10 +144,9 @@ func Test_Input_templateVarsForScheme(t *testing.T) { }, { "task execution happy path", - TemplateSchemeTaskExecution, taskExecutionBase, nil, - TemplateVars{ + []TemplateVar{ {defaultRegexes.LogName, "main_logs"}, {defaultRegexes.PodName, ""}, {defaultRegexes.PodUID, ""}, @@ -182,19 +174,14 @@ func Test_Input_templateVarsForScheme(t *testing.T) { }, { "task execution with extra vars", - TemplateSchemeTaskExecution, taskExecutionBase, - &TemplateVarsByScheme{ - Common: TemplateVars{ - {testRegexes.Foo, "foo"}, - }, - TaskExecution: TemplateVars{ - {testRegexes.Bar, "bar"}, - {testRegexes.Baz, "baz"}, - }, + []TemplateVar{ + {testRegexes.Foo, "foo"}, + {testRegexes.Bar, "bar"}, + {testRegexes.Baz, "baz"}, }, nil, - TemplateVars{ + []TemplateVar{ {testRegexes.Foo, "foo"}, {testRegexes.Bar, "bar"}, {testRegexes.Baz, "baz"}, @@ -220,21 +207,19 @@ func Test_Input_templateVarsForScheme(t *testing.T) { // }, { "flyin happy path", - TemplateSchemeDynamic, flyinBase, nil, nil, - TemplateVars{ + []TemplateVar{ {defaultRegexes.Port, "1234"}, }, nil, }, { "flyin and pod happy path", - TemplateSchemeDynamic, flyinBase, nil, - TemplateVars{ + []TemplateVar{ {defaultRegexes.LogName, "main_logs"}, {defaultRegexes.Port, "1234"}, {defaultRegexes.PodName, "my-pod"}, @@ -253,12 +238,11 @@ func Test_Input_templateVarsForScheme(t *testing.T) { }, { "pod with port not affected", - TemplateSchemePod, podBase, nil, nil, nil, - TemplateVars{ + []TemplateVar{ {defaultRegexes.Port, "1234"}, }, }, @@ -266,8 +250,8 @@ func Test_Input_templateVarsForScheme(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { base := tt.baseVars - base.ExtraTemplateVarsByScheme = tt.extraVars - got := base.templateVarsForScheme(tt.scheme) + base.ExtraTemplateVars = tt.extraVars + got := base.templateVarsForScheme() if tt.exact != nil { assert.Equal(t, got, tt.exact) } @@ -466,7 +450,6 @@ func TestTemplateLogPlugin(t *testing.T) { { "task-with-task-execution-identifier", TemplateLogPlugin{ - Scheme: TemplateSchemeTaskExecution, TemplateURIs: []TemplateURI{"https://flyte.corp.net/console/projects/{{ .executionProject }}/domains/{{ .executionDomain }}/executions/{{ .executionName }}/nodeId/{{ .nodeID }}/taskId/{{ .taskID }}/attempt/{{ .taskRetryAttempt }}/view/logs"}, MessageFormat: core.TaskLog_JSON, }, @@ -498,7 +481,6 @@ func TestTemplateLogPlugin(t *testing.T) { { "mapped-task-with-task-execution-identifier", TemplateLogPlugin{ - Scheme: TemplateSchemeTaskExecution, TemplateURIs: []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, }, @@ -515,12 +497,10 @@ func TestTemplateLogPlugin(t *testing.T) { PodUnixStartTime: 123, PodUnixFinishTime: 12345, TaskExecutionID: dummyTaskExecID(), - ExtraTemplateVarsByScheme: &TemplateVarsByScheme{ - TaskExecution: TemplateVars{ - {MustCreateRegex("subtaskExecutionIndex"), "1"}, - {MustCreateRegex("subtaskRetryAttempt"), "1"}, - {MustCreateRegex("subtaskParentRetryAttempt"), "0"}, - }, + ExtraTemplateVars: []TemplateVar{ + {MustCreateRegex("subtaskExecutionIndex"), "1"}, + {MustCreateRegex("subtaskRetryAttempt"), "1"}, + {MustCreateRegex("subtaskParentRetryAttempt"), "0"}, }, }, }, @@ -538,7 +518,6 @@ func TestTemplateLogPlugin(t *testing.T) { "flyin", TemplateLogPlugin{ Name: "vscode", - Scheme: TemplateSchemeDynamic, DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, MessageFormat: core.TaskLog_JSON, }, @@ -593,7 +572,6 @@ func TestTemplateLogPlugin(t *testing.T) { "flyin - no link_type in task template", TemplateLogPlugin{ Name: "vscode", - Scheme: TemplateSchemeDynamic, DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, MessageFormat: core.TaskLog_JSON, DisplayName: "Flyin Logs", diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go deleted file mode 100644 index d942a34344..0000000000 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/templatescheme_enumer.go +++ /dev/null @@ -1,85 +0,0 @@ -// Code generated by "enumer --type=TemplateScheme --trimprefix=TemplateScheme -json -yaml"; DO NOT EDIT. - -package tasklog - -import ( - "encoding/json" - "fmt" -) - -const _TemplateSchemeName = "PodTaskExecutionDynamic" - -var _TemplateSchemeIndex = [...]uint8{0, 3, 16, 23} - -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]] -} - -var _TemplateSchemeValues = []TemplateScheme{0, 1, 2} - -var _TemplateSchemeNameToValueMap = map[string]TemplateScheme{ - _TemplateSchemeName[0:3]: 0, - _TemplateSchemeName[3:16]: 1, - _TemplateSchemeName[16:23]: 2, -} - -// 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 - } - return 0, fmt.Errorf("%s does not belong to TemplateScheme values", s) -} - -// TemplateSchemeValues returns all values of the enum -func TemplateSchemeValues() []TemplateScheme { - return _TemplateSchemeValues -} - -// 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/flyteplugins/go/tasks/plugins/array/k8s/subtask_exec_context.go b/flyteplugins/go/tasks/plugins/array/k8s/subtask_exec_context.go index 8ac4d6edc0..77b3ac6501 100644 --- a/flyteplugins/go/tasks/plugins/array/k8s/subtask_exec_context.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/subtask_exec_context.go @@ -187,22 +187,20 @@ var logTemplateRegexes = struct { tasklog.MustCreateRegex("subtaskParentRetryAttempt"), } -func (s SubTaskExecutionID) TemplateVarsByScheme() *tasklog.TemplateVarsByScheme { - return &tasklog.TemplateVarsByScheme{ - TaskExecution: tasklog.TemplateVars{ - {Regex: logTemplateRegexes.ParentName, Value: s.parentName}, - { - Regex: logTemplateRegexes.ExecutionIndex, - Value: strconv.FormatUint(uint64(s.executionIndex), 10), - }, - { - Regex: logTemplateRegexes.RetryAttempt, - Value: strconv.FormatUint(s.subtaskRetryAttempt, 10), - }, - { - Regex: logTemplateRegexes.ParentRetryAttempt, - Value: strconv.FormatUint(uint64(s.taskRetryAttempt), 10), - }, +func (s SubTaskExecutionID) TemplateVarsByScheme() []tasklog.TemplateVar { + return []tasklog.TemplateVar{ + {Regex: logTemplateRegexes.ParentName, Value: s.parentName}, + { + Regex: logTemplateRegexes.ExecutionIndex, + Value: strconv.FormatUint(uint64(s.executionIndex), 10), + }, + { + Regex: logTemplateRegexes.RetryAttempt, + Value: strconv.FormatUint(s.subtaskRetryAttempt, 10), + }, + { + Regex: logTemplateRegexes.ParentRetryAttempt, + Value: strconv.FormatUint(uint64(s.taskRetryAttempt), 10), }, } } diff --git a/flyteplugins/go/tasks/plugins/array/k8s/subtask_exec_context_test.go b/flyteplugins/go/tasks/plugins/array/k8s/subtask_exec_context_test.go index c3e213e403..103980fab0 100644 --- a/flyteplugins/go/tasks/plugins/array/k8s/subtask_exec_context_test.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/subtask_exec_context_test.go @@ -36,13 +36,11 @@ 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.TemplateVarsByScheme{ - TaskExecution: tasklog.TemplateVars{ - {Regex: logTemplateRegexes.ParentName, Value: "notfound"}, - {Regex: logTemplateRegexes.ExecutionIndex, Value: "0"}, - {Regex: logTemplateRegexes.RetryAttempt, Value: "1"}, - {Regex: logTemplateRegexes.ParentRetryAttempt, Value: "0"}, - }, + []tasklog.TemplateVar{ + {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(), ) diff --git a/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go b/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go index eae0ac98b7..f72d4eb1d7 100644 --- a/flyteplugins/go/tasks/plugins/k8s/pod/plugin.go +++ b/flyteplugins/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)", nil) } -func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string, extraLogTemplateVarsByScheme *tasklog.TemplateVarsByScheme) (pluginsCore.PhaseInfo, error) { +func (plugin) GetTaskPhaseWithLogs(ctx context.Context, pluginContext k8s.PluginContext, r client.Object, logPlugin tasklog.Plugin, logSuffix string, extraLogTemplateVarsByScheme []tasklog.TemplateVar) (pluginsCore.PhaseInfo, error) { pluginState := k8s.PluginState{} _, err := pluginContext.PluginStateReader().Get(&pluginState) if err != nil { diff --git a/flyteplugins/go/tasks/plugins/k8s/ray/ray.go b/flyteplugins/go/tasks/plugins/k8s/ray/ray.go index f6abb58c93..3f8d678ef5 100644 --- a/flyteplugins/go/tasks/plugins/k8s/ray/ray.go +++ b/flyteplugins/go/tasks/plugins/k8s/ray/ray.go @@ -457,13 +457,13 @@ func getEventInfoForRayJob(logConfig logs.LogConfig, pluginContext k8s.PluginCon taskExecID := pluginContext.TaskExecutionMetadata().GetTaskExecutionID() input := tasklog.Input{ - Namespace: rayJob.Namespace, - TaskExecutionID: taskExecID, - ExtraTemplateVarsByScheme: &tasklog.TemplateVarsByScheme{}, + Namespace: rayJob.Namespace, + TaskExecutionID: taskExecID, + ExtraTemplateVars: []tasklog.TemplateVar{}, } if rayJob.Status.JobId != "" { - input.ExtraTemplateVarsByScheme.Common = append( - input.ExtraTemplateVarsByScheme.Common, + input.ExtraTemplateVars = append( + input.ExtraTemplateVars, tasklog.TemplateVar{ Regex: logTemplateRegexes.RayJobID, Value: rayJob.Status.JobId, @@ -471,8 +471,8 @@ func getEventInfoForRayJob(logConfig logs.LogConfig, pluginContext k8s.PluginCon ) } if rayJob.Status.RayClusterName != "" { - input.ExtraTemplateVarsByScheme.Common = append( - input.ExtraTemplateVarsByScheme.Common, + input.ExtraTemplateVars = append( + input.ExtraTemplateVars, tasklog.TemplateVar{ Regex: logTemplateRegexes.RayClusterName, Value: rayJob.Status.RayClusterName, diff --git a/flyteplugins/go/tasks/plugins/k8s/ray/ray_test.go b/flyteplugins/go/tasks/plugins/k8s/ray/ray_test.go index b171ae9aa8..beab938768 100644 --- a/flyteplugins/go/tasks/plugins/k8s/ray/ray_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/ray/ray_test.go @@ -738,7 +738,6 @@ func TestGetEventInfo_LogTemplates(t *testing.T) { TemplateURIs: []tasklog.TemplateURI{ "http://test/projects/{{ .executionProject }}/domains/{{ .executionDomain }}/executions/{{ .executionName }}/nodeId/{{ .nodeID }}/taskId/{{ .taskID }}/attempt/{{ .taskRetryAttempt }}", }, - Scheme: tasklog.TemplateSchemeTaskExecution, }, expectedTaskLogs: []*core.TaskLog{ { @@ -823,7 +822,6 @@ func TestGetEventInfo_DashboardURL(t *testing.T) { dashboardURLTemplate: tasklog.TemplateLogPlugin{ DisplayName: "Ray Dashboard", TemplateURIs: []tasklog.TemplateURI{"http://test/{{.generatedName}}"}, - Scheme: tasklog.TemplateSchemeTaskExecution, }, expectedTaskLogs: []*core.TaskLog{ { From 6ca8fece3f53de020d7901fb1b164ff9761888fc Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Fri, 26 Jan 2024 17:03:43 -0800 Subject: [PATCH 11/19] Evaluate templatizable taskConfig values at runtime Signed-off-by: Eduardo Apolinario --- .../tasks/pluginmachinery/tasklog/template.go | 21 ++++----- .../pluginmachinery/tasklog/template_test.go | 45 +++++++------------ 2 files changed, 26 insertions(+), 40 deletions(-) diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index aa386ef6a9..0de13d432c 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -41,7 +41,6 @@ type templateRegexes struct { ExecutionProject *regexp.Regexp ExecutionDomain *regexp.Regexp GeneratedName *regexp.Regexp - Port *regexp.Regexp } func initDefaultRegexes() templateRegexes { @@ -67,7 +66,6 @@ func initDefaultRegexes() templateRegexes { MustCreateRegex("executionProject"), MustCreateRegex("executionDomain"), MustCreateRegex("generatedName"), - MustCreateDynamicLogRegex("port"), } } @@ -82,7 +80,7 @@ func replaceAll(template string, vars []TemplateVar) string { return template } -func (input Input) templateVarsForScheme() []TemplateVar { +func (input Input) templateVars() []TemplateVar { vars := []TemplateVar{ TemplateVar{defaultRegexes.LogName, input.LogName}, } @@ -92,14 +90,6 @@ func (input Input) templateVarsForScheme() []TemplateVar { vars = append(vars, input.ExtraTemplateVars...) } - port := input.TaskTemplate.GetConfig()["port"] - if port != "" { - vars = append( - vars, - TemplateVar{defaultRegexes.Port, port}, - ) - } - // 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 @@ -193,6 +183,13 @@ func (input Input) templateVarsForScheme() []TemplateVar { }, ) + // Add values from task template config as dynamic regexes (i.e. templates prefixed by .taskConfig) + for key, value := range input.TaskTemplate.GetConfig() { + if value != "" { + vars = append(vars, TemplateVar{MustCreateDynamicLogRegex(key), value}) + } + } + return vars } @@ -212,7 +209,7 @@ func getDynamicLogLinkTypes(taskTemplate *core.TaskTemplate) []string { } func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { - templateVars := input.templateVarsForScheme() + templateVars := input.templateVars() taskLogs := make([]*core.TaskLog, 0, len(p.TemplateURIs)) for _, templateURI := range p.TemplateURIs { taskLogs = append(taskLogs, &core.TaskLog{ diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go index e7001003dc..a94af5e23f 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go @@ -43,19 +43,23 @@ func dummyTaskExecID() pluginCore.TaskExecutionID { return tID } -func Test_Input_templateVarsForScheme(t *testing.T) { +func Test_Input_templateVars(t *testing.T) { testRegexes := struct { - Foo *regexp.Regexp - Bar *regexp.Regexp - Baz *regexp.Regexp - Ham *regexp.Regexp - Spam *regexp.Regexp + Foo *regexp.Regexp + Bar *regexp.Regexp + Baz *regexp.Regexp + Ham *regexp.Regexp + Spam *regexp.Regexp + LinkType *regexp.Regexp + Port *regexp.Regexp }{ MustCreateRegex("foo"), MustCreateRegex("bar"), MustCreateRegex("baz"), MustCreateRegex("ham"), MustCreateRegex("spam"), + MustCreateDynamicLogRegex("link_type"), + MustCreateDynamicLogRegex("port"), } podBase := Input{ HostName: "my-host", @@ -93,7 +97,8 @@ func Test_Input_templateVarsForScheme(t *testing.T) { PodUnixFinishTime: 12345, TaskTemplate: &core.TaskTemplate{ Config: map[string]string{ - "port": "1234", + "link_type": "vscode", + "port": "1234", }, }, } @@ -188,30 +193,13 @@ func Test_Input_templateVarsForScheme(t *testing.T) { }, 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"}, - // }, - // }, { "flyin happy path", flyinBase, nil, nil, []TemplateVar{ - {defaultRegexes.Port, "1234"}, + {testRegexes.Port, "1234"}, }, nil, }, @@ -221,7 +209,6 @@ func Test_Input_templateVarsForScheme(t *testing.T) { nil, []TemplateVar{ {defaultRegexes.LogName, "main_logs"}, - {defaultRegexes.Port, "1234"}, {defaultRegexes.PodName, "my-pod"}, {defaultRegexes.PodUID, "my-pod-uid"}, {defaultRegexes.Namespace, "my-namespace"}, @@ -232,6 +219,8 @@ func Test_Input_templateVarsForScheme(t *testing.T) { {defaultRegexes.PodRFC3339FinishTime, "1970-01-01T04:25:45+01:00"}, {defaultRegexes.PodUnixStartTime, "123"}, {defaultRegexes.PodUnixFinishTime, "12345"}, + {testRegexes.LinkType, "vscode"}, + {testRegexes.Port, "1234"}, }, nil, nil, @@ -243,7 +232,7 @@ func Test_Input_templateVarsForScheme(t *testing.T) { nil, nil, []TemplateVar{ - {defaultRegexes.Port, "1234"}, + {testRegexes.Port, "1234"}, }, }, } @@ -251,7 +240,7 @@ func Test_Input_templateVarsForScheme(t *testing.T) { t.Run(tt.name, func(t *testing.T) { base := tt.baseVars base.ExtraTemplateVars = tt.extraVars - got := base.templateVarsForScheme() + got := base.templateVars() if tt.exact != nil { assert.Equal(t, got, tt.exact) } From 634c17cc46604e6ab76a6d85f0e13eff74f5b1f3 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Mon, 29 Jan 2024 08:53:33 -0800 Subject: [PATCH 12/19] Use capture groups to create dynamic log regexes. Signed-off-by: Eduardo Apolinario --- .../tasks/pluginmachinery/tasklog/template.go | 18 +-- .../pluginmachinery/tasklog/template_test.go | 105 +++++------------- 2 files changed, 37 insertions(+), 86 deletions(-) diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index 0de13d432c..d5e20ccb93 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -183,13 +183,6 @@ func (input Input) templateVars() []TemplateVar { }, ) - // Add values from task template config as dynamic regexes (i.e. templates prefixed by .taskConfig) - for key, value := range input.TaskTemplate.GetConfig() { - if value != "" { - vars = append(vars, TemplateVar{MustCreateDynamicLogRegex(key), value}) - } - } - return vars } @@ -219,9 +212,20 @@ func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { }) } + // Compile the potential dynamic log links + dynamicLogRegex := regexp.MustCompile(`(?i){{\s*.taskConfig[\.$]([a-zA-Z_]+)\s*}}`) + for _, dynamicLogLinkType := range getDynamicLogLinkTypes(input.TaskTemplate) { for _, dynamicTemplateURI := range p.DynamicTemplateURIs { if p.Name == dynamicLogLinkType { + for _, match := range dynamicLogRegex.FindAllStringSubmatch(dynamicTemplateURI, -1) { + if len(match) > 1 { + value := input.TaskTemplate.GetConfig()[match[1]] + if value != "" { + templateVars = append(templateVars, TemplateVar{MustCreateDynamicLogRegex(match[1]), value}) + } + } + } taskLogs = append(taskLogs, &core.TaskLog{ Uri: replaceAll(dynamicTemplateURI, templateVars), Name: p.DisplayName + input.LogName, diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go index a94af5e23f..dbbab6bb11 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go @@ -83,26 +83,6 @@ func Test_Input_templateVars(t *testing.T) { PodUnixFinishTime: 12345, } - flyinBase := 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, - TaskTemplate: &core.TaskTemplate{ - Config: map[string]string{ - "link_type": "vscode", - "port": "1234", - }, - }, - } - tests := []struct { name string baseVars Input @@ -193,38 +173,6 @@ func Test_Input_templateVars(t *testing.T) { }, nil, }, - { - "flyin happy path", - flyinBase, - nil, - nil, - []TemplateVar{ - {testRegexes.Port, "1234"}, - }, - nil, - }, - { - "flyin and pod happy path", - flyinBase, - nil, - []TemplateVar{ - {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"}, - {testRegexes.LinkType, "vscode"}, - {testRegexes.Port, "1234"}, - }, - nil, - nil, - }, { "pod with port not affected", podBase, @@ -530,33 +478,6 @@ func TestTemplateLogPlugin(t *testing.T) { }, }, }, - // { - // "flyin - default port", - // TemplateLogPlugin{ - // Name: "vscode", - // Scheme: TemplateSchemeDynamic, - // DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, - // MessageFormat: core.TaskLog_JSON, - // }, - // args{ - // input: Input{ - // PodName: "my-pod-name", - // TaskTemplate: &core.TaskTemplate{ - // Config: map[string]string{ - // "link_type": "vscode", - // }, - // }, - // }, - // }, - // Output{ - // TaskLogs: []*core.TaskLog{ - // { - // Uri: "vscode://flyin:8080/my-pod-name", - // MessageFormat: core.TaskLog_JSON, - // }, - // }, - // }, - // }, { "flyin - no link_type in task template", TemplateLogPlugin{ @@ -574,6 +495,32 @@ func TestTemplateLogPlugin(t *testing.T) { TaskLogs: []*core.TaskLog{}, }, }, + { + "kubernetes", + TemplateLogPlugin{ + TemplateURIs: []TemplateURI{"https://dashboard.k8s.net/#!/log/{{.namespace}}/{{.podName}}/pod?namespace={{.namespace}}"}, + MessageFormat: core.TaskLog_JSON, + }, + args{ + input: Input{ + PodName: "flyteexamples-development-task-name", + PodUID: "pod-uid", + Namespace: "flyteexamples-development", + ContainerName: "ignore", + 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, + }, + }, + Output{TaskLogs: []*core.TaskLog{{ + Uri: "https://dashboard.k8s.net/#!/log/flyteexamples-development/flyteexamples-development-task-name/pod?namespace=flyteexamples-development", + MessageFormat: core.TaskLog_JSON, + Name: "main_logs", + }}}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 1058d036d46c983b1c1bc88c0ce5ceb0003d471c Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Mon, 29 Jan 2024 09:02:57 -0800 Subject: [PATCH 13/19] Remove mentions to flyin Signed-off-by: Eduardo Apolinario --- .../go/tasks/logs/logging_utils_test.go | 20 +++++++++---------- .../tasks/pluginmachinery/tasklog/template.go | 11 ++-------- .../pluginmachinery/tasklog/template_test.go | 12 +++++------ 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index 6b7aaf5685..46f4f1cbe8 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -350,7 +350,7 @@ func TestGetLogsForContainerInPod_Templates(t *testing.T) { }) } -func TestGetLogsForContainerInPod_Flyin(t *testing.T) { +func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { tests := []struct { name string config *LogConfig @@ -358,27 +358,27 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { expectedTaskLogs []*core.TaskLog }{ { - "Flyin enabled but no task template", + "Flyteinteractive enabled but no task template", &LogConfig{ DynamicLogLinks: map[string]tasklog.TemplateURI{ - "vscode": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + "vscode": "https://flyteinteractive.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, nil, nil, }, { - "Flyin enabled but config not found in task template", + "Flyteinteractive enabled but config not found in task template", &LogConfig{ DynamicLogLinks: map[string]tasklog.TemplateURI{ - "vscode": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + "vscode": "https://flyteinteractive.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, &core.TaskTemplate{}, nil, }, { - "Flyin disabled but config present in TaskTemplate", + "Flyteinteractive disabled but config present in TaskTemplate", &LogConfig{}, &core.TaskTemplate{ Config: map[string]string{ @@ -389,7 +389,7 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { nil, }, { - "Flyin disabled and K8s enabled and flyin config present in TaskTemplate", + "Flyteinteractive disabled and K8s enabled and flyteinteractive config present in TaskTemplate", &LogConfig{ IsKubernetesEnabled: true, KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", @@ -409,12 +409,12 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { }, }, { - "Flyin and K8s enabled", + "Flyteinteractive and K8s enabled", &LogConfig{ IsKubernetesEnabled: true, KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", DynamicLogLinks: map[string]tasklog.TemplateURI{ - "vscode": "https://flyin.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + "vscode": "https://flyteinteractive.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", }, }, &core.TaskTemplate{ @@ -430,7 +430,7 @@ func TestGetLogsForContainerInPod_Flyin(t *testing.T) { Name: "Kubernetes Logs my-Suffix", }, { - Uri: "https://flyin.mydomain.com:65535/my-namespace/my-pod/ContainerName/ContainerID", + Uri: "https://flyteinteractive.mydomain.com:65535/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, Name: "vscode logs my-Suffix", }, diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index d5e20ccb93..c9d5bb477a 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -15,6 +15,8 @@ func MustCreateRegex(varName string) *regexp.Regexp { return regexp.MustCompile(fmt.Sprintf(`(?i){{\s*[\.$]%s\s*}}`, varName)) } +var dynamicLogRegex = regexp.MustCompile(`(?i){{\s*.taskConfig[\.$]([a-zA-Z_]+)\s*}}`) + func MustCreateDynamicLogRegex(varName string) *regexp.Regexp { return regexp.MustCompile(fmt.Sprintf(`(?i){{\s*.taskConfig[\.$]%s\s*}}`, varName)) } @@ -106,9 +108,6 @@ func (input Input) templateVars() []TemplateVar { TemplateVar{defaultRegexes.ContainerID, containerID}, TemplateVar{defaultRegexes.Hostname, input.HostName}, ) - if gotExtraTemplateVars { - vars = append(vars, input.ExtraTemplateVars...) - } if input.TaskExecutionID != nil { taskExecutionIdentifier := input.TaskExecutionID.GetID() vars = append( @@ -165,9 +164,6 @@ func (input Input) templateVars() []TemplateVar { ) } } - if gotExtraTemplateVars { - vars = append(vars, input.ExtraTemplateVars...) - } vars = append( vars, @@ -212,9 +208,6 @@ func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { }) } - // Compile the potential dynamic log links - dynamicLogRegex := regexp.MustCompile(`(?i){{\s*.taskConfig[\.$]([a-zA-Z_]+)\s*}}`) - for _, dynamicLogLinkType := range getDynamicLogLinkTypes(input.TaskTemplate) { for _, dynamicTemplateURI := range p.DynamicTemplateURIs { if p.Name == dynamicLogLinkType { diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go index dbbab6bb11..42226bd7c0 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template_test.go @@ -452,10 +452,10 @@ func TestTemplateLogPlugin(t *testing.T) { }, }, { - "flyin", + "flyteinteractive", TemplateLogPlugin{ Name: "vscode", - DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, + DynamicTemplateURIs: []TemplateURI{"vscode://flyteinteractive:{{ .taskConfig.port }}/{{ .podName }}"}, MessageFormat: core.TaskLog_JSON, }, args{ @@ -472,19 +472,19 @@ func TestTemplateLogPlugin(t *testing.T) { Output{ TaskLogs: []*core.TaskLog{ { - Uri: "vscode://flyin:1234/my-pod-name", + Uri: "vscode://flyteinteractive:1234/my-pod-name", MessageFormat: core.TaskLog_JSON, }, }, }, }, { - "flyin - no link_type in task template", + "flyteinteractive - no link_type in task template", TemplateLogPlugin{ Name: "vscode", - DynamicTemplateURIs: []TemplateURI{"vscode://flyin:{{ .taskConfig.port }}/{{ .podName }}"}, + DynamicTemplateURIs: []TemplateURI{"vscode://flyteinteractive:{{ .taskConfig.port }}/{{ .podName }}"}, MessageFormat: core.TaskLog_JSON, - DisplayName: "Flyin Logs", + DisplayName: "Flyteinteractive Logs", }, args{ input: Input{ From ea9fdaca78250e9bed937af9aa816b782ff457f5 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Mon, 29 Jan 2024 09:11:08 -0800 Subject: [PATCH 14/19] Add unit test to demo the use of duplicated dynamic log links Signed-off-by: Eduardo Apolinario --- .../go/tasks/logs/logging_utils_test.go | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index 46f4f1cbe8..887009e95e 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -388,6 +388,27 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { }, nil, }, + { + "Flyteinteractive - multiple uses of the template (invalid use of ports in a URI)", + &LogConfig{ + DynamicLogLinks: map[string]tasklog.TemplateURI{ + "vscode": "https://abc.com:{{ .taskConfig.port }}:{{ .taskConfig.port}}", + }, + }, + &core.TaskTemplate{ + Config: map[string]string{ + "link_type": "vscode", + "port": "65535", + }, + }, + []*core.TaskLog{ + { + Uri: "https://abc.com:65535:65535", + MessageFormat: core.TaskLog_JSON, + Name: "vscode logs my-Suffix", + }, + }, + }, { "Flyteinteractive disabled and K8s enabled and flyteinteractive config present in TaskTemplate", &LogConfig{ From b345864fb4d297aa8bb03e90fdfb36ad08f3e26e Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Mon, 29 Jan 2024 10:01:57 -0800 Subject: [PATCH 15/19] Simplify dynamic regex matching Signed-off-by: Eduardo Apolinario --- flyteplugins/go/tasks/pluginmachinery/tasklog/template.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index c9d5bb477a..1027050556 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -213,8 +213,7 @@ func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { if p.Name == dynamicLogLinkType { for _, match := range dynamicLogRegex.FindAllStringSubmatch(dynamicTemplateURI, -1) { if len(match) > 1 { - value := input.TaskTemplate.GetConfig()[match[1]] - if value != "" { + if value, found := input.TaskTemplate.GetConfig()[match[1]]; found { templateVars = append(templateVars, TemplateVar{MustCreateDynamicLogRegex(match[1]), value}) } } From 4200022048c9e1a4d568b1e063d8895e407bb103 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Mon, 29 Jan 2024 10:08:48 -0800 Subject: [PATCH 16/19] One more dynamic test Signed-off-by: Eduardo Apolinario --- .../go/tasks/logs/logging_utils_test.go | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index 887009e95e..22b207fe7f 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -388,6 +388,28 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { }, nil, }, + { + "Flyteinteractive - multiple dynamic options", + &LogConfig{ + DynamicLogLinks: map[string]tasklog.TemplateURI{ + "vscode": "https://abc.com:{{ .taskConfig.port }}/{{ .taskConfig.route }}", + }, + }, + &core.TaskTemplate{ + Config: map[string]string{ + "link_type": "vscode", + "port": "65535", + "route": "a-route", + }, + }, + []*core.TaskLog{ + { + Uri: "https://abc.com:65535/a-route", + MessageFormat: core.TaskLog_JSON, + Name: "vscode logs my-Suffix", + }, + }, + }, { "Flyteinteractive - multiple uses of the template (invalid use of ports in a URI)", &LogConfig{ From 5c3b813c3452f3b06af80d01c37f02bac05c3d81 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Mon, 29 Jan 2024 10:46:24 -0800 Subject: [PATCH 17/19] Remove suffix from dynamic log links display name Signed-off-by: Eduardo Apolinario --- flyteplugins/go/tasks/logs/logging_utils.go | 2 +- flyteplugins/go/tasks/logs/logging_utils_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index 4a34419808..bde0fe33a9 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -116,7 +116,7 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { dynamicPlugins, tasklog.TemplateLogPlugin{ Name: logLinkType, - DisplayName: fmt.Sprintf("%s logs", logLinkType), + DisplayName: logLinkType, DynamicTemplateURIs: []tasklog.TemplateURI{ dynamicLogLink, }, diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index 22b207fe7f..ce171bfc39 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -406,7 +406,7 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { { Uri: "https://abc.com:65535/a-route", MessageFormat: core.TaskLog_JSON, - Name: "vscode logs my-Suffix", + Name: "vscode my-Suffix", }, }, }, @@ -427,7 +427,7 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { { Uri: "https://abc.com:65535:65535", MessageFormat: core.TaskLog_JSON, - Name: "vscode logs my-Suffix", + Name: "vscode my-Suffix", }, }, }, @@ -475,7 +475,7 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { { Uri: "https://flyteinteractive.mydomain.com:65535/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, - Name: "vscode logs my-Suffix", + Name: "vscode my-Suffix", }, }, }, From 28aaa051fc1ba01749eab5a4a280c8d0e1a2e981 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Mon, 29 Jan 2024 12:07:33 -0800 Subject: [PATCH 18/19] Use a map of TemplateLogPlugin in log config Signed-off-by: Eduardo Apolinario --- flyteplugins/go/tasks/logs/config.go | 2 +- flyteplugins/go/tasks/logs/logconfig_flags.go | 1 - .../go/tasks/logs/logconfig_flags_test.go | 14 ----- flyteplugins/go/tasks/logs/logging_utils.go | 10 ++-- .../go/tasks/logs/logging_utils_test.go | 51 ++++++++++++----- .../tasks/pluginmachinery/tasklog/plugin.go | 1 - .../tasks/plugins/array/k8s/config_flags.go | 1 - .../plugins/array/k8s/config_flags_test.go | 14 ----- .../tasks/plugins/k8s/spark/config_flags.go | 4 -- .../plugins/k8s/spark/config_flags_test.go | 56 ------------------- 10 files changed, 43 insertions(+), 111 deletions(-) diff --git a/flyteplugins/go/tasks/logs/config.go b/flyteplugins/go/tasks/logs/config.go index b644dd12c4..c8eb293035 100644 --- a/flyteplugins/go/tasks/logs/config.go +++ b/flyteplugins/go/tasks/logs/config.go @@ -28,7 +28,7 @@ type LogConfig struct { StackdriverLogResourceName string `json:"stackdriver-logresourcename" pflag:",Name of the logresource in stackdriver"` StackDriverTemplateURI tasklog.TemplateURI `json:"stackdriver-template-uri" pflag:",Template Uri to use when building stackdriver log links"` - DynamicLogLinks map[string]tasklog.TemplateURI `json:"dynamic-log-links" pflag:",Map of dynamic log links"` + DynamicLogLinks map[string]tasklog.TemplateLogPlugin `json:"dynamic-log-links" pflag:"-,Map of dynamic log links"` Templates []tasklog.TemplateLogPlugin `json:"templates" pflag:"-,"` } diff --git a/flyteplugins/go/tasks/logs/logconfig_flags.go b/flyteplugins/go/tasks/logs/logconfig_flags.go index 5cf5b8f88c..00c08a8a58 100755 --- a/flyteplugins/go/tasks/logs/logconfig_flags.go +++ b/flyteplugins/go/tasks/logs/logconfig_flags.go @@ -61,6 +61,5 @@ func (cfg LogConfig) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "gcp-project"), DefaultConfig.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stackdriver-logresourcename"), DefaultConfig.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "stackdriver-template-uri"), DefaultConfig.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "dynamic-log-links"), DefaultConfig.DynamicLogLinks, "Map of dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/logs/logconfig_flags_test.go b/flyteplugins/go/tasks/logs/logconfig_flags_test.go index 6f42d36d38..8bb775df1f 100755 --- a/flyteplugins/go/tasks/logs/logconfig_flags_test.go +++ b/flyteplugins/go/tasks/logs/logconfig_flags_test.go @@ -253,18 +253,4 @@ func TestLogConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_dynamic-log-links", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "a=1,b=2" - - cmdFlags.Set("dynamic-log-links", testValue) - if vStringToString, err := cmdFlags.GetStringToString("dynamic-log-links"); err == nil { - testDecodeRaw_LogConfig(t, vStringToString, &actual.DynamicLogLinks) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) } diff --git a/flyteplugins/go/tasks/logs/logging_utils.go b/flyteplugins/go/tasks/logs/logging_utils.go index bde0fe33a9..04d23f4ec8 100644 --- a/flyteplugins/go/tasks/logs/logging_utils.go +++ b/flyteplugins/go/tasks/logs/logging_utils.go @@ -115,12 +115,10 @@ func InitializeLogPlugins(cfg *LogConfig) (tasklog.Plugin, error) { dynamicPlugins = append( dynamicPlugins, tasklog.TemplateLogPlugin{ - Name: logLinkType, - DisplayName: logLinkType, - DynamicTemplateURIs: []tasklog.TemplateURI{ - dynamicLogLink, - }, - MessageFormat: core.TaskLog_JSON, + Name: logLinkType, + DisplayName: dynamicLogLink.DisplayName, + DynamicTemplateURIs: dynamicLogLink.TemplateURIs, + MessageFormat: core.TaskLog_JSON, }) } diff --git a/flyteplugins/go/tasks/logs/logging_utils_test.go b/flyteplugins/go/tasks/logs/logging_utils_test.go index ce171bfc39..946d069d50 100644 --- a/flyteplugins/go/tasks/logs/logging_utils_test.go +++ b/flyteplugins/go/tasks/logs/logging_utils_test.go @@ -360,8 +360,13 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { { "Flyteinteractive enabled but no task template", &LogConfig{ - DynamicLogLinks: map[string]tasklog.TemplateURI{ - "vscode": "https://flyteinteractive.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + DynamicLogLinks: map[string]tasklog.TemplateLogPlugin{ + "vscode": tasklog.TemplateLogPlugin{ + DisplayName: "vscode link", + TemplateURIs: []tasklog.TemplateURI{ + "https://flyteinteractive.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + }, + }, }, }, nil, @@ -370,8 +375,13 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { { "Flyteinteractive enabled but config not found in task template", &LogConfig{ - DynamicLogLinks: map[string]tasklog.TemplateURI{ - "vscode": "https://flyteinteractive.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + DynamicLogLinks: map[string]tasklog.TemplateLogPlugin{ + "vscode": tasklog.TemplateLogPlugin{ + DisplayName: "vscode link", + TemplateURIs: []tasklog.TemplateURI{ + "https://flyteinteractive.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + }, + }, }, }, &core.TaskTemplate{}, @@ -391,8 +401,13 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { { "Flyteinteractive - multiple dynamic options", &LogConfig{ - DynamicLogLinks: map[string]tasklog.TemplateURI{ - "vscode": "https://abc.com:{{ .taskConfig.port }}/{{ .taskConfig.route }}", + DynamicLogLinks: map[string]tasklog.TemplateLogPlugin{ + "vscode": tasklog.TemplateLogPlugin{ + DisplayName: "vscode link", + TemplateURIs: []tasklog.TemplateURI{ + "https://abc.com:{{ .taskConfig.port }}/{{ .taskConfig.route }}", + }, + }, }, }, &core.TaskTemplate{ @@ -406,15 +421,20 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { { Uri: "https://abc.com:65535/a-route", MessageFormat: core.TaskLog_JSON, - Name: "vscode my-Suffix", + Name: "vscode link my-Suffix", }, }, }, { "Flyteinteractive - multiple uses of the template (invalid use of ports in a URI)", &LogConfig{ - DynamicLogLinks: map[string]tasklog.TemplateURI{ - "vscode": "https://abc.com:{{ .taskConfig.port }}:{{ .taskConfig.port}}", + DynamicLogLinks: map[string]tasklog.TemplateLogPlugin{ + "vscode": tasklog.TemplateLogPlugin{ + DisplayName: "vscode link", + TemplateURIs: []tasklog.TemplateURI{ + "https://abc.com:{{ .taskConfig.port }}:{{ .taskConfig.port}}", + }, + }, }, }, &core.TaskTemplate{ @@ -427,7 +447,7 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { { Uri: "https://abc.com:65535:65535", MessageFormat: core.TaskLog_JSON, - Name: "vscode my-Suffix", + Name: "vscode link my-Suffix", }, }, }, @@ -456,8 +476,13 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { &LogConfig{ IsKubernetesEnabled: true, KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", - DynamicLogLinks: map[string]tasklog.TemplateURI{ - "vscode": "https://flyteinteractive.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + DynamicLogLinks: map[string]tasklog.TemplateLogPlugin{ + "vscode": tasklog.TemplateLogPlugin{ + DisplayName: "vscode link", + TemplateURIs: []tasklog.TemplateURI{ + "https://flyteinteractive.mydomain.com:{{ .taskConfig.port }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", + }, + }, }, }, &core.TaskTemplate{ @@ -475,7 +500,7 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { { Uri: "https://flyteinteractive.mydomain.com:65535/my-namespace/my-pod/ContainerName/ContainerID", MessageFormat: core.TaskLog_JSON, - Name: "vscode my-Suffix", + Name: "vscode link my-Suffix", }, }, }, diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go index c995892d97..1caf8a2bac 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/plugin.go @@ -46,7 +46,6 @@ type Plugin interface { } type TemplateLogPlugin struct { - // TODO: these don't need pflags anymore? Name string `json:"name" pflag:",Name of the plugin."` 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."` diff --git a/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go b/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go index a593c5879c..1497f64bdc 100755 --- a/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/config_flags.go @@ -70,6 +70,5 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.gcp-project"), defaultConfig.LogConfig.Config.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.stackdriver-logresourcename"), defaultConfig.LogConfig.Config.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.config.stackdriver-template-uri"), defaultConfig.LogConfig.Config.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.config.dynamic-log-links"), defaultConfig.LogConfig.Config.DynamicLogLinks, "Map of dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go b/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go index 8dbce02379..3737c45c3b 100755 --- a/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go +++ b/flyteplugins/go/tasks/plugins/array/k8s/config_flags_test.go @@ -379,18 +379,4 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_logs.config.dynamic-log-links", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "a=1,b=2" - - cmdFlags.Set("logs.config.dynamic-log-links", testValue) - if vStringToString, err := cmdFlags.GetStringToString("logs.config.dynamic-log-links"); err == nil { - testDecodeRaw_Config(t, vStringToString, &actual.LogConfig.Config.DynamicLogLinks) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) } diff --git a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go index b8807386e3..d5d6945f71 100755 --- a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go +++ b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags.go @@ -62,7 +62,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.gcp-project"), defaultConfig.LogConfig.Mixed.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.stackdriver-logresourcename"), defaultConfig.LogConfig.Mixed.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.mixed.stackdriver-template-uri"), defaultConfig.LogConfig.Mixed.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.mixed.dynamic-log-links"), defaultConfig.LogConfig.Mixed.DynamicLogLinks, "Map of dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-enabled"), defaultConfig.LogConfig.User.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-region"), defaultConfig.LogConfig.User.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.cloudwatch-log-group"), defaultConfig.LogConfig.User.CloudwatchLogGroup, "Log group to which streams are associated.") @@ -74,7 +73,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.gcp-project"), defaultConfig.LogConfig.User.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.stackdriver-logresourcename"), defaultConfig.LogConfig.User.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.user.stackdriver-template-uri"), defaultConfig.LogConfig.User.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.user.dynamic-log-links"), defaultConfig.LogConfig.User.DynamicLogLinks, "Map of dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-enabled"), defaultConfig.LogConfig.System.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-region"), defaultConfig.LogConfig.System.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.cloudwatch-log-group"), defaultConfig.LogConfig.System.CloudwatchLogGroup, "Log group to which streams are associated.") @@ -86,7 +84,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.gcp-project"), defaultConfig.LogConfig.System.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.stackdriver-logresourcename"), defaultConfig.LogConfig.System.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.system.stackdriver-template-uri"), defaultConfig.LogConfig.System.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.system.dynamic-log-links"), defaultConfig.LogConfig.System.DynamicLogLinks, "Map of dynamic log links") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-enabled"), defaultConfig.LogConfig.AllUser.IsCloudwatchEnabled, "Enable Cloudwatch Logging") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-region"), defaultConfig.LogConfig.AllUser.CloudwatchRegion, "AWS region in which Cloudwatch logs are stored.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.cloudwatch-log-group"), defaultConfig.LogConfig.AllUser.CloudwatchLogGroup, "Log group to which streams are associated.") @@ -98,6 +95,5 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.gcp-project"), defaultConfig.LogConfig.AllUser.GCPProjectName, "Name of the project in GCP") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.stackdriver-logresourcename"), defaultConfig.LogConfig.AllUser.StackdriverLogResourceName, "Name of the logresource in stackdriver") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "logs.all-user.stackdriver-template-uri"), defaultConfig.LogConfig.AllUser.StackDriverTemplateURI, "Template Uri to use when building stackdriver log links") - cmdFlags.StringToString(fmt.Sprintf("%v%v", prefix, "logs.all-user.dynamic-log-links"), defaultConfig.LogConfig.AllUser.DynamicLogLinks, "Map of dynamic log links") return cmdFlags } diff --git a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go index c8e16e6d7d..ea8659a48a 100755 --- a/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go +++ b/flyteplugins/go/tasks/plugins/k8s/spark/config_flags_test.go @@ -267,20 +267,6 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_logs.mixed.dynamic-log-links", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "a=1,b=2" - - cmdFlags.Set("logs.mixed.dynamic-log-links", testValue) - if vStringToString, err := cmdFlags.GetStringToString("logs.mixed.dynamic-log-links"); err == nil { - testDecodeRaw_Config(t, vStringToString, &actual.LogConfig.Mixed.DynamicLogLinks) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) t.Run("Test_logs.user.cloudwatch-enabled", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -435,20 +421,6 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_logs.user.dynamic-log-links", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "a=1,b=2" - - cmdFlags.Set("logs.user.dynamic-log-links", testValue) - if vStringToString, err := cmdFlags.GetStringToString("logs.user.dynamic-log-links"); err == nil { - testDecodeRaw_Config(t, vStringToString, &actual.LogConfig.User.DynamicLogLinks) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) t.Run("Test_logs.system.cloudwatch-enabled", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -603,20 +575,6 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_logs.system.dynamic-log-links", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "a=1,b=2" - - cmdFlags.Set("logs.system.dynamic-log-links", testValue) - if vStringToString, err := cmdFlags.GetStringToString("logs.system.dynamic-log-links"); err == nil { - testDecodeRaw_Config(t, vStringToString, &actual.LogConfig.System.DynamicLogLinks) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) t.Run("Test_logs.all-user.cloudwatch-enabled", func(t *testing.T) { t.Run("Override", func(t *testing.T) { @@ -771,18 +729,4 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) - t.Run("Test_logs.all-user.dynamic-log-links", func(t *testing.T) { - - t.Run("Override", func(t *testing.T) { - testValue := "a=1,b=2" - - cmdFlags.Set("logs.all-user.dynamic-log-links", testValue) - if vStringToString, err := cmdFlags.GetStringToString("logs.all-user.dynamic-log-links"); err == nil { - testDecodeRaw_Config(t, vStringToString, &actual.LogConfig.AllUser.DynamicLogLinks) - - } else { - assert.FailNow(t, err.Error()) - } - }) - }) } From d013b3188c22fc5fba6c67188df3a983498098f6 Mon Sep 17 00:00:00 2001 From: Eduardo Apolinario Date: Mon, 29 Jan 2024 14:12:54 -0800 Subject: [PATCH 19/19] Rename taskConfig regex Signed-off-by: Eduardo Apolinario --- flyteplugins/go/tasks/pluginmachinery/tasklog/template.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go index 1027050556..e5481ecfbd 100644 --- a/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go +++ b/flyteplugins/go/tasks/pluginmachinery/tasklog/template.go @@ -10,12 +10,10 @@ import ( ) func MustCreateRegex(varName string) *regexp.Regexp { - // TODO: Is this regex correct? Why are we allowing $? Shouldn't it be {{\s*\.%s\s*}}? - // Also, case insensitive matching is enabled. Is this correct? return regexp.MustCompile(fmt.Sprintf(`(?i){{\s*[\.$]%s\s*}}`, varName)) } -var dynamicLogRegex = regexp.MustCompile(`(?i){{\s*.taskConfig[\.$]([a-zA-Z_]+)\s*}}`) +var taskConfigVarRegex = regexp.MustCompile(`(?i){{\s*.taskConfig[\.$]([a-zA-Z_]+)\s*}}`) func MustCreateDynamicLogRegex(varName string) *regexp.Regexp { return regexp.MustCompile(fmt.Sprintf(`(?i){{\s*.taskConfig[\.$]%s\s*}}`, varName)) @@ -211,7 +209,7 @@ func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) { for _, dynamicLogLinkType := range getDynamicLogLinkTypes(input.TaskTemplate) { for _, dynamicTemplateURI := range p.DynamicTemplateURIs { if p.Name == dynamicLogLinkType { - for _, match := range dynamicLogRegex.FindAllStringSubmatch(dynamicTemplateURI, -1) { + for _, match := range taskConfigVarRegex.FindAllStringSubmatch(dynamicTemplateURI, -1) { if len(match) > 1 { if value, found := input.TaskTemplate.GetConfig()[match[1]]; found { templateVars = append(templateVars, TemplateVar{MustCreateDynamicLogRegex(match[1]), value})