add hostname to input

[novahow]

Tracking issue

https://github.com/flyteorg/flyte/issues/

closes #4750

Why are the changes needed?

hostname not resolvable in plugin's GetTaskLogs

What changes were proposed in this pull request?

add a hostname parameter to the Input in GetTaskLogs method.

How was this patch tested?

cd flyteplugins
make test

Setup process

Screenshots

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Related PRs

Docs link

[welcome]

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

• Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
• Sign off your commits (Reference: DCO Guide).

[pingsutw]

Thanks, could you add a unit test to this file? https://github.com/flyteorg/flyte/blob/a4ab37a852ee796996b356540984b702153bfa67/flyteplugins/go/tasks/logs/logging_utils_test.go

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.97%. Comparing base (df1b034) to head (b14a00e).
Report is 62 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4939      +/-   ##
==========================================
- Coverage   58.98%   58.97%   -0.02%     
==========================================
  Files         644      645       +1     
  Lines       55145    55562     +417     
==========================================
+ Hits        32528    32767     +239     
- Misses      20043    20200     +157     
- Partials     2574     2595      +21     

Flag	Coverage Δ
unittests	58.97% <100.00%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[novahow]

Thanks, could you add a unit test to this file? https://github.com/flyteorg/flyte/blob/a4ab37a852ee796996b356540984b702153bfa67/flyteplugins/go/tasks/logs/logging_utils_test.go

Added a basic test. Not sure if the added test is legitimate though. Is template testing, for example something like

func TestGetLogsForContainerInPodTemplates_Hostname(t *testing.T) {
	assertTestSucceeded(t, &LogConfig{
		Templates: []tasklog.TemplateLogPlugin{
			{
				DisplayName: "StackDriver",
				TemplateURIs: []string{
					"{{ .hostname }}/{{ .namespace }}/{{ .podName }}/{{ .containerName }}/{{ .containerId }}",
				},
				MessageFormat: core.TaskLog_JSON,
			},
		},
	}, nil, []*core.TaskLog{
		{
			Uri:           "my-hostname/my-namespace/my-pod/ContainerName/ContainerID",
			MessageFormat: core.TaskLog_JSON,
			Name:          "StackDriver my-Suffix",
		},
	}, "my-hostname")
}

func assertTestSucceeded(tb testing.TB, config *LogConfig, taskTemplate *core.TaskTemplate, expectedTaskLogs []*core.TaskLog, hostname string) {
	logPlugin, err := InitializeLogPlugins(config)
	assert.NoError(tb, err)

	pod := &v1.Pod{
		ObjectMeta: v12.ObjectMeta{
			Namespace: "my-namespace",
			Name:      "my-pod",
		},
		Spec: v1.PodSpec{
			Containers: []v1.Container{
				{
					Name: "ContainerName",
				},
			},
			Hostname: hostname,
		},
		Status: v1.PodStatus{
			ContainerStatuses: []v1.ContainerStatus{
				{
					ContainerID: "ContainerID",
				},
			},
		},
	}

	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 {
		assert.FailNowf(tb, "Not Equal.", "Diff: %v", diff)
	}
}

needed?

[eapolinario]

@novahow , can you add this test you're proposing? It tests the templating logic.

[welcome]

Congrats on merging your first pull request! 🎉