Fix main process retrieve logic for early stopping

[shaowei-su]

What this PR does / why we need it:
The GetMainProcesses function expects the directory for completed marker, e.g. /var/log/katib:
https://github.com/kubeflow/katib/blob/master/pkg/metricscollector/v1beta1/common/pns.go#L94
The current code will pass the entire log file i.e /var/log/katib/metrics.log. When there is only single container, this code path will work since if will assign the first non-0 pid as main process. However, when multiple sidecar containers presented this method will break.

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

[coveralls]

Coverage decreased (-0.02%) to 73.522% when pulling b3b4cc9 on shaowei-su:shaowei--fix-main-process-retrieve into 6b55540 on kubeflow:master.

[tenzen-y]

@shaowei-su Thanks for creating this!

I remember that we don't support multiple sidecar containers (multiple processes), now.

katib/cmd/metricscollector/v1beta1/file-metricscollector/main.go

Line 282 in aee2109

// TODO (andreyvelich): Currently support only single child process.

What do you think about this? @andreyvelich

[shaowei-su]

Hi @tenzen-y , in the deployment env I'm working on, the main process will take care of closing other sidecars.
This PR is mainly to address the bug regarding how to locate main process (you can find more info here in the comment section: https://github.com/kubeflow/katib/blob/master/pkg/metricscollector/v1beta1/common/pns.go#L94)

[tenzen-y]

Hi @tenzen-y , in the deployment env I'm working on, the main process will take care of closing other sidecars. This PR is mainly to address the bug regarding how to locate main process (you can find more info here in the comment section: https://github.com/kubeflow/katib/blob/master/pkg/metricscollector/v1beta1/common/pns.go#L94)

@shaowei-su
I see. Thanks for clarifying!
Also, can you rebase this PR since we fixed Charmed Katib / Test?

[tenzen-y]

/lgtm

[tenzen-y]

/lgtm cancel

[shaowei-su]

Thanks @tenzen-y, PR rebased, PTAL!

[tenzen-y]

@shaowei-su Thanks for the updates!
/lgtm

/cc @andreyvelich @johnugeorge

[tenzen-y]

/assign @andreyvelich @johnugeorge

[shaowei-su]

Thanks for the stamp @tenzen-y ! can we merge in this change now?

[tenzen-y]

Thanks for the stamp @tenzen-y ! can we merge in this change now?

This PR will be merged after the approvers has reviewed the changes.

[andreyvelich]

Thank you for notice this @shaowei-su!
/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich, shaowei-su

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