Skip to content

feat(kubernetes): select clusters by discovery_args.cluster_ids - #13991

Open
AlinsRan wants to merge 4 commits into
apache:masterfrom
AlinsRan:feat/k8s-discovery-cluster-ids
Open

AlinsRan wants to merge 4 commits into
apache:masterfrom
AlinsRan:feat/k8s-discovery-cluster-ids

Conversation

@AlinsRan

@AlinsRan AlinsRan commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Description

In multi-cluster mode, Kubernetes service discovery can only resolve a service_name against one cluster (id/namespace/name:port_name). This PR lets an upstream pick several clusters explicitly and get the union of their endpoints:

{
    "discovery_type": "kubernetes",
    "service_name": "default/plat-dev:port",
    "discovery_args": {
        "cluster_ids": ["release", "staging"]
    }
}

Behaviour:

  • The nodes are the union of the matching endpoints in the listed clusters (an id from the discovery.kubernetes array), cached per cluster endpoint version, so endpoint changes in those clusters keep refreshing the upstream. The same host:port found in more than one cluster is used once.
  • Clusters that are not listed, including clusters added to the configuration later, never contribute nodes.
  • If none of the listed clusters has matching endpoints, the upstream goes through the existing "no valid upstream node" path. There is no fallback to other clusters.
  • The upstream check gains an optional discovery hook: when the discovery module named by discovery_type is loaded and exports check_discovery_args(discovery_args, service_name, in_dp), check_upstream_conf calls it and returns its error. Kubernetes discovery implements it:
    • On every path, with cluster_ids, service_name must be namespace/name:port_name; a cluster_id/namespace/name:port_name value is rejected and the error shows it.
    • In the Admin API (not in_dp), cluster_ids is rejected when kubernetes discovery is configured as a single cluster, and ids that are not in discovery.kubernetes of that instance are rejected with one error listing all of them.
  • An unknown cluster id that still reaches the data plane (configuration written before a cluster was removed, rolling restarts, standalone yaml) is skipped at runtime, with one warning per resolution listing all skipped ids.
  • If a service_name with a cluster id prefix reaches the discovery module anyway (for example in a stream route, which the Admin API does not pass through check_upstream_conf), it logs an error and returns no nodes.
  • In single-cluster mode at runtime, cluster_ids has no cluster to match, so the upstream gets no nodes and an error is logged.
  • Without cluster_ids, the existing behaviour is unchanged (namespace/name:port_name in single-cluster mode, id/namespace/name:port_name in multi-cluster mode).

cluster_ids is added to discovery_args in the upstream schema next to the existing namespace_id / group_name, as a non-empty array of unique strings.

Tests: t/kubernetes/discovery/kubernetes5.t covers selection against the kind cluster (listed vs unlisted clusters, unknown ids, endpoint updates), the union and dedup with fixed endpoint data, proxied requests through HTTP routes and a stream route with cluster_ids (the stream case also checks the union from the -stream dicts), single-cluster mode, and the data plane config-load path. t/discovery/kubernetes_cluster_ids.t covers the Admin API validation (unknown ids, prefix, single-cluster configuration, schema errors, inline upstreams in routes and services).

Which issue(s) this PR fixes:

None.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The implementation matches the documented behavior and includes broad test coverage for relevant success and failure paths.

Review effort: Balanced
Findings: None

What changed in this PR

Adds explicit multi-cluster selection for Kubernetes-discovered upstreams while preserving existing discovery behavior.

Changes:

  • Adds discovery_args.cluster_ids schema validation and service-name checks.
  • Merges and deduplicates endpoints from selected clusters with version-aware caching.
  • Adds bilingual documentation and comprehensive integration tests.
File Description
apisix/​discovery/​kubernetes/​init.lua Resolves and merges endpoints from selected clusters.
apisix/​schema_def.lua Defines the cluster_ids schema.
apisix/​upstream.lua Rejects incompatible prefixed service names.
docs/​en/​latest/​discovery/​kubernetes.md Documents the feature in English.
docs/​zh/​latest/​discovery/​kubernetes.md Documents the feature in Chinese.
t/​kubernetes/​discovery/​kubernetes5.t Tests selection, caching, deduplication, validation, and proxying.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The service-name validation regex accepts malformed delimiter usage that should be rejected by the Admin API.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Comment thread apisix/discovery/kubernetes/init.lua Outdated

@membphis membphis 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.

LGTM

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.

4 participants