[k8s] Refactored k8s operator to use kopf for controller logic

[tgaddair]

Why are these changes needed?

The current implementation of the k8s Ray Operator relies on watching the event stream to handle lifecycle events like cluster creation, update, and deletion. However, the event stream is unreliable because (1) events do not provide any durability or exactly-once processing guarantees, and (2) the Ray Operator does not persist any state internally and so cannot recover gracefully in the event of Ray Operator pod restart.

kopf is a Python framework that abstracts the complexity of writing a Kubernetes Controller to manage event handling for CRDs with fault tolerance and state management provided out of the box. By integrating with kopf, the Ray Operator should be more reliable in the face of restarts and other intermittent errors.

Related issue number

Fixes #15744.

Checks

• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

cc @DmitriGekhtman @richardliaw

[DmitriGekhtman]

If I understand right, a watch on an k8s api object just responds to updates to the object's resourceVersion.
I imagine @kopf.on.update probably does more or less the same thing under the hood? Maybe with some sort of improvement to robustness? Curious to take a look and see...

I don't think the issues observed with the Ray 1.3.0 are with the watch itself -- it's mostly that events are handled plain wrong.
That said, looks like using kopf could definitely make things simpler and less error-prone.

[tgaddair]

If I understand right, a watch on an k8s api object just responds to updates to the object's resourceVersion.
I imagine @kopf.on.update probably does more or less the same thing under the hood? Maybe with some sort of improvement to robustness? Curious to take a look and see...

I don't think the issues observed with the Ray 1.3.0 are with the watch itself -- it's mostly that events are handled plain wrong.
That said, looks like using kopf could definitely make things simpler and less error-prone.

The biggest difference between using the watch and on.update is around robustness, retries, etc. See this section for a more detailed overview of some feature kopf provides in this area: https://kopf.readthedocs.io/en/stable/continuity/

This is not something that watching the event stream alone would provide.

[DmitriGekhtman]

Just heads up on the merge conflicts:
Un-templated versions of the cluster_scoped/namespaced operators have moved to deploy/components.
Helm versions are in deploy/charts/ray/templates (will merge those into one file at some point for easier maintenance).

To add Kopf to the Python dependencies for Ray images, add it to docker/ray-deps/Dockerfile

[richardliaw]

@DmitriGekhtman may I suggest you help resolve the merge conflicts to land this PR in 1.4?

[DmitriGekhtman]

@DmitriGekhtman may I suggest you help resolve the merge conflicts to land this PR in 1.4?

Yeah -- let me take care of both of the logistical things from my last comment.

[DmitriGekhtman]

Took care of above logistics, fixed lint complaints from local pre-push hook.

[tgaddair]

I think everything should be aligned with the review comments at this point, please feel free to take another look.

[DmitriGekhtman]

Looks good! Last thing is to align on the queue detail.

[tgaddair]

Cool! Sounds good to me.

[DmitriGekhtman]

okidoki -- docs update and e2e tests are looking good! Will ping reviewer for review.

[DmitriGekhtman]

Final changes:

• Moved operator to main process
• Improved teardown logic

The sequence of events when the RayCluster API resource is deleted is now

1. Monitor loop exits normally, monitor subprocess is joined.
2. ray down is called for an attempt at orderly cluster shutdown
3. kopf removes the finalizer on the RayCluster resource, resource is deleted

I hope step 2 will mitigate some weird pod-deletion-blocking RPC errors that have been recently spotted.
Consistently stopping the monitor before the head pod should prevent the errors @tgaddair has been seeing lately.

[AmeerHajAli]

@richardliaw , is this a release blocker. If so, can you please provide some information for why we should treat this new feature as a release blocker?

[DmitriGekhtman]

@richardliaw or @edoakes could you merge this into master?

[DmitriGekhtman]

@richardliaw , is this a release blocker. If so, can you please provide some information for why we should treat this new feature as a release blocker?

Note, this a stability fix, not a new feature.
We'll defer to @richardliaw on picking it into 1.4.0.

[richardliaw]

this may not work, but let's see :)

[DmitriGekhtman]

I think it's actually not important here -- just installing in the docker image is enough.

[ddelange]

it seems this PR broke support for py36 ref #26886.

it seems that kopf never supported py36: is there a fallback, or is this reason to open a 'drop py36 support' tracker issue?

[DmitriGekhtman]

The issue is with documentation, which is missing this fact: py36 is not supported in the operator image, but it is perfectly fine to use a py37 operator image with py36 Ray images as long as the Ray versions are the same.

[richardliaw]

Nice work here! Let's not cherry-pick this to 1.4.