feat(spark): check control plane version ConfigMap (#221) by Neha-Nimsatkar · Pull Request #846 · kubeflow/sdk

Neha-Nimsatkar · 2026-10-09T18:12:22Z

What this PR does / why we need it

Implements control-plane version check during Spark backend/client initialization, validating the kubeflow-spark-public ConfigMap in the control-plane namespace.

To support modular namespace overrides across Kubeflow components, this PR introduces a shared get_component_namespace utility in kubeflow/common. The helper resolves namespaces by prioritizing component-specific variables (KUBEFLOW_SPARK_NAMESPACE / KUBEFLOW_TRAINER_NAMESPACE), falling back to the deprecated KUBEFLOW_SYSTEM_NAMESPACE with a UserWarning, and defaulting to kubeflow-system. Both Spark and Trainer backends have been aligned to this shared resolution pattern.

Fixes #221

Changes

`kubeflow/common`

Added get_component_namespace(component_env_var: str, default: str = "kubeflow-system") -> str in kubeflow/common/utils.py with fallback and deprecation warning handling.
Added unit tests in kubeflow/common/utils_test.py covering precedence hierarchy, environment overrides, and warning assertions.

`kubeflow/spark`

Added verify_backend() to KubernetesBackend in kubeflow/spark/backends/kubernetes/backend.py and invoked it during backend __init__.
Checked kubeflow-spark-public ConfigMap for kubeflow_spark_version without blocking initialization or raising unhandled exceptions on read failure.
Updated docs/source/spark/index.rst to document KUBEFLOW_SPARK_NAMESPACE and fallback order.
Added mock helpers and parametrized tests in kubeflow/spark/backends/kubernetes/backend_test.py enforcing strict mock assertions (assert_called_with for name and namespace) and verifying env-var wiring.

`kubeflow/trainer`

Updated verify_backend() in kubeflow/trainer/backends/kubernetes/backend.py to use common_utils.get_component_namespace with KUBEFLOW_TRAINER_NAMESPACE_ENV_VAR.
Added test_verify_backend_respects_trainer_namespace_env_var in kubeflow/trainer/backends/kubernetes/backend_test.py to assert custom trainer namespace resolution and mock arguments.

Verification

Shared common utility tests pass:
Spark backend unit tests pass:
Trainer backend unit tests pass:
Pre-commit hooks and linter checks pass cleanly (pre-commit run --all-files).
DCO sign-off verified on git commit.

google-oss-prow · 2026-10-09T18:12:29Z

[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 astefanutti 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

RobuRishabh · 2026-10-09T18:30:58Z

/ok-to-test

RobuRishabh

Hi @Neha-Nimsatkar, Thanks for the contribution. I have left some questions about the ConfigMap deployment and a test-strengthening suggestion. PTAL

RobuRishabh · 2026-10-11T04:55:27Z

+    warnings, call_count = _run_verify_backend_with_core_api(core_api)
+    combined = "\n".join(warnings)
+
+    assert call_count >= 1


Could this assert the exact API request rather than only checking that some call occurred? The current test would still pass if the implementation used the wrong ConfigMap name, wrong namespace, or performed duplicate reads. Please consider assert_called_once_with(name="kubeflow-spark-public", namespace="kubeflow-system") and add a case covering the KUBEFLOW_SYSTEM_NAMESPACE override.

Neha-Nimsatkar · 2026-10-11T08:00:19Z

Hi @Neha-Nimsatkar, Thanks for the contribution. I have left some questions about the ConfigMap deployment and a test-strengthening suggestion. PTAL

Hi @RobuRishabh, Thanks for the feedback!
Before making the updates, I wanted to discuss the intended scope and parity with Kubeflow Trainer:

I made this PR by following Trainer PR #250 (577cba2). In that PR:

The check only looks if the ConfigMap and key exist. It does not check the version.
It runs at the start (initialization) and does not block the client.
The tests only check that the check was called, not the exact arguments.

Your suggestions regarding adding a request timeout, testing the exact call arguments, and testing the (KUBEFLOW_SYSTEM_NAMESPACE) override make total sense and strengthen the implementation.

I just have two questions:

Should we strictly keep 1:1 parity with Trainer (metadata existence check only)? Or do we want to deviate from Trainer and enforce a version range check here?
Is it okay if I keep the current discovery pattern and just add the timeout and stronger test checks?

Would appreciate your thoughts before I push the changes.....

Neha-Nimsatkar · 2026-10-11T08:06:09Z

Also, the kubeflow-spark-public ConfigMap and its RBAC permissions are being added upstream in kubeflow/spark-operator#3225 (mirroring how Trainer introduced its producer upstream prior to the SDK check).

tariq-hasan · 2026-10-21T21:48:45Z

From today's community call the decision was to use one namespace env var per component (KUBEFLOW_TRAINER_NAMESPACE, KUBEFLOW_SPARK_NAMESPACE, KUBEFLOW_PIPELINES_NAMESPACE), plus a deprecation warning for the shared KUBEFLOW_SYSTEM_NAMESPACE. I think it is best to create a shared helper in kubeflow/common for this capability. @Neha-Nimsatkar

Please also raise a PR in http://github.com/kubeflow/pipelines to add a version check in the kfp.kubeflow_client package that uses KUBEFLOW_PIPELINES_NAMESPACE - since this package is re-exported in Kubeflow SDK.

Let me know if there are any further questions.

Neha-Nimsatkar · 2026-10-22T10:27:21Z

From today's community call the decision was to use one namespace env var per component (KUBEFLOW_TRAINER_NAMESPACE, KUBEFLOW_SPARK_NAMESPACE, KUBEFLOW_PIPELINES_NAMESPACE), plus a deprecation warning for the shared KUBEFLOW_SYSTEM_NAMESPACE. I think it is best to create a shared helper in kubeflow/common for this capability. @Neha-Nimsatkar

Please also raise a PR in http://github.com/kubeflow/pipelines to add a version check in the kfp.kubeflow_client package that uses KUBEFLOW_PIPELINES_NAMESPACE - since this package is re-exported in Kubeflow SDK.

Let me know if there are any further questions.

Thanks for the review! I’ve updated the PR with those suggestions:

Added the component-specific namespace environment variable constants (KUBEFLOW_SPARK_NAMESPACE_ENV_VAR, etc.) to kubeflow/common/constants.py.
Added the get_component_namespace helper in kubeflow/common/utils.py. It checks the component variable first, falls back to KUBEFLOW_SYSTEM_NAMESPACE with a DeprecationWarning, and defaults to kubeflow. Parameterized unit tests using TestCase have been added to kubeflow/common/utils_test.py.
Updated verify_backend in kubeflow/spark/backends/kubernetes/backend.py to use get_component_namespace(common_constants.KUBEFLOW_SPARK_NAMESPACE_ENV_VAR).

RobuRishabh · 2026-10-23T00:34:22Z

+        system_namespace = utils.get_component_namespace(
+            common_constants.KUBEFLOW_SPARK_NAMESPACE_ENV_VAR,
+            default="kubeflow",
+        )


In the community call it was decided the env-var naming, but not the default and kubeflow doesn't match either side of the contract:

The Trainer mirror defaults to kubeflow-system

The operator chart creates kubeflow-spark-public in the release namespace by default, not kubeflow.

So a plain Helm install of the operator with publicConfigMap.create=true puts the ConfigMap in e.g. spark-operator, the SDK looks in kubeflow, and every healthy install logs a spurious warning. That turns the feature into noise out of the box. So now can we align the default with the operator-side contract before this ships?

I think you can pick kubeflow-system as the canonical default.

SDK: default="kubeflow-system" to match Trainer.

Chart: default publicConfigMap.namespace to kubeflow-system when create=true, or at minimum document "set KUBEFLOW_SPARK_NAMESPACE=<release-ns>" right next to the publicConfigMap values in the chart README.
But there will be a tradeoff that's standalone operator installs (no Kubeflow platform) must create kubeflow-system or override both the chart value and the env var. That's the same friction Trainer already accepts.

Makes sense to me as well. If the default is chosen to be kubeflow-system the expectation on the spark operator opt-in would be to deploy the configmap in the kubeflow-system namespace.

@Neha-Nimsatkar Please update the docs in the repo to reflect this assumption.

RobuRishabh · 2026-10-23T00:48:43Z

+        warnings.warn(
+            f"{constants.KUBEFLOW_SYSTEM_NAMESPACE_ENV_VAR} is deprecated. "
+            f"Please use {component_env_var} instead.",
+            DeprecationWarning,
+            stacklevel=2,


Two small things on the deprecation path:

DeprecationWarning is filtered out by default for library consumers, so most users will never see the migration notice. I'd use UserWarning (or FutureWarning) so the decided nudge actually reaches people.

Transition hazard: A user who follows this warning and switches to KUBEFLOW_SPARK_NAMESPACE might breaks their Trainer client. I think we should either sequence Trainer's adoption of this helper in the same release cycle, or soften the message to mention both vars are read during the transition.

and accordingly update the PR description

I think it makes sense to update the PR to include Trainer support as well. So KUBEFLOW_SYSTEM_NAMESPACE_ENV_VAR can be marked as deprecated for Trainer as well (but still supported for this release cycle) and KUBEFLOW_TRAINER_NAMESPACE_ENV_VAR becomes the supported mechanism for Trainer going forward.

RobuRishabh · 2026-10-23T00:56:47Z

    print("test execution complete")


+@pytest.mark.parametrize(


The mocked CoreV1Api returns data for any name/namespace, and we only assert call_count >= 1 so a typo in "kubeflow-spark-public" or a wrong namespace would still pass. Could we assert read_namespaced_config_map.call_args.kwargs for both name and namespace?

Also, the env-var resolution is tested against get_component_namespace directly, but never through verify_backend with ``KUBEFLOW_SPARK_NAMESPACE` set, one backend-level case would prove the wiring.

Neha-Nimsatkar · 2026-10-23T12:38:13Z

Hi @RobuRishabh, I've addressed the feedback and updated the PR description and pushed the latest changes. All unit tests and pre-commit checks are passing cleanly. PTAL when you get a chance!

RobuRishabh

just quick nit Helper's own default is "kubeflow", but the only production caller passes "kubeflow-system" explicitly. So a caller (Trainer/Pipelines migration) that forgets the default kwarg silently gets kubeflow a footgun the tests won't catch. So maybe make default a required parameter, or align it to kubeflow-system.

Neha-Nimsatkar · 2026-10-30T06:44:17Z

just quick nit Helper's own default is "kubeflow", but the only production caller passes "kubeflow-system" explicitly. So a caller (Trainer/Pipelines migration) that forgets the default kwarg silently gets kubeflow a footgun the tests won't catch. So maybe make default a required parameter, or align it to kubeflow-system.

Thanks for catching this @RobuRishabh . I have aligned the default parameter in get_component_namespace and its corresponding unit test to "kubeflow-system".

tariq-hasan · 2026-11-02T22:58:46Z

+# Environment variables for Kubeflow component control plane namespaces.
+KUBEFLOW_SYSTEM_NAMESPACE_ENV_VAR = "KUBEFLOW_SYSTEM_NAMESPACE"
+KUBEFLOW_SPARK_NAMESPACE_ENV_VAR = "KUBEFLOW_SPARK_NAMESPACE"
+KUBEFLOW_TRAINER_NAMESPACE_ENV_VAR = "KUBEFLOW_TRAINER_NAMESPACE"


Please also update Trainer to use this constant.

tariq-hasan · 2026-11-02T22:59:05Z

+KUBEFLOW_SYSTEM_NAMESPACE_ENV_VAR = "KUBEFLOW_SYSTEM_NAMESPACE"
+KUBEFLOW_SPARK_NAMESPACE_ENV_VAR = "KUBEFLOW_SPARK_NAMESPACE"
+KUBEFLOW_TRAINER_NAMESPACE_ENV_VAR = "KUBEFLOW_TRAINER_NAMESPACE"
+KUBEFLOW_PIPELINES_NAMESPACE_ENV_VAR = "KUBEFLOW_PIPELINES_NAMESPACE"


The Pipelines constant should be removed. It belongs in the KFP repo.

tariq-hasan · 2026-11-02T23:20:17Z

+        system_namespace = utils.get_component_namespace(
+            common_constants.KUBEFLOW_SPARK_NAMESPACE_ENV_VAR,
+            default="kubeflow",
+        )


Makes sense to me as well. If the default is chosen to be kubeflow-system the expectation on the spark operator opt-in would be to deploy the configmap in the kubeflow-system namespace.

@Neha-Nimsatkar Please update the docs in the repo to reflect this assumption.

tariq-hasan · 2026-11-02T23:27:00Z

+        warnings.warn(
+            f"{constants.KUBEFLOW_SYSTEM_NAMESPACE_ENV_VAR} is deprecated. "
+            f"Please use {component_env_var} instead.",
+            DeprecationWarning,
+            stacklevel=2,


I think it makes sense to update the PR to include Trainer support as well. So KUBEFLOW_SYSTEM_NAMESPACE_ENV_VAR can be marked as deprecated for Trainer as well (but still supported for this release cycle) and KUBEFLOW_TRAINER_NAMESPACE_ENV_VAR becomes the supported mechanism for Trainer going forward.

…zation Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

…lution in backend tests Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

…EFLOW_SYSTEM_NAMESPACE Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

Neha-Nimsatkar · 2026-11-03T12:00:22Z

Thank you @tariq-hasan , All review feedback has been addressed, and CI is now fully passing.
Could you please take another look when you have a moment?

tariq-hasan

I have a few minor comments otherwise lgtm from me.

tariq-hasan · 2026-11-10T01:11:33Z

+def test_verify_backend_respects_trainer_namespace_env_var(monkeypatch):
+    """Test that verify_backend uses KUBEFLOW_TRAINER_NAMESPACE when set."""
+    monkeypatch.setenv(common_constants.KUBEFLOW_TRAINER_NAMESPACE_ENV_VAR, "custom-trainer-ns")
+    core_api = _build_core_api_mock({"kubeflow_trainer_version": "1.0.0"})
+
+    _, call_count = _run_verify_backend_with_core_api(core_api)
+
+    assert call_count == 1
+    core_api.read_namespaced_config_map.assert_called_with(
+        name="kubeflow-trainer-public",
+        namespace="custom-trainer-ns",
+    )


Please bundle this test as a test case under test_verify_backend.

Thanks, @tariq-hasan . Bundled this as a parameterized test case under test_verify_backend and verified all tests pass.

tariq-hasan · 2026-11-10T01:12:27Z


    # ------------------------------------------------------------------
    # Spark Connect sessions
    # ------------------------------------------------------------------



Please move this header below the verify_backend method.

Moved the section header below verify_backend.

Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

tariq-hasan · 2026-11-10T23:59:33Z

Thanks!
/lgtm

Please rebase once #930 merges to pass all the E2E tests.

RobuRishabh · 2026-11-11T02:30:39Z

I think along with rebase we need to pin Spark Operator to the last working commit ref: 511100cc3f4ee95efdc401d314ee44afc4524dfd

RobuRishabh · 2026-11-11T02:32:50Z

+        return os.environ[component_env_var]
+
+    if os.getenv(constants.KUBEFLOW_SYSTEM_NAMESPACE_ENV_VAR):
+        warnings.warn(


one small nit the deprecation warning can become an exception when warnings are treated as errors. change it to DeprecationWarning so it does not break Spark or Trainer backend initialization.

tariq-hasan · 2026-11-11T06:54:18Z

I think along with rebase we need to pin Spark Operator to the last working commit ref: 511100cc3f4ee95efdc401d314ee44afc4524dfd

I've pinned to the latest commit in #930 so a rebase would do now.

github-project-automation Bot added this to Kubeflow SDK Oct 9, 2026

google-oss-prow Bot added the do-not-merge/work-in-progress label Oct 9, 2026

google-oss-prow Bot requested review from Fiona-Waters and szaher October 9, 2026 18:12

google-oss-prow Bot added the size/L label Oct 9, 2026

google-oss-prow Bot added the ok-to-test label Oct 9, 2026

Neha-Nimsatkar marked this pull request as ready for review October 9, 2026 18:43

google-oss-prow Bot removed the do-not-merge/work-in-progress label Oct 9, 2026

RobuRishabh moved this from Ready to Merge to In Progress in Kubeflow SDK Oct 10, 2026

RobuRishabh suggested changes Oct 11, 2026

View reviewed changes

google-oss-prow Bot assigned RobuRishabh Oct 11, 2026

andreyvelich added this to the v0.6 milestone Oct 21, 2026

Neha-Nimsatkar force-pushed the feat/spark-client-control-plane-check-221 branch 2 times, most recently from dd3bac1 to abd9433 Compare October 22, 2026 10:21

Neha-Nimsatkar force-pushed the feat/spark-client-control-plane-check-221 branch from b1cef65 to abd9433 Compare October 22, 2026 10:45

RobuRishabh suggested changes Oct 23, 2026

View reviewed changes

RobuRishabh approved these changes Oct 29, 2026

View reviewed changes

google-oss-prow Bot added lgtm and removed lgtm labels Oct 29, 2026

tariq-hasan suggested changes Nov 2, 2026

View reviewed changes

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

Neha-Nimsatkar force-pushed the feat/spark-client-control-plane-check-221 branch from fce50aa to a34b82d Compare November 3, 2026 10:00

Neha-Nimsatkar added 5 commits November 3, 2026 15:40

feat(spark): verify control plane availability during client initiali…

2b94b76

…zation Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

test(spark): assert core api mock arguments and verify namespace reso…

dda4e2d

…lution in backend tests Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

Update utils.py

7f4eea4

Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

Update utils_test.py

ff28f3c

Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

refactor(trainer): adopt KUBEFLOW_TRAINER_NAMESPACE and deprecate KUB…

3cd9f8c

…EFLOW_SYSTEM_NAMESPACE Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

Neha-Nimsatkar force-pushed the feat/spark-client-control-plane-check-221 branch from a34b82d to 3cd9f8c Compare November 3, 2026 10:10

tariq-hasan suggested changes Nov 10, 2026

View reviewed changes

refactor: bundle verify_backend test case and relocate session header

77d1d49

Signed-off-by: Neha Nimsatkar <1492neha@gmail.com>

google-oss-prow Bot added the lgtm label Nov 10, 2026

RobuRishabh reviewed Nov 11, 2026

View reviewed changes

Conversation

Neha-Nimsatkar commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it

Changes

kubeflow/common

kubeflow/spark

kubeflow/trainer

Verification

Uh oh!

google-oss-prow Bot commented Oct 9, 2026

Uh oh!

RobuRishabh commented Oct 9, 2026

Uh oh!

RobuRishabh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neha-Nimsatkar commented Oct 11, 2026

Uh oh!

Neha-Nimsatkar commented Oct 11, 2026

Uh oh!

tariq-hasan commented Oct 21, 2026

Uh oh!

Neha-Nimsatkar commented Oct 22, 2026

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!

Neha-Nimsatkar commented Oct 23, 2026

Uh oh!

RobuRishabh 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!

Neha-Nimsatkar commented Oct 30, 2026

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!

Neha-Nimsatkar commented Nov 3, 2026

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!

tariq-hasan commented Nov 10, 2026

Uh oh!

RobuRishabh commented Nov 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tariq-hasan commented Nov 11, 2026

Uh oh!

This branch has not been deployed

Neha-Nimsatkar commented Oct 9, 2026 •

edited

Loading

`kubeflow/common`

`kubeflow/spark`

`kubeflow/trainer`

RobuRishabh left a comment •

edited

Loading