Skip to content

Conversation

@jiangzho
Copy link
Contributor

@jiangzho jiangzho commented Aug 6, 2025

What changes were proposed in this pull request?

Spark Operator provides prometheus format metrics by default. This PR adds metrics port spec to the operator helm chart.

Why are the changes needed?

Declaring the port in spec for better clarity and maintainability.

Does this PR introduce any user-facing change?

No

How was this patch tested?

CIs, helm lint, and local testing

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

No

### What changes were proposed in this pull request?

Spark Operator provides prometheus format metrics by default. This PR adds metrics port spec to the operator helm chart.

### Why are the changes needed?

Declaring the port in spec for better clarity and maintainability.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

CIs, helm lint, and local testing

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

No
@github-actions github-actions bot added the BUILD label Aug 6, 2025
@jiangzho
Copy link
Contributor Author

jiangzho commented Aug 6, 2025

cc @peter-toth can you please help to review ?

Copy link
Contributor

@peter-toth peter-toth left a comment

Choose a reason for hiding this comment

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

LGTM.

spark.kubernetes.operator.namespace={{ .Release.Namespace }}
spark.kubernetes.operator.name={{- include "spark-operator.name" . }}
spark.kubernetes.operator.dynamicConfig.enabled={{ .Values.operatorConfiguration.dynamicConfig.enable }}
spark.kubernetes.operator.metrics.port={{ include "spark-operator.metricsPort" . }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to this PR, but @jiangzho can you please check why spark.kubernetes.operator.health.probePort is not set here from spark-operator.probePort?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the catch!

Yes, we'll need to fix that as well when user override the port from Helm chart values. I'll track that over SPARK-53186

@jiangzho
Copy link
Contributor Author

jiangzho commented Aug 7, 2025

@peter-toth would you mind rerun the failed test suite ? I checked the failure and not able to reproduce, seems a blip

@peter-toth peter-toth closed this in c95a8c0 Aug 8, 2025
@peter-toth
Copy link
Contributor

Tests passed. Thank you @jiangzho for the fix.

Merged to main (0.5.0).

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants