Keep the launcher suspended until the workers are ready

[poojarishreyas]

Picking up #615, open since January 2024. #617 proposed skipping the launcher creation while the MPIJob is suspended, which leaves the suspend/resume case open, as pointed out in the review there; this PR takes the alternative approach suggested in that same thread. Credit to @wang-mask for the report.

What this PR does / why we need it

With launcherCreationPolicy: WaitForWorkersReady, the launcher must not start before all the worker Pods are ready. The policy is only evaluated when the launcher Job is created, which makes it ineffective as soon as the MPIJob is suspended:

• Worker Pods are not created while the MPIJob is suspended, so worker is empty and c.countReadyWorkerPods(worker) == len(worker) trivially holds (0 == 0). The launcher Job is created — suspended, so nothing runs yet.
• On resume the launcher Job already exists, so the creation check is skipped altogether and the Job is unsuspended in the very sync that creates the worker Pods. The launcher Pod starts while no worker is ready, which is exactly what the policy is meant to prevent. A running MPIJob that is suspended and then resumed hits the same path.

This is the case @alculquicondor pointed out in #617:

With this implementation: what happens if the job is running, then it is suspended and unsuspended? Is a launcher pod created as soon as it is unsuspended the second time? If so, this solution is not sufficient.

and this PR implements the fix suggested right after:

Or you could still create the Job object but not flip the suspend flag until the workers are ready.

The policy check is extracted into launcherCanStart() and applied when unsuspending the launcher Job as well as when creating it. The launcher Job is therefore still created upfront (suspended), which Kueue relies on, but its Pod only starts once every worker is ready. AtStartup behaviour is unchanged.

Which issue(s) this PR fixes

Fixes #615

Checklist

• Unit tests added:
  • TestResumeMPIJobWithWaitForWorkersReady — a suspended MPIJob is resumed; the worker Pods are created in that sync and aren't ready, so the launcher must stay suspended. On master this fails with an unexpected update jobs action carrying Suspend:*false.
  • TestResumeMPIJobWithWaitForWorkersReadyAndReadyWorkers — same, with all workers already running and ready: the launcher is unsuspended as expected.
• go test ./pkg/... and golangci-lint run pass.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tenzen-y for approval. For more information see the Kubernetes Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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