Add unit test for ray with an existing cluster and fix nil pointer

[Sovietaced]

Tracking issue

Relayed to flyteorg/flyte#5877

In a follow up pull request I'd like to make the worker_node_config optional.

Why are the changes needed?

When submitting a job to an existing ray cluster you shouldn't need to configure details for an ephemeral cluster you will never use.

What changes were proposed in this pull request?

Adds a unit test and just handles a nil pointer case since head node specs can be None

How was this patch tested?

Unit tests

Setup process

Screenshots

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Related PRs

Docs link

Summary by Bito

This PR implements fixes for a nil pointer issue in the Ray plugin's head node configuration and enhances test coverage for existing cluster scenarios. The changes include adding null checks for head_node_config and restructuring head group spec initialization. The PR also adds comprehensive unit testing for Ray task execution with existing cluster configurations.

Unit tests added: True

Estimated effort to review (1-5, lower is better): 2

[flyte-bot]

Code Review Agent Run Status

• Limitations and other issues: ❌ Failure - The AI Code Review Agent skipped reviewing this change because it is configured to exclude certain pull requests based on the source/target branch or the pull request status. You can change the settings here, or contact the agent instance creator at [email protected].

[flyte-bot]

Code Review Agent Run #66e24f

Actionable Suggestions - 2

• plugins/flytekit-ray/flytekitplugins/ray/task.py - 1

  • Consider extracting head node template logic · Line 107-122
• plugins/flytekit-ray/tests/test_ray.py - 1

  • Consider moving settings to test fixture · Line 46-54

Review Details

• Files reviewed - 2 · Commit Range: 7955aee..7955aee

  • plugins/flytekit-ray/flytekitplugins/ray/task.py
  • plugins/flytekit-ray/tests/test_ray.py
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful

---

AI Code Review powered by

[flyte-bot]

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted
Bug Fix - Fix nil pointer in Ray head node configuration	- task.py - Added null check for head_node_config and improved head group spec initialization
Testing - Add Ray existing cluster test coverage	- test_ray.py - Added unit test for Ray task execution with existing cluster configuration

[flyte-bot]

Consider extracting head node template logic

Consider extracting the head node pod template creation logic into a separate helper method to improve code readability and maintainability. The current implementation has similar logic repeated for both head and worker nodes.

Code suggestion

Check the AI-generated fix before applying

 -        head_group_spec = None
 -        if cfg.head_node_config:
 -            if cfg.head_node_config.requests or cfg.head_node_config.limits:
 -                head_pod_template = PodTemplate(
 -                    pod_spec=pod_spec_from_resources(
 -                        primary_container_name=_RAY_HEAD_CONTAINER_NAME,
 -                        requests=cfg.head_node_config.requests,
 -                        limits=cfg.head_node_config.limits,
 -                    )
 -                )
 -            else:
 -                head_pod_template = cfg.head_node_config.pod_template
 +        def _create_pod_template(node_config, container_name):
 +            if node_config.requests or node_config.limits:
 +                return PodTemplate(
 +                    pod_spec=pod_spec_from_resources(
 +                        primary_container_name=container_name,
 +                        requests=node_config.requests,
 +                        limits=node_config.limits
 +                    )
 +                )
 +            return node_config.pod_template
 +
 +        head_group_spec = None 
 +        if cfg.head_node_config:
 +            head_pod_template = _create_pod_template(cfg.head_node_config, _RAY_HEAD_CONTAINER_NAME)

Code Review Run #66e24f

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them

[flyte-bot]

Consider moving settings to test fixture

Consider moving the SerializationSettings configuration to a test fixture since it appears to be test configuration that could be reused across multiple tests. This would help reduce code duplication and make the tests more maintainable.

Code suggestion

Check the AI-generated fix before applying

Suggested change

	default_img = Image(name="default", fqn="test", tag="tag")
	settings = SerializationSettings(
	project="proj",
	domain="dom",
	version="123",
	image_config=ImageConfig(default_image=default_img, images=[default_img]),
	env={},
	)
	@pytest.fixture(scope="module")
	def serialization_settings():
	default_img = Image(name="default", fqn="test", tag="tag")
	return SerializationSettings(
	project="proj",
	domain="dom",
	version="123",
	image_config=ImageConfig(default_image=default_img, images=[default_img]),
	env={}
	)

Code Review Run #66e24f

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them