Skip to content
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

[BUG] hostname v.s. nodeName in logging link template #6086

Closed
2 tasks done
ketian-indeed opened this issue Dec 5, 2024 · 3 comments · Fixed by #6088
Closed
2 tasks done

[BUG] hostname v.s. nodeName in logging link template #6086

ketian-indeed opened this issue Dec 5, 2024 · 3 comments · Fixed by #6088
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation help wanted Extra attention is needed

Comments

@ketian-indeed
Copy link
Contributor

Describe the bug

It's about the {{ .hostname }} parameter to generate a templated logging link.
As per the doc (https://docs.flyte.org/en/latest/user_guide/productionizing/configuring_logging_links_in_the_ui.html), this parameter is

The hostname where the pod is running and logs reside

I interpreted it as the specific node's hostname where the pod has been scheduled to run, i.e., the pod's spec.nodeName.

However, it's currently set to the pod's spec.hostname, which is used to override the hostname the pod uses internally within its own network namespace:
https://github.com/flyteorg/flyte/blob/v1.13.3/flyteplugins/go/tasks/logs/logging_utils.go#L57

In practice, the node name is likely more useful for generating a logging link than the spec.hostname (actually, we're using it to generate a Datadog dashboard link and need this to query the host metrics).

Expected behavior

I expect {{ .hostname }} parameter to be the hostname where the pod has been scheduled to run (pod.Spec.NodeName) instead of pod.Spec.Hostname.

If we believe it makes sense to have {{ .hostname }} parameter presenting pod.Spec.Hostname. It's better to update the doc mentioned above and also add a new parameter e.g. {{ .nodeName }}.

Thanks!

Additional context to reproduce

No response

Screenshots

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@ketian-indeed ketian-indeed added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Dec 5, 2024
Copy link

welcome bot commented Dec 5, 2024

Thank you for opening your first issue here! 🛠

@eapolinario eapolinario added documentation Improvements or additions to documentation help wanted Extra attention is needed and removed untriaged This issues has not yet been looked at by the Maintainers labels Dec 5, 2024
@eapolinario
Copy link
Contributor

@ketian-indeed , feel free to open a PR to disambiguate the meaning there. Here's the file to edit: https://github.com/flyteorg/flyte/blob/master/docs/user_guide/productionizing/configuring_logging_links_in_the_ui.md. Thank you!

@ketian-indeed
Copy link
Contributor Author

@eapolinario, thanks! I filed #6088.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation help wanted Extra attention is needed
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants