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

[Docs] .hostname is not available in log templates #4750

Closed
2 tasks done
olyashok opened this issue Jan 19, 2024 · 4 comments · Fixed by #4939
Closed
2 tasks done

[Docs] .hostname is not available in log templates #4750

olyashok opened this issue Jan 19, 2024 · 4 comments · Fixed by #4939
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@olyashok
Copy link

Description

.hostname is mentioned as variable in several places in docs but actually is not resolveable by code

https://github.com/flyteorg/flyteplugins/blob/b0684d97a1cf240f1a44f310f4a79cc21844caa9/go/tasks/pluginmachinery/tasklog/template.go#L69

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

  • Yes

Have you read the Code of Conduct?

  • Yes
@olyashok olyashok added documentation Improvements or additions to documentation untriaged This issues has not yet been looked at by the Maintainers labels Jan 19, 2024
Copy link

dosubot bot commented Jan 19, 2024

🤖

Hey @olyashok! Good to see you back. Hope you're doing well!

Thank you for bringing this to our attention. Based on the code in the Flyte repository, the .hostname variable is indeed resolvable.

The initDefaultRegexes() function creates a regular expression for the hostname variable using the MustCreateRegex() function. This regular expression is then stored in the Hostname field of the templateRegexes struct.

Later, in the templateVarsForScheme() function, the Hostname regular expression is used to create a TemplateVar struct, which pairs the regular expression with the actual value of the hostname (input.HostName). This is done in the TemplateSchemePod case of the switch statement.

Finally, in the GetTaskLogs() function, the replaceAll() function is called, which replaces all instances of the hostname variable in the template string with the actual value of the hostname.

Here is the relevant code:

func initDefaultRegexes() templateRegexes {
	return templateRegexes{
		// ...
		MustCreateRegex("hostname"),
		// ...
	}
}

// ...

func (input Input) templateVarsForScheme(scheme TemplateScheme) TemplateVars {
	// ...
	case TemplateSchemePod:
		// ...
		vars = append(
			vars,
			TemplateVar{defaultRegexes.Hostname, input.HostName},
		)
		// ...
	}
	// ...
}

// ...

func (p TemplateLogPlugin) GetTaskLogs(input Input) (Output, error) {
	// ...
		Uri:           replaceAll(templateURI, templateVars),
	// ...
}

As for the discrepancy between the Flyte documentation and the actual implementation of the .hostname variable in the codebase, I wasn't able to find an answer within the repository. The same goes for any known issues or bugs related to the .hostname variable in the Flyte codebase.

If you have more specific information or examples that could help clarify the issue, it would be greatly appreciated.

Sources


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@dosubot dosubot bot added the bug Something isn't working label Jan 19, 2024
@eapolinario
Copy link
Contributor

good catch! Hostname is indeed not being handled right now, although it should be very easy to add. Before we add it, it's worth understanding how hostname is set in k8s, which is described here. We simply have to add hostname when instantiating taskLog.Input here.

@eapolinario eapolinario added good first issue Good for newcomers and removed untriaged This issues has not yet been looked at by the Maintainers labels Jan 23, 2024
@novahow
Copy link
Contributor

novahow commented Feb 3, 2024

I am interested in this issue.

@eapolinario
Copy link
Contributor

@novahow , feel free to work on it. Tag me on the PR and I'll gladly review it.

@eapolinario eapolinario removed the documentation Improvements or additions to documentation label Feb 6, 2024
@novahow novahow mentioned this issue Feb 23, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants