[WIP] Update Jobeframework's IsActive() for RayJob

[mszadkow]

What type of PR is this?

/kind bug

What this PR does / why we need it:

The default value for the JobDeploymentStatus is “”, which equals JobDeploymentStatusNew.

Ray-operator is the one to change the JobDeploymentStatus from New to Initialising.
When the RayJob is Suspend it puts the JobDeploymentStatus to Suspending and then to Suspended.

However in MultiKueue setup we lack ray-operator in the manager cluster.
JobDeploymentStatus prevents workloads from being created in the manager cluster and copied to worker clusters as it remains "New" and it's not updated to Suspended.

Which issue(s) this PR fixes:

Relates #3822

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

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

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue ready!

Name	Link
🔨 Latest commit	48f7006
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/677f89e73acb6000082f9dbe
😎 Deploy Preview	https://deploy-preview-3949--kubernetes-sigs-kueue.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[mszadkow]

@andrewsykim @mimowo
I have created a separate PR to discuss it, based on the comment.
Is it the proper approach and explanation?

[mszadkow]

/cc @andrewsykim @mimowo

[mimowo]

Could you clarify what are the user-facing consequences of the bug? Is it reproducible on a running system reliably or a rare race condition?


Kueue’s IsActive()old condition prevents workload creation as the IsActive is used for workload construction, to ensure that old workload is not running.

Can you show a link where this is used specifically so that we can better understand the e2e consequences of the issue?

[mimowo]

Also, would this issue be reproducible and testable with integration tests for the RayJob, as being added in #3892?

[mszadkow]

Also, would this issue be reproducible and testable with integration tests for the RayJob, as being added in #3892?

Ray-operator is the one to change the JobDeploymentStatus from New to Initializing.
However bc the Job is Suspend it puts the JobDeploymentStatus to Suspending and then to Suspended.
It can only happen in MultiKueue scenario where we lack ray-operator...

We need some solution for that problem.

[mimowo]

It can only happen in MultiKueue scenario

I'm still not sure what makes it MK-specific. Can this happen if a regular RayJob is getting suspended, for example, due to preemption or setting the CQ to inactive? Also, in integration tests we could simulate what the Ray-operator would do.

[mszadkow]

It can only happen in MultiKueue scenario

I'm still not sure what makes it MK-specific. Can this happen if a regular RayJob is getting suspended, for example, due to preemption or setting the CQ to inactive? Also, in integration tests we could simulate what the Ray-operator would do.

1. Regular(single cluster scenario) RayJob is not affected by this, bc we have running ray-operator.
   The ray-operator is responsible to put RayJob into non-active state, from Kueue perspective.
   Non-active meaning its .Status.JobDeploymentStatus is equal rayv1.JobDeploymentStatusSuspended and that's perfectly normal if the job is suspended.

2. We do simulate ray-operator in the tests for single cluster by manually updating .Status.JobDeploymentStatus to rayv1.JobDeploymentStatusSuspended.
   As a matter of fact I can do the same for MultiKueue integration tests.
   However there is no agency to change .Status.JobDeploymentStatus from New to Suspended in MK scenario in e2e tests.

[mimowo]

Thank you for the summary @mszadkow - this clarifies a lot. Since this is MK-specific I would prefer to again combine the change with the main PR - as initially done.

However there is no agency to change .Status.JobDeploymentStatus from New to Suspended in MK scenario in e2e tests.

This makes me wonder about the remaining question - why we didn't have an issue with IsActive for other JobCRDs before supporting managedBy in MultiKueue, for example, with kubeflow Jobs?