Skip to content

Spark on k8s : changes for using supporting multi tenant(multi namespace) k8s cluster - #462

Open
ashokkumarrathore wants to merge 1 commit into
apache:masterfrom
ashokkumarrathore:livy_nmsp_fix
Open

Spark on k8s : changes for using supporting multi tenant(multi namespace) k8s cluster#462
ashokkumarrathore wants to merge 1 commit into
apache:masterfrom
ashokkumarrathore:livy_nmsp_fix

Conversation

@ashokkumarrathore

@ashokkumarrathore ashokkumarrathore commented Jan 1, 2025 •
edited
Loading

Copy link
Copy Markdown

What changes were proposed in this pull request?

Related issue : #461

How was this patch tested?

Tested by deploying updated server in a k8s cluster with namespaces and running a job.

@ashokkumarrathore ashokkumarrathore mentioned this pull request Jan 1, 2025
Open
@ashokkumarrathore ashokkumarrathore changed the title Livy nmsp fix Spark on k8s : changes for using namspaces with Kubernetes client APIs. Jan 1, 2025
@ashokkumarrathore ashokkumarrathore mentioned this pull request Jan 8, 2025
Open
jahstreet
jahstreet reviewed Jan 23, 2025
View reviewed changes
Comment thread server/src/main/scala/org/apache/livy/utils/SparkKubernetesApp.scala Outdated
@github-actions

github-actions Bot commented Feb 21, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has had no activity for at least 3 months. If you are still working on this change or plan to move it forward, please leave a comment or push a new commit so we know to keep it open. Otherwise, this PR will be closed automatically in about one month. Thank you for your contribution to Apache Livy!

@github-actions github-actions Bot added the stale label Feb 21, 2026
@ashokkumarrathore ashokkumarrathore changed the title Spark on k8s : changes for using namspaces with Kubernetes client APIs. Spark on k8s : changes for using namespaces with Kubernetes client APIs. Jun 19, 2026
@ashokkumarrathore ashokkumarrathore changed the title Spark on k8s : changes for using namespaces with Kubernetes client APIs. Spark on k8s : changes for using supporting multi tenant(multi namespace) k8s cluster Jun 19, 2026
@ashokkumarrathore

ashokkumarrathore commented Jun 19, 2026

Copy link
Copy Markdown
Author

Rebased onto the latest master and resolved all conflicts — the PR is now mergeable. Summary of what changed since the last review:

Rebase / conflict resolution

  • Rebased onto current master (the branch was ~42 commits behind).
  • Dropped changes that master has since made independently: the commons.lang3 import and the SQLInterpreterSpec assertion (kept master's broader version).
  • Removed redundant per-module pom overrides (maven-shade-plugin, commons-lang3, netty-all, hadoop-common) now that the parent pom manages those versions; one of them would otherwise have downgraded maven-shade-plugin 3.5.0 → 3.4.1.

Version bumps (security) — kept the higher version across master/PR: kubernetes-client 6.8.1, netty 4.1.108.Final. Verified the code compiles and unit tests pass against the fabric8 6.x client.

Bug fix in SparkApp.getNamespace — the previous version unconditionally read $SPARK_HOME/conf/spark-defaults.conf and called .get on sparkHome(), which threw FileNotFoundException/NoSuchElementException and broke batch-session creation (and several existing tests) for non-Kubernetes deployments. It now:

  • resolves a namespace only on Kubernetes (empty string otherwise, no filesystem access),
  • treats a missing or key-less spark-defaults.conf as "no value" and falls back to the default namespace,
  • closes the input stream.

This also addresses @jahstreet's earlier question about whether the namespace can be empty initially.

Tests — added SparkAppSpec covering the full namespace-resolution precedence chain (session conf → spark-defaults.conf → default), plus the non-Kubernetes and missing-SPARK_HOME paths. All touched suites pass locally (SparkAppSpec, BatchSessionSpec, SessionManagerSpec, SparkKubernetesAppSpec); scalastyle is clean.

Commits were squashed into one. @gyogal — could you kick off the check jobs when you get a chance? Thanks!

Note on the watched-namespace truncation idea (@jahstreet): I've left that out of this PR to keep it focused; happy to follow up in a separate change since picking a safe truncation point needs more thought.

@ashokkumarrathore
ashokkumarrathore requested a review from gyogal June 19, 2026 11:02
@ashokkumarrathore

ashokkumarrathore commented Jun 19, 2026

Copy link
Copy Markdown
Author

Pushed an update that folds in five additional Kubernetes/session fixes that were developed on top of this branch (authored by @vallenki). All are rebased onto current master, compile cleanly against kubernetes-client 6.8.1, pass scalastyle/checkstyle, and have unit tests:

  1. Livy Interactive session fixes — adds livy.rsc.driver.address.use-hostname (RSCConf) so the driver's reported hostname is used instead of the socket IP, which is needed for Kubernetes + Istio setups.
  2. Reduce Kubernetes API load in polling — narrows the driver lookup to spark-role=driver at the API server (single-pod LIST instead of all executor pods) and gates the executor LIST behind a new flag livy.server.kubernetes.executor-tracking.enabled (default true, so no behavior change). Large clusters can disable it to skip per-executor LISTs.
  3. SessionManager refresh + /recovery operator endpoint — a superuser-guarded (livy.superusers, returns 403 otherwise) REST endpoint to re-scan the recovery state store and import sessions written by another Livy server, without a restart.
  4. Fix recovery-file leak after session deletion — kill() now removes the app from the shared monitor queue and the monitor early-returns after a terminal state, so a deleted session's recovery file is no longer resurrected by an in-flight monitor tick. killed becomes AtomicBoolean for cross-thread visibility.
  5. Fix Promise.failure race that falsely failed healthy apps — switches the catch path to Promise.tryFailure, matching the trySuccess on the success path, so a late failure on an already-completed promise is dropped instead of flipping a healthy app to FAILED. This became reachable once app-lookup.thread-pool.size > 1.

Combined diff is ~545 insertions across 16 files. Happy to split any of these into separate PRs if you'd prefer to keep this one scoped purely to multi-namespace support — just let me know.

@gyogal gyogal removed the stale label Jun 20, 2026
@ashokkumarrathore

ashokkumarrathore commented Jun 22, 2026

Copy link
Copy Markdown
Author

@gyogal @jahstreet looks like tests are passing now. Just fyi, we are running Livy server in production with these changes to run spark on k8s jobs.

@soumyadeeplogin soumyadeeplogin mentioned this pull request Aug 20, 2026
Merged
@gyogal

gyogal commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Hi @ashokkumarrathore , I noticed that there are multiple related PRs opened for Kubernetes enhancements (for example #544, #545 and #546 ) and some of these address issues that your PR also addresses. Could you please take a look to see how those are related to your change and if it would be possible to similarly break up your changes to individual smaller PRs that each focus on a single new feature? Also would it be possible to open a JIRA ticket for each of the remaining changes?

@gyogal gyogal mentioned this pull request Sep 9, 2026
Open
@ashokkumarrathore

ashokkumarrathore commented Sep 9, 2026

Copy link
Copy Markdown
Author

Thanks @gyogal — you're right, this PR had grown to bundle several independent changes, and there's real overlap with the newer Kubernetes PRs. Here's how they relate and how I've split things up:

Already merged upstream — dropped from this PR:

I've rebased this branch onto current master, which already carries both.

Overlaps an open PR — dropped here:

This PR now focuses on exactly one feature:

  • LIVY-461 — Multi-tenant (multi-namespace) Kubernetes support (namespace resolution + inNamespace() request threading). I've force-pushed the branch down to just this change.

Two remaining independent bug fixes — a focused PR + JIRA each:

  1. Fix recovery-file leak after session deletion.
  2. Fix Promise.failure race that falsely fails healthy Spark apps.

I'll file JIRAs for those two and open a small PR for each, then link them back here.

@ashokkumarrathore
ashokkumarrathore force-pushed the livy_nmsp_fix branch 2 times, most recently from 11893e1 to 06b0e77 Compare September 9, 2026 08:28
Ashok Kumar @claude
[LIVY-461] Support multi-tenant (multi-namespace) Kubernetes clusters
e1e60d2 
Livy on Kubernetes previously assumed a single namespace and listed/killed
applications across all namespaces (inAnyNamespace). In a multi-tenant
cluster the Livy service account is typically scoped to a subset of
namespaces, so cluster-wide calls fail.

This change makes the namespace a first-class, per-session property:

* SparkApp.getNamespace resolves the target namespace from the session
  Spark conf (spark.kubernetes.namespace), falling back to
  $SPARK_HOME/conf/spark-defaults.conf and finally the 'default'
  namespace. It only touches the filesystem on Kubernetes and returns an
  empty namespace for YARN/local, closing the input stream and tolerating
  a missing file, an unset SPARK_HOME, or a malformed spark-defaults.conf
  (an invalid property escape no longer aborts session creation).

* The namespace is threaded through SparkApp.create and persisted in the
  batch and interactive recovery metadata, so a recovered session keeps
  the namespace it was created with. Recovery metadata written before this
  change has no namespace field and deserializes to null; such sessions are
  treated as "unknown" and fall back to scanning all namespaces until the
  driver pod is found, whereupon its real namespace is adopted. This keeps
  in-flight sessions recoverable across the upgrade.

* SparkKubernetesApp scopes every client call with .inNamespace(...) and
  tracks the set of namespaces it has seen in a thread-safe set
  (ConcurrentHashMap-backed) so leaked-application cleanup iterates only
  over namespaces Livy actually has access to. Leaked-app GC isolates
  per-namespace failures and guards the whole cycle, so losing access to a
  single namespace can neither stop the sweep of the others nor terminate
  the GC thread.

* Bumps kubernetes-client to 6.8.1 and netty to 4.1.108.Final for
  security fixes.

* Adds SparkAppSpec covering the namespace-resolution precedence chain
  (including malformed-conf fallback), a BatchSession recovery test
  asserting the namespace round-trips through recovery metadata, and
  updates existing recovery-metadata fixtures for the new field.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gyogal gyogal Awaiting requested review from gyogal
@duc-dn duc-dn Awaiting requested review from duc-dn
@jahstreet jahstreet Awaiting requested review from jahstreet
1 more reviewer
@Rchie121 Rchie121 Rchie121 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ashokkumarrathore @gyogal @jahstreet @duc-dn @Rchie121