feat(datalayer): extend file-discovery to support cluster endpoints

[liavweiss]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Extends the file-discovery plugin to accept hostname addresses (not just IPv4), enabling multi-cluster hub mode where entire clusters are treated as endpoints.

Changes:

• Relax address validation to accept RFC 1123 hostnames using validation.IsDNS1123Subdomain from k8s.io/apimachinery
• Set PodName = "" for hostname-based entries (cluster endpoints), preserving PodName = e.Name for IPv4 entries (pod endpoints)
• Add optional metricsPort field to EndpointEntry so the metrics scrape port can differ from the traffic port (needed when the spoke EPP metrics endpoint is on a separate port from the spoke gateway)
• IPv6 addresses remain rejected (same as before)
• Fully backward compatible - existing pod YAML files with IPv4 addresses work identically

Hub-mode YAML example:

endpoints:
  - name: cluster-us-east
    address: spoke-us-east.example.com
    port: "443"
    metricsPort: "9090"

Which issue(s) this PR fixes:

Fixes #1903

Note:

The metricsPort field is optional and backward compatible. When omitted, MetricsHost defaults to Address:Port (existing behavior). When set, MetricsHost uses Address:metricsPort, allowing the hub EPP to scrape spoke EPP metrics on a different port than the traffic gateway port. This complements #1858 (mTLS scraper) and #1919 (mTLS metrics server).

[ahg-g]

/assign @ezrasilvera

[ezrasilvera]

Thank you for the PR, overall, it looks good. I added a few comments inside.

[ezrasilvera]

Please go over the README and update it according to your proposed changes. Few examples:
pkg/epp/framework/plugins/datalayer/discovery/file/README.md:45

address: <IPv4>             # required -- must be a valid IPv4 address

This is not true anymore, hostnames are allowed now. Two more lines in the same file are also out of date:

pkg/epp/framework/plugins/datalayer/discovery/file/README.md:93

- `address` must be a literal IPv4 address. Hostnames are not resolved;

pkg/epp/framework/plugins/datalayer/discovery/file/README.md:95

- Metrics are scraped from `address:port` (same host and port ...);
  separate metrics endpoints are not supported.

[ezrasilvera]

nit: (for the future) The address/port checks are inline in the file plugin. When we add more discovery plugins, it may be better to move this into a shared helper so all plugins use the same rules (including the metricsPort check from comment inside).

[liavweiss]

Thank you @ezrasilvera, I made the necessary changes.

[elevran]

Trying to get a high leve read on the goal first.
I believe this PR lays the ground work for treating remote cluster EPPs as endpoints for scheduling.
Not sure the end-to-end flow for this has been considered. The current file plugin populates "Pods" from a file instead of a k8s reconciler. They both feed into the same data store Endpoint abstraction. From the point of ingestion and onwards, file-discovery endpoints and pod-reconciler endpoints land in the exact same ds.pods map/fwkdl.Endpoint abstraction, and nothing downstream (scheduler, scorers, metrics collector) distinguishes "came from file" vs "came from a real Pod" except via the fields on EndpointMetadata itself (PodName, Address, MetricsHost).
My concern is that the use of hostname and metrics ports could potentially break that assumption.

[elevran]

Q: how is the podName fields used? What's the downside of setting it non-empty?

[liavweiss]

PodName is used in three places:

1. datastore.PodDelete() - matches endpoints by PodName to delete them on pod lifecycle events. If we set PodName non-empty for cluster entries, a pod deletion event with a coincidentally matching name would accidentally remove a cluster endpoint from the datastore.
2. metrics.go - used as a Prometheus label on schedulerAttemptsTotal. Blank means the label is empty for cluster entries, which is cosmetic.
3. mmcacheaffinity scorer - used for debug logging only.

Setting it empty is intentional: it prevents PodDelete from accidentally matching cluster endpoints, and it's semantically correct since these entries represent clusters, not pods.

[elevran]

IIUC, PodDelete is called from the reconciler and the EPP enables only one endpoint discovery mechanism at a time - either file based or k8s reconcilation, but not both.

[elevran]

nit: is multicluster the only mode where hostnames are used?

Suggested change

	A cluster endpoints file for multi-cluster hub mode:
	A cluster endpoints file with host names instead of IP addresses:

[elevran]

is this value reflected into the datalayer's metrics collection?
An alternative path might be to define a new data source to scrape remote endpoints by providing a label/tag with the hostname:port.
I think that the current endpoints assume an inference server and you are scraping remote clusters (EPPs, I presume?), so the use of the current metrics data source and extractors seem at odds with your design/goal.

[liavweiss]

Yes, metricsPort feeds into MetricsHost which the metrics scraper uses to connect. You're right that the current core-metrics-extractor assumes vLLM-format metrics - that's why a spoke-epp engine type is being added in PR #1928 to handle the different metric format from spoke EPPs. This PR only adds discovery support; it intentionally does not modify the metrics extraction logic.

[elevran]

Thanks for confirming.
I would prefer we don't overload the engine type for this but instead define a new Extractor for EPP metrics (which are not the same as inference engine metrics). You are essentially extracting different metrics from different objects for slightly different use cases and the overload can be confusing (e.g., introduce logic into existing components to separate server and cluster instead of just creating a different extractor for cluster).
Left comments on #1928 with more detail.

[hexfusion]

I agree with @elevran , les put the extra effort in here to make this feel more native and idiomatic.

[elevran]

Additional notes

1. mmcacheaffinity scorer and metrics counters use PodName purely for log/metric labels, so an empty PodName for cluster entries means their scheduler-attempt metrics and trace logs will show a blank pod label. Cosmetic, not a correctness bug, but worth knowing metrics dashboards keyed on pod will show blanks for hub-mode cluster targets.

2. There are no integration, e2e and coverage tests, only unit tests + one wiring smoke test. Since the existing file discovery changed only the ingest and not the actual endpoint properties, this was less of an issue. I think an integration and e2e tests are needed to confirm the changes in this PR. No need to spin up different clusters, but maybe using file based plugin with hostnames and ports for vllm-simulator would be sufficient to exercise the metrics-scrape path end-to-end with a real HTTP server behind a hostname+metricsPort.

3. A few downstream address usage that assumes IP-like semantics. A few places build raw host:port strings from Address for non-metrics purposes (disagg_profile_handler.go, p2psource/producer.go, preciseprefixcache). These will pass a hostname through fine syntactically (net.JoinHostPort doesn't care), but if any consumer assumes Address is dialable/resolvable in the same network namespace as the EPP (e.g., prefix-cache indexer keying, disaggregated P/D routing), a hostname pointing to a remote cluster's gateway may behave differently than a pod IP. That's inherent to the hub-mode feature intent, not a bug, but it's the kind of thing that would only surface once other plugins (P/D disaggregation, precise-prefix-cache) are exercised against cluster-type endpoints, which the PR doesn't add tests for.

Overall, I would recommend considering which data sources and plugins would be useful for the cross cluster case and validate if they carry no IP address or other assumptions that are violated by the use case's input. Either conform to the assumptions or create cross-cluster variants of the relevant plugins to prevent unexpected failures from broken assumptions,
For example, I suspect the metrics collected are EPP level and not inference-engine level. The current extractor will not work well with that.

cc: @vMaroon @hexfusion

[liavweiss]

Additional notes

1. mmcacheaffinity scorer and metrics counters use PodName purely for log/metric labels, so an empty PodName for cluster entries means their scheduler-attempt metrics and trace logs will show a blank pod label. Cosmetic, not a correctness bug, but worth knowing metrics dashboards keyed on pod will show blanks for hub-mode cluster targets.
2. There are no integration, e2e and coverage tests, only unit tests + one wiring smoke test. Since the existing file discovery changed only the ingest and not the actual endpoint properties, this was less of an issue. I think an integration and e2e tests are needed to confirm the changes in this PR. No need to spin up different clusters, but maybe using file based plugin with hostnames and ports for vllm-simulator would be sufficient to exercise the metrics-scrape path end-to-end with a real HTTP server behind a hostname+metricsPort.
3. A few downstream address usage that assumes IP-like semantics. A few places build raw host:port strings from Address for non-metrics purposes (disagg_profile_handler.go, p2psource/producer.go, preciseprefixcache). These will pass a hostname through fine syntactically (net.JoinHostPort doesn't care), but if any consumer assumes Address is dialable/resolvable in the same network namespace as the EPP (e.g., prefix-cache indexer keying, disaggregated P/D routing), a hostname pointing to a remote cluster's gateway may behave differently than a pod IP. That's inherent to the hub-mode feature intent, not a bug, but it's the kind of thing that would only surface once other plugins (P/D disaggregation, precise-prefix-cache) are exercised against cluster-type endpoints, which the PR doesn't add tests for.

Overall, I would recommend considering which data sources and plugins would be useful for the cross cluster case and validate if they carry no IP address or other assumptions that are violated by the use case's input. Either conform to the assumptions or create cross-cluster variants of the relevant plugins to prevent unexpected failures from broken assumptions, For example, I suspect the metrics collected are EPP level and not inference-engine level. The current extractor will not work well with that.

cc: @vMaroon @hexfusion

Correct - plugins like disagg and precise-prefix-cache are pod-level and wouldn't be configured in hub mode.
We can use for example load-aware scorers and session-affinity and add new hub-specific scorers as needed.

Agreed that integration testing is needed. This PR only changes what gets stored in the datastore at ingest - the actual scraping behavior is unchanged. Full integration testing of the hub-mode scrape flow will be done in a follow-up PR after this one and #1928 are merged, since #1928 is the first PR that actually uses hostname+metricsPort for scraping.

[liavweiss]

Hi @elevran , to address your concern about relying on empty PodName as a signal for cluster endpoints https://github.com/llm-d/llm-d-router/pull/1928#discussion_r3620392199 - I've added an explicit llm-d.ai/endpoint-type label in the file-discovery plugin. The label is auto-detected from the address format (IPv4 → pod, hostname → cluster) and can also be set explicitly in the endpoints file. Yehudit's PR can use this label instead of checking PodName == "".

[elevran]

Hi @elevran , to address your concern about relying on empty PodName as a signal for cluster endpoints https://github.com/llm-d/llm-d-router/pull/1928#discussion_r3620392199 - I've added an explicit llm-d.ai/endpoint-type label in the file-discovery plugin. The label is auto-detected from the address format (IPv4 → pod, hostname → cluster) and can also be set explicitly in the endpoints file. Yehudit's PR can use this label instead of checking PodName == "".

what about an endpint "type" attribute that is set, instead of a label? The file plugin can set it on creation. I would prefer we give cluster endpoints a name (even if not a pod name).
Consider a different PR that replaces PodName (which is k8s centric) with a more generic endpoint name//identity?

[hexfusion]

what about an endpint "type" attribute that is set, instead of a label? The file plugin can set it on creation. I would prefer we give cluster endpoints a name (even if not a pod name).
Consider a different PR that replaces PodName (which is k8s centric) with a more generic endpoint name//identity?

I am picking this up, hope this helps

#2147

[liavweiss]

Ok, I'll wait for #2147 to merge, then rebase and adapt my PR to focus on the hostname acceptance and metricsPort support