yalindogusahin commented on code in PR #794:
URL:
https://github.com/apache/spark-kubernetes-operator/pull/794#discussion_r4015275206
##########
spark-submission-worker/src/main/java/org/apache/spark/k8s/operator/SparkClusterResourceSpec.java:
##########
@@ -467,35 +469,67 @@ private static Optional<PodDisruptionBudget>
buildPodDisruptionBudget(
* resource and does not carry the cluster label, yet the driver must reach
the executors' block
* manager to fetch task results larger than {@code
spark.task.maxDirectResultSize}.
*
+ * <p>If both {@link WorkerNetworkPolicySpec#getMetricsPort()} and {@link
+ * WorkerNetworkPolicySpec#getMetricsIngress()} are set, a separate rule
admits the listed peers
+ * on that port only. Both are required: the port alone would open the
endpoint to every source in
+ * the cluster, and the peers alone would grant them every port. The worker
web UI port is
+ * deliberately not usable as the metrics port: it is served by the same
embedded HTTP server as
+ * the UI itself, so admitting it would also expose the full UI, not just a
metrics endpoint. A
+ * dedicated metrics port (e.g. a JMX-to-Prometheus exporter agent) keeps
metrics scraping
+ * separate from the UI.
+ *
* @param clusterName The name of the SparkCluster.
* @param namespace The namespace of the SparkApplication.
+ * @param workerSpec The WorkerSpec, used to look up the optional metrics
port and its peers.
* @return A NetworkPolicy object.
+ * @throws IllegalArgumentException if a metrics ingress rule is requested
on the worker web UI
+ * port.
*/
- private NetworkPolicy buildWorkerNetworkPolicy(String clusterName, String
namespace) {
- return new NetworkPolicyBuilder()
- .withNewMetadata()
- .withName(clusterName + "-worker")
- .withNamespace(namespace)
- .addToLabels(LABEL_SPARK_CLUSTER_NAME, clusterName)
- .endMetadata()
- .withNewSpec()
- .withNewPodSelector()
- .addToMatchLabels(LABEL_SPARK_ROLE_NAME, LABEL_SPARK_ROLE_WORKER_VALUE)
- .addToMatchLabels(LABEL_SPARK_CLUSTER_NAME, clusterName)
- .endPodSelector()
- .addNewIngress()
- .addNewFrom()
- .withNewPodSelector()
- .addToMatchLabels(LABEL_SPARK_CLUSTER_NAME, clusterName)
- .endPodSelector()
- .endFrom()
- .addNewFrom()
- .withNewPodSelector()
- .addToMatchLabels(LABEL_SPARK_ROLE_NAME, LABEL_SPARK_ROLE_DRIVER_VALUE)
- .endPodSelector()
- .endFrom()
- .endIngress()
- .endSpec()
- .build();
+ private NetworkPolicy buildWorkerNetworkPolicy(
+ String clusterName, String namespace, WorkerSpec workerSpec) {
+ var builder =
+ new NetworkPolicyBuilder()
+ .withNewMetadata()
+ .withName(clusterName + "-worker")
+ .withNamespace(namespace)
+ .addToLabels(LABEL_SPARK_CLUSTER_NAME, clusterName)
+ .endMetadata()
+ .withNewSpec()
+ .withNewPodSelector()
+ .addToMatchLabels(LABEL_SPARK_ROLE_NAME,
LABEL_SPARK_ROLE_WORKER_VALUE)
+ .addToMatchLabels(LABEL_SPARK_CLUSTER_NAME, clusterName)
+ .endPodSelector()
+ .addNewIngress()
+ .addNewFrom()
+ .withNewPodSelector()
+ .addToMatchLabels(LABEL_SPARK_CLUSTER_NAME, clusterName)
+ .endPodSelector()
+ .endFrom()
+ .addNewFrom()
+ .withNewPodSelector()
+ .addToMatchLabels(LABEL_SPARK_ROLE_NAME,
LABEL_SPARK_ROLE_DRIVER_VALUE)
+ .endPodSelector()
+ .endFrom()
+ .endIngress();
+ WorkerNetworkPolicySpec networkPolicy = workerSpec.getNetworkPolicy();
+ if (networkPolicy != null
+ && networkPolicy.getMetricsPort() != null
+ && networkPolicy.getMetricsIngress() != null
+ && !networkPolicy.getMetricsIngress().isEmpty()) {
+ if (networkPolicy.getMetricsPort() == 8081) {
Review Comment:
Removed it and updated the docs/Javadoc to recommend a dedicated exporter
port and clarify that the operator doesn't verify this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]