refine arguments of ControllerInterface.UpdateJobStatus

[sperlingxx]

Move UpdateJobStatus out of ReconcilePods loop, and remove bool arg(restart) which can be inferred from replicas.

---

This change is 

[terrytangyuan]

Cannot you obtain this information by iterating through the replicas map?

[sperlingxx]

Cannot you obtain this information by iterating through the replicas map?

for rtype, spec := range replicas {
     err = jc.ReconcilePods(metaObject, &jobStatus, pods, rtype, spec, replicasStatus, replicas)

Because UpdateJobStatus is called in jc.ReconcilePods, which is wrapped by replicas map iteraton.

[terrytangyuan]

Sure but I mean in your actual implementation of UpdateJobStatus, you can obtain the types and specs by iterating through the map, right?

[sperlingxx]

Yes, but I think, we need to know specific ReplicaType in UpdateJobStatus , because this method will be called for each ReplicaType .

[terrytangyuan]

Oh I see. That makes sense. cc'ing others here to double check as well.
/lgtm
/cc @gaocegege @johnugeorge @jian-he @richardsliu

[jian-he]

I think the implementation can still be that , loop over the "rtype, replicas" and based on each type, set the job status accordingly.

Also, could this updateJobStatus call be moved out from this reconcilePods method and placed after the for loop, here. This way, it can also be merged with the UpdateJobStatusInApiServer interface.

[sperlingxx]

I think the implementation can still be that , loop over the "rtype, replicas" and based on each type, set the job status accordingly.

Also, could this updateJobStatus call be moved out from this reconcilePods method and placed after the for loop, here. This way, it can also be merged with the UpdateJobStatusInApiServer interface.

I agreed with you.

[gaocegege]

Thanks for your contribution

Generally, LGTM

[gaocegege]

Please fix the test failures

[sperlingxx]

Please fix the test failures

Done

[gaocegege]

/lgtm
/cc @johnugeorge @richardsliu

[johnugeorge]

/lgtm

[terrytangyuan]

Some questions on your new changes.

[terrytangyuan]

Why removing this?

[sperlingxx]

Because restart or not can be inferred from replicas.

[gaocegege]

/lgtm

[terrytangyuan]

Okay sounds good. Thanks.
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: terrytangyuan

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 [terrytangyuan]

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