fix(spark): clean up session and port-forward on failure by shreya-024 · Pull Request #779 · kubeflow/sdk

shreya-024 · 2026-09-01T20:21:01Z

What this PR does / why we need it:

Fixes the SparkConnect resource handling gap identified in #476.

Map unrecognised CR states to SparkConnectState.UNKNOWN instead of PROVISIONING in get_spark_connect_info_from_cr
Delete the SparkConnect CR on failure in create_and_connect to prevent orphaned cluster resources
Terminate kubectl port-forward subprocess on connection/timeout failure
Added "unknown status" parametrized test case inside test_get_spark_connect_info_from_cr
Added test_create_and_connect_cleanup_on_failure in backend_test.py

Which issue(s) this PR fixes:

Fixes SparkConnect CR cleanup behavior on failure and SDK state mismatch #476

google-oss-prow · 2026-09-01T20:21:09Z

[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 electronic-waste 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

github-actions · 2026-09-01T20:21:42Z

🎉 Welcome to the Kubeflow SDK! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards
Our team will review your PR soon! cc @kubeflow/kubeflow-sdk-team

Join the community:

Slack: Join our #kubeflow-ml-experience and #kubeflow-trainer Slack channels
Meetings: Attend the Kubeflow SDK and ML Experience bi-weekly meetings

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

tariq-hasan

Hi @shreya-024! I have added a few comments.

Please also follow these instructions to sign your commits: https://github.com/kubeflow/sdk/pull/779/checks?check_run_id=100012874640.

tariq-hasan · 2026-09-02T01:45:02Z

    RUNNING = "Running"  # Operator may set this when server is up; treated as ready
    NOT_READY = "NotReady"
    FAILED = "Failed"
+    UNKNOWN = "Unknown"


This should be removed.

Please remove UNKNOWN = "Unknown".

tariq-hasan · 2026-09-02T01:46:03Z

            state = SparkConnectState(spark_connect_cr.status.state)
        except ValueError:
-            state = SparkConnectState.PROVISIONING
+            state = SparkConnectState.UNKNOWN


Please use this instead.

sdk/kubeflow/common/constants.py

Lines 21 to 22 in 3febcd7

# Unknown indicates that the value can't be identified.

UNKNOWN = "Unknown"

tariq-hasan · 2026-09-02T01:57:52Z

The RUNNING state should be removed.

Suggested change

if info.state == SparkConnectState.READY:

logger.info(

tariq-hasan · 2026-09-02T01:59:12Z

@@ -32,6 +32,7 @@ class SparkConnectState(str, Enum):
    RUNNING = "Running"  # Operator may set this when server is up; treated as ready


The RUNNING state should be removed.

tariq-hasan · 2026-09-02T02:27:33Z

/ok-to-test
/retest

tariq-hasan · 2026-09-02T03:31:50Z

/assign

Signed-off-by: shreya-024 <shreyak.2406@gmail.com>

vjkumar2756

LGTM
@shreya-024
I went through the changes, and the cleanup handling looks solid. Mapping unrecognized states to UNKNOWN, cleaning up the SparkConnect CR on failure, and terminating the port forward subprocess should prevent orphaned resources and state mismatches.

tariq-hasan

Hi @shreya-024! I have added a few comments.

tariq-hasan · 2026-09-06T18:40:25Z

    RUNNING = "Running"  # Operator may set this when server is up; treated as ready
    NOT_READY = "NotReady"
    FAILED = "Failed"
+    UNKNOWN = "Unknown"


Please remove UNKNOWN = "Unknown".

tariq-hasan · 2026-09-06T18:59:35Z

 from enum import Enum
 import logging
 from typing import Any



Let's add import kubeflow.common.constants as common_constants.

tariq-hasan · 2026-09-06T19:01:02Z

Let's rewrite as follows.

Suggested change

state: str = common_constants.UNKNOWN

tariq-hasan · 2026-09-06T19:01:33Z

Same here

Suggested change

status: str = common_constants.UNKNOWN

tariq-hasan · 2026-09-06T19:02:08Z

            state = SparkConnectState(spark_connect_cr.status.state)
        except ValueError:
-            state = SparkConnectState.PROVISIONING
+            state = SparkConnectState(common_constants.UNKNOWN)


Suggested change

state = SparkConnectState(common_constants.UNKNOWN)

state = common_constants.UNKNOWN

tariq-hasan · 2026-09-06T19:59:45Z

                    f"{self.namespace}/{name} (timeout: {timeout}s)"
                )

            time.sleep(polling_interval)


We need to be able to retry for transient errors.

Signed-off-by: shreya-024 <shreyak.2406@gmail.com>

google-oss-prow Bot requested review from Fiona-Waters and tariq-hasan September 1, 2026 20:21

google-oss-prow Bot added the size/L label Sep 1, 2026

shreya-024 mentioned this pull request Sep 1, 2026

SparkConnect CR cleanup behavior on failure and SDK state mismatch #476

Open

tariq-hasan reviewed Sep 2, 2026

View reviewed changes

google-oss-prow Bot added the ok-to-test label Sep 2, 2026

google-oss-prow Bot assigned tariq-hasan Sep 2, 2026

shreya-024 added 2 commits September 2, 2026 11:09

fix(spark): clean up session and port-forward on failure

a62368d

Signed-off-by: shreya-024 <shreyak.2406@gmail.com>

fix(spark): remove RUNNING state and use constant.UNKNOWN fallback

92e552c

Signed-off-by: shreya-024 <shreyak.2406@gmail.com>

shreya-024 force-pushed the fix/spark-connect-state-unknown-cleanup branch from 8d74bdc to 92e552c Compare September 2, 2026 05:40

vjkumar2756 reviewed Sep 2, 2026

View reviewed changes

shreya-024 requested a review from tariq-hasan September 4, 2026 20:05

tariq-hasan suggested changes Sep 6, 2026

View reviewed changes

fix(spark): address review comments on state and constants

fdb5397

Signed-off-by: shreya-024 <shreyak.2406@gmail.com>

	# Unknown indicates that the value can't be identified.
	UNKNOWN = "Unknown"

		@@ -32,6 +32,7 @@ class SparkConnectState(str, Enum):
		RUNNING = "Running" # Operator may set this when server is up; treated as ready

	state = SparkConnectState(common_constants.UNKNOWN)
	state = common_constants.UNKNOWN

Uh oh!

Conversation

shreya-024 commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-oss-prow Bot commented Sep 1, 2026

Uh oh!

github-actions Bot commented Sep 1, 2026

Uh oh!

tariq-hasan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tariq-hasan commented Sep 2, 2026

Uh oh!

tariq-hasan commented Sep 2, 2026

Uh oh!

vjkumar2756 left a comment

Choose a reason for hiding this comment

Uh oh!

tariq-hasan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shreya-024 commented Sep 1, 2026 •

edited

Loading

tariq-hasan left a comment •

edited

Loading