[GSoC] Add New Parameter in tune

[Electronic-Waste]

What this PR does / why we need it:

I add a new parameter metrics_collector_config to tune function. Design details: https://github.com/kubeflow/katib/blob/cc95ef03cb25df3d86a1a1c20c9c69bad17fce92/docs/proposals/push-based-metrics-collection.md#add-new-parameter-in-tune

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Checklist:

• Docs included if any changes are user facing

[Electronic-Waste]

ref issue: #2340

[Electronic-Waste]

PTAL👀 @andreyvelich @johnugeorge

[Electronic-Waste]

@andreyvelich I add my code here:

katib/pkg/webhook/v1beta1/pod/inject_webhook.go

Lines 140 to 142 in db17214

	// Add Katib Trial labels to the Pod metadata.
	mutatePodMetadata(mutatedPod, trial)

// Pass env variable KATIB_TRIAL_NAME to training containers using fieldPath.
for idx := range mutatedPod.Spec.Containers {
	if mutatedPod.Spec.Containers[idx].Env == nil {
		mutatedPod.Spec.Containers[idx].Env = []v1.EnvVar{}
	}
	mutatedPod.Spec.Containers[idx].Env = append(
		mutatedPod.Spec.Containers[idx].Env, 
		v1.EnvVar{
			Name: consts.EnvTrialName,
			ValueFrom: &v1.EnvVarSource{
				FieldRef: &v1.ObjectFieldSelector{
					FieldPath: fmt.Sprintf("metadata.labels['%s']", consts.LabelTrialName),
				},
			},
		},
	)
}

PTAL👀. Thank you for your review!

[Electronic-Waste]

/area gsoc

[andreyvelich]

I don't think, we should mutate KATIB_TRIAL_NAME env to all containers in the Pod.
So maybe, similar as in wrapWorkerContainer function, we should only add this environment to the primary container.

[andreyvelich]

Please rebase your PR as well @Electronic-Waste

[andreyvelich]

@Electronic-Waste Additionally, I think we forgot to add this line to the post-gen script of Katib SDK:

# Import Katib report metrics functions
from kubeflow.katib.api.report_metrics import report_metrics

Similar to how we include other imports to the__init__.py.
Since this file is auto-generated by code-gen, we need to update it via post-gen script.

@Electronic-Waste Please update this script, otherwise after we re-generate the SDK, your import will be deleted.

[Electronic-Waste]

@andreyvelich Done! I've rebased the branch, modified the code to pass env variable only to the primary container and added the importing code to the post-gen script of Katib SDK.

[Electronic-Waste]

@andreyvelich Thank you for your detailed review!

[andreyvelich]

@Electronic-Waste Additionally, since we always pass the Trial name to the training container, we should not allow to pass Trial name and namespace via template generator

[Electronic-Waste]

I guess it's not friendly to users. Since junior users are unfamiliar with that, they may be puzzled about why they can't pass Trial's namespace and name to the training container. It's more straightforward for them to simply define trialParameter trialName and use it in the pod env as an example suggests.

Also, it's more backward-compatible to just allow users passing Trial name and namespace via triallParameter and pod env. WDYT👀 @andreyvelich @tenzen-y @johnugeorge .

[andreyvelich]

Since junior users are unfamiliar with that, they may be puzzled about why they can't pass Trial's namespace and name to the training container.

Since we always pass KATIB_TRIAL_NAME to the Trial's Pods, users will always be able to read this environment in their training code, isn't ?
However, this can be useful for other part of the Pod spec: What if user wants to generate different volume which contains Trial name.

@Electronic-Waste Please can you an create an issue to track this discussion ? I think, we should discuss the future of Trial metadata parameters.

[Electronic-Waste]

@andreyvelich I've wrapped the code into mutatePodEnv and added some unit tests for it. PTAL👀

[andreyvelich]

Thank you @Electronic-Waste!
just a small comment from me.

[andreyvelich]

I think, we should be ready to merge it.
Thanks to your contribution @tenzen-y 🎉
/lgtm
/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [andreyvelich]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment