Skip to content

[SPARK-58691] Allow metrics scraping ingress to SparkCluster worker web port - #794

Open
yalindogusahin wants to merge 2 commits into
apache:mainfrom
yalindogusahin:SPARK-58691
Open

[SPARK-58691] Allow metrics scraping ingress to SparkCluster worker web port#794
yalindogusahin wants to merge 2 commits into
apache:mainfrom
yalindogusahin:SPARK-58691

Conversation

@yalindogusahin

Copy link
Copy Markdown

What changes were proposed in this pull request?

This PR adds a second ingress rule to the worker NetworkPolicy generated for every
SparkCluster, admitting any source on the worker's web port (8081) — the port that
serves the worker web UI and, when PrometheusServlet is configured via
spark.metrics.conf, the /metrics/prometheus scrape endpoint. The existing rule
(cluster label / driver label allow-list, unrestricted ports) is untouched, so RPC,
shuffle, and block-manager ports stay locked down exactly as before.

Why are the changes needed?

The worker NetworkPolicy generated in SparkClusterResourceSpec#buildWorkerNetworkPolicy
only admits ingress from pods carrying the cluster label or, since SPARK-58649, the driver
role label. Prometheus scraper pods carry neither, so they're indistinguishable from any
other unrelated pod and are locked out entirely — including from the metrics endpoint,
which is not sensitive and is meant to be broadly scrapable.

Verified against a live cluster running an affected build: with only the pre-fix policy in
place, curl http://<worker-ip>:8081/metrics/prometheus from an unlabeled pod times out.
The worker itself serves real metrics fine locally (PrometheusServlet works as configured
via sparkConf); the NetworkPolicy is what blocks the scrape. Masters are unaffected
since they have no NetworkPolicy applied.

Does this PR introduce any user-facing change?

Yes. The generated worker NetworkPolicy now also admits ingress on the web port from
any source. Since NetworkPolicies are additive this can only widen ingress on that one
port, so it cannot deny traffic that was previously allowed. Clusters running on a CNI that
does not enforce NetworkPolicy are unaffected either way.

How was this patch tested?

Extended SparkClusterResourceSpecTest#testWorkerNetworkPolicy to assert the new ingress
rule (no from restriction, single port named web).

./gradlew :spark-submission-worker:test --tests '*SparkClusterResourceSpecTest*'
./gradlew :spark-submission-worker:spotlessCheck

Both pass.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Sonnet 5

@dongjoon-hyun dongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making a PR, @yalindogusahin .

However, I'm -1 for this AS-IS PR because 8081 is used for the whole worker UI.

@yalindogusahin

yalindogusahin commented Aug 11, 2026

Copy link
Copy Markdown
Author

Thank you for reviewing and that is correct my bad at all I haven't checked the metric names.

However, if we decide to use jmx exporter to expose spark related metrics we should enable this network policy. For the statsd thats not the case I believe.

If we decide to go with jmx exporter we should bake jmx exporter to the image itself and + mapping config.
Looking forward to hear you opinion about this.

@yalindogusahin

Copy link
Copy Markdown
Author

Pushed a redesign that addresses this directly: added an optional workerSpec.metricsPort field. When set, the generated NetworkPolicy opens ingress on that specific port only — not port 8081/the web UI. The idea is to pair this with a JMX-to-Prometheus exporter agent (jmx_prometheus_javaagent) attached to the worker JVM as a -javaagent, which serves metrics on its own dedicated port, fully decoupled from the web UI. Default behavior (no metricsPort set) is unchanged from before this PR — the web UI port is never touched. Added docs (docs/operations.md, new "Exposing SparkCluster Worker Metrics" section) and a runnable example (examples/cluster-with-jmx-exporter.yaml) showing the full setup. Let me know if this addresses your concern or if you'd prefer a different shape for the config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants