[Bug][RayJob] Fix FailedToGetJobStatus by allowing transition to Running

[architkulkarni]

The job reconciler pings the job status endpoint every 3 seconds. Previously, if this failed at any time during the job execution, the job deployment status would be set to FailedToGetJobStatus, and it would never be updated back to JobDeploymentStatusRunning because due to an oversight, the existing code only updates the JobDeploymentStatus if the JobStatus changed. Thus in the case where the JobStatus doesn't change (e.g. "RUNNING" -> "RUNNING"), but there is an intermittent failure to get the job status, the JobDeploymentStatus is never updated from JobDeploymentStatusFailedToGetJobStatus to JobDeploymentStatusRunning.

This PR fixes this bug by explicitly updating the status back to JobDeploymentStatusRunning if the reconcile loop gets a JobStatus when the previous status is FailedToGetJobStatus.

Testing: unit test

Why are these changes needed?

Related issue number

Closes #1489

Checks

• I've made sure the tests are passing.
• Testing Strategy
  • Unit tests
  • Manual tests
  • This PR is not tested :(

[architkulkarni]

In the bug reproduction by killing the head pod, the previous and current JobStatus were both RUNNING so we wouldn't go into this branch.

[kevin85421]

There is another question for the issue #1489. Does KubeRay respect TTL when the status is JobDeploymentStatusFailedToGetJobStatus?

[architkulkarni]

There is another question for the issue #1489. Does KubeRay respect TTL when the status is JobDeploymentStatusFailedToGetJobStatus?

@kevin85421 No, the shutdown logic is only entered if the JobStatus is SUCCEEDED or FAILED. It is currently done by comparing a timestamp with JobInfo.end_time which is populated by Ray and retrieved via HTTP.

I will create a followup issue to respect TTL in the case where the JobStatus cannot be retrieved.

[kevin85421]

Could you please open an issue to track the TTL issue? Thanks! Because this PR does not have tests, I will clone your fork to test it manually.

[kevin85421]

Manual test

1. Deploy a job which sleeps 3000s
2. Kill the head pod
3. The job automatically retries, and the status is successfully updated to Running (Previously it would be stuck at FailedToGetJobStatus)

I followed the reproduction script. When I killed the head Pod, the submitter K8s Job reached its backoffLimit within 3 seconds. Therefore, when the head Pod became ready again, the submitter K8s Job would no longer submit the Ray job.

[kevin85421]

Discussed with @architkulkarni offline. The reproduction in the PR description cannot test this PR properly. Hence, Archit will work on tests or reproduction scripts to simulate: (1) head Pod is always healthy (2) A status check request from KubeRay is dropped due to some network issues.

[architkulkarni]

@kevin85421 Adding mocks for an end to end test in CI turned out to be cumbersome. I also tried to do a manual test using NetworkPolicy to block the GetJobInfo requests, but I couldn't successfully block them (even though the NetworkPolicy was successfully able to block requests from a debug sidecar on the ray operator pod...)

For now I have added a unit test, let me know if this is sufficient.

[kevin85421]

Unit tests are not sufficient to prove this PR's correctness, but I am OK with not adding e2e tests and testing this PR manually instead because (1) We have a plan to refactor the RayJob codebase after the v1.0.0 release (2) This PR is a release blocker for KubeRay v1.0.0.

I also tried to do a manual test using NetworkPolicy to block the GetJobInfo requests, but I couldn't successfully block them (even though the NetworkPolicy was successfully able to block requests from a debug sidecar on the ray operator pod...)

Wow. I will try it.

[kevin85421]

@architkulkarni Could you try to use iptables? Thanks!

# Add the following securityContext to the Ray head configuration in your RayJob YAML.
securityContext:
  capabilities:
    add: ['NET_ADMIN']

# Log in to the head Pod and install `curl` and `iptables`.
sudo apt-get update; sudo apt-get install -y curl iptables

# Try to connect to the Ray dashboard
curl -X GET 127.0.0.1:8265
# <!doctype html><html lang="en"><head><meta charset="utf-8"/><link rel="shortcut icon" href="./favicon.ico"/><meta name="viewport" content="width=device-width,initial-scale=1"/><title>Ray Dashboard</title><script defer="defer" src="./static/js/main.1f147255.js"></script><link href="./static/css/main.388a904b.css" rel="stylesheet"></head><body><noscript>You need to enable JavaScript to run this app.</noscript><div id="root"></div></body></html>

# Drop all incoming packets to the dashboard port.
sudo iptables -A INPUT -p tcp --dport 8265 -j DROP

# Try to connect to the Ray dashboard again.
curl -X GET 127.0.0.1:8265
# curl: (28) Failed to connect to 127.0.0.1 port 8265: Connection timed out

[architkulkarni]

@kevin85421 Thanks for the manual test instructions! It works and the fix works correctly.

After disabling the packets and waiting about 1-2 minutes:

Status:
  Dashboard URL:          rayjob-sample-raycluster-plc9l-head-svc.default.svc.cluster.local:8265
  Job Deployment Status:  FailedToGetJobStatus
  Job Id:                 rayjob-sample-hrmmr
  Job Status:             RUNNING
  Message:                Job is currently running.
  Observed Generation:    2

After deleting the blocking rule with sudo iptables -D INPUT -p tcp --dport 8265 -j DROP:

Status:
  Dashboard URL:          rayjob-sample-raycluster-plc9l-head-svc.default.svc.cluster.local:8265
  Job Deployment Status:  Running
  Job Id:                 rayjob-sample-hrmmr
  Job Status:             RUNNING
  Message:                Job is currently running.
  Observed Generation:    2