-
Notifications
You must be signed in to change notification settings - Fork 700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for generic dynamic log links #5066
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -566,3 +566,204 @@ func TestGetLogsForContainerInPod_Flyteinteractive(t *testing.T) { | |||||||||||
}) | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
func TestGetLogsForContainerInPod_GenericDynamicLogLinks(t *testing.T) { | ||||||||||||
tests := []struct { | ||||||||||||
name string | ||||||||||||
config *LogConfig | ||||||||||||
template *core.TaskTemplate | ||||||||||||
expectedTaskLogs []*core.TaskLog | ||||||||||||
}{ | ||||||||||||
{ | ||||||||||||
"Generic Dynamic Log Links enabled but no task template", | ||||||||||||
&LogConfig{ | ||||||||||||
GenericDynamicLogLinksEnabled: true, | ||||||||||||
}, | ||||||||||||
nil, | ||||||||||||
nil, | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
"Generic Dynamic Log Links enabled and empty task template", | ||||||||||||
&LogConfig{ | ||||||||||||
GenericDynamicLogLinksEnabled: true, | ||||||||||||
}, | ||||||||||||
&core.TaskTemplate{}, | ||||||||||||
nil, | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
"Generic Dynamic Log Links enabled and simple config present in TaskTemplate", | ||||||||||||
&LogConfig{ | ||||||||||||
GenericDynamicLogLinksEnabled: true, | ||||||||||||
}, | ||||||||||||
&core.TaskTemplate{ | ||||||||||||
Config: map[string]string{ | ||||||||||||
"link_type": "vscode", | ||||||||||||
"port": "65535", | ||||||||||||
"generic_dynamic_log_links": `{"links": [{"name": "vscode", "display_name": "vscode", "template_uri": "vscode://def.com:{{ .taskConfig.port }}/{{ .podName }}"}]}`, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
[]*core.TaskLog{ | ||||||||||||
{ | ||||||||||||
Uri: "vscode://def.com:65535/my-pod", | ||||||||||||
MessageFormat: core.TaskLog_JSON, | ||||||||||||
Name: "vscode", | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
"Generic Dynamic Log Links disabled and simple config present in TaskTemplate - Notice how we can mix and match parameters defined in the task template, i.e. using {{ .taskConfig.port }} also works", | ||||||||||||
&LogConfig{}, | ||||||||||||
&core.TaskTemplate{ | ||||||||||||
Config: map[string]string{ | ||||||||||||
"link_type": "vscode", | ||||||||||||
"port": "65535", | ||||||||||||
"generic_dynamic_log_links": `{"links": [{"name": "vscode", "display_name": "vscode", "template_uri": "vscode://def.com:{{ .taskConfig.port }}/{{ .podName }}"}]}`, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
nil, | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
"Generic Dynamic Log Links - multiple dynamic options", | ||||||||||||
&LogConfig{ | ||||||||||||
GenericDynamicLogLinksEnabled: true, | ||||||||||||
DynamicLogLinks: map[string]tasklog.TemplateLogPlugin{ | ||||||||||||
"vscode": tasklog.TemplateLogPlugin{ | ||||||||||||
DisplayName: "vscode link", | ||||||||||||
TemplateURIs: []tasklog.TemplateURI{ | ||||||||||||
"https://abc.com:{{ .taskConfig.port }}/{{ .taskConfig.route }}", | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
&core.TaskTemplate{ | ||||||||||||
Config: map[string]string{ | ||||||||||||
"link_type": "vscode", | ||||||||||||
"port": "65535", | ||||||||||||
"route": "a-route", | ||||||||||||
"generic_dynamic_log_links": `{"links": [{"name": "vscode", "display_name": "Other logs", "template_uri": "coming://from/other/place"}]}`, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
[]*core.TaskLog{ | ||||||||||||
{ | ||||||||||||
Uri: "https://abc.com:65535/a-route", | ||||||||||||
MessageFormat: core.TaskLog_JSON, | ||||||||||||
Name: "vscode link my-Suffix", | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
Uri: "coming://from/other/place", | ||||||||||||
MessageFormat: core.TaskLog_JSON, | ||||||||||||
Name: "Other logs", | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
"Generic Dynamic Log Links disabled and K8s enabled and generic dynamic log links config present in TaskTemplate", | ||||||||||||
&LogConfig{ | ||||||||||||
IsKubernetesEnabled: true, | ||||||||||||
KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", | ||||||||||||
GenericDynamicLogLinksEnabled: false, | ||||||||||||
}, | ||||||||||||
&core.TaskTemplate{ | ||||||||||||
Config: map[string]string{ | ||||||||||||
"link_type": "abc", | ||||||||||||
"generic_dynamic_log_links": `{"links": [{"display_name": "Other logs", "template_uri": "coming://from/other/place"}]}`, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
[]*core.TaskLog{ | ||||||||||||
{ | ||||||||||||
Uri: "https://k8s.com/my-namespace/my-pod/ContainerName/ContainerID", | ||||||||||||
MessageFormat: core.TaskLog_JSON, | ||||||||||||
Name: "Kubernetes Logs my-Suffix", | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
"Generic Dynamic Log Links enabled and K8s enabled and generic dynamic log links config present in TaskTemplate", | ||||||||||||
&LogConfig{ | ||||||||||||
IsKubernetesEnabled: true, | ||||||||||||
KubernetesTemplateURI: "https://k8s.com/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}", | ||||||||||||
GenericDynamicLogLinksEnabled: true, | ||||||||||||
}, | ||||||||||||
&core.TaskTemplate{ | ||||||||||||
Config: map[string]string{ | ||||||||||||
"link_type": "abc", | ||||||||||||
"generic_dynamic_log_links": `{"links": [{"name": "abc", "display_name": "Other logs", "template_uri": "coming://from/other/place"}]}`, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
[]*core.TaskLog{ | ||||||||||||
{ | ||||||||||||
Uri: "https://k8s.com/my-namespace/my-pod/ContainerName/ContainerID", | ||||||||||||
MessageFormat: core.TaskLog_JSON, | ||||||||||||
Name: "Kubernetes Logs my-Suffix", | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
Uri: "coming://from/other/place", | ||||||||||||
MessageFormat: core.TaskLog_JSON, | ||||||||||||
Name: "Other logs", | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
"Multiple Generic Dynamic Log Links defined in task template", | ||||||||||||
&LogConfig{ | ||||||||||||
GenericDynamicLogLinksEnabled: true, | ||||||||||||
}, | ||||||||||||
&core.TaskTemplate{ | ||||||||||||
Config: map[string]string{ | ||||||||||||
"link_type": "abc,defg", | ||||||||||||
"generic_dynamic_log_links": `{"links": [{"name": "abc", "display_name": "Other logs", "template_uri": "coming://from/abc"},{"name": "defg", "display_name": "defg logs", "template_uri": "coming://from/defg"}]}`, | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider splitting long configuration line
Consider splitting the long line containing Code suggestionCheck the AI-generated fix before applying
Suggested change
Code Review Run #51871a Is this a valid issue, or was it incorrectly flagged by the Agent?
|
||||||||||||
}, | ||||||||||||
}, | ||||||||||||
[]*core.TaskLog{ | ||||||||||||
{ | ||||||||||||
Uri: "coming://from/abc", | ||||||||||||
MessageFormat: core.TaskLog_JSON, | ||||||||||||
Name: "Other logs", | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
Uri: "coming://from/defg", | ||||||||||||
MessageFormat: core.TaskLog_JSON, | ||||||||||||
Name: "defg logs", | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
"Multiple Generic Dynamic Log Links defined in task template - one defined as a Generic Dynamic Log Link and a regular Dynamic Log Link", | ||||||||||||
&LogConfig{ | ||||||||||||
GenericDynamicLogLinksEnabled: true, | ||||||||||||
DynamicLogLinks: map[string]tasklog.TemplateLogPlugin{ | ||||||||||||
"abc": tasklog.TemplateLogPlugin{ | ||||||||||||
DisplayName: "abc link", | ||||||||||||
TemplateURIs: []tasklog.TemplateURI{ | ||||||||||||
"coming://from/abc", | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
&core.TaskTemplate{ | ||||||||||||
Config: map[string]string{ | ||||||||||||
// Notice that only defg is defined as a generic dynamic log link | ||||||||||||
"link_type": "abc,defg", | ||||||||||||
"generic_dynamic_log_links": `{"links": [{"name": "defg", "display_name": "defg logs", "template_uri": "coming://from/defg"}]}`, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
[]*core.TaskLog{ | ||||||||||||
{ | ||||||||||||
Uri: "coming://from/abc", | ||||||||||||
MessageFormat: core.TaskLog_JSON, | ||||||||||||
Name: "abc link my-Suffix", | ||||||||||||
}, | ||||||||||||
{ | ||||||||||||
Uri: "coming://from/defg", | ||||||||||||
MessageFormat: core.TaskLog_JSON, | ||||||||||||
Name: "defg logs", | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
} | ||||||||||||
for _, tt := range tests { | ||||||||||||
t.Run(tt.name, func(t *testing.T) { | ||||||||||||
assertTestSucceeded(t, tt.config, tt.template, tt.expectedTaskLogs, "") | ||||||||||||
}) | ||||||||||||
} | ||||||||||||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using
[]tasklog.TemplateURI
instead of[]string
forDynamicTemplateURIs
to maintain type consistency. TheTemplateLogPlugin
struct expectsDynamicTemplateURIs
to be of type[]TemplateURI
.Code suggestion
Code Review Run #51871a
Is this a valid issue, or was it incorrectly flagged by the Agent?