feat: direct clients to external shard endpoints

[scrothers]

Part of the external cluster access effort (umbrella #276). Stacked on #279 (external-clusters/announce-hostnames), and it completes the series.

Summary

This is the piece that actually sends external clients to the right place. Each node announces the external client port that its shard Service allocated, and preferredEndpointType: hostname makes MOVED/ASK and CLUSTER SLOTS hand clients the shard hostname instead of the pod IP. With that in place, a client connecting to any shard gets correct cross-shard redirects to a reachable hostname:port.

Node-to-node traffic still rides on pod IPs, so none of the operator's failover, slot, or topology logic changes.

Features / Behaviour Changes

• externalAccess gains preferredEndpointType (ip default, or hostname).
• With hostname, clients are redirected to the announced shard hostname and the node's external port.

Implementation

• The cluster controller resolves each node's external port from status.externalEndpoints (populated by the shard Service reconcile) and sets it on the ValkeyNode spec. buildContainersDef announces it with --cluster-announce-client-port, or --cluster-announce-client-tls-port when TLS is enabled, since a TLS cluster runs its client listener on the TLS port.
• cluster-preferred-endpoint-type hostname goes into the shared base config when selected, so it rolls through the existing config-hash path. It's cluster-wide rather than per-node.
• A CEL rule requires domain when preferredEndpointType is hostname.

Limitations

preferredEndpointType is cluster-wide, so in-cluster clients are redirected to the hostname too. You can resolve it to the shard Service internally with split-horizon DNS, or accept that the traffic hairpins out and back. To turn external access off, set preferredEndpointType: ip first so clients move off the hostnames, then remove domain or disable. The docs spell out that order.

Testing

• Unit tests cover the client-port flag selection (plain vs TLS) and that cluster-preferred-endpoint-type is only rendered when hostname is selected.
• make test and make lint pass locally.
• A couple of behaviours are best confirmed against a live Valkey 9.0 cluster and are covered by the ExternalAccess-labelled e2e specs: that preferred-endpoint-type=hostname round-trips with the TLS client port, and full cross-shard external routing with valkey-cli -c. See docs/valkeycluster.md for the end-to-end example.

Checklist

• This Pull Request is related to one issue.
• Commit message explains what changed and why
• Tests are added or updated.
• Documentation files are updated.
• I have run pre-commit locally (ran make test and make lint instead)

[Copilot]

Pull request overview

This PR completes the “external cluster access” series by making Valkey redirect external clients to routable shard endpoints (hostname + externally allocated port) while keeping node-to-node traffic on pod IPs, preserving existing reconciliation and failover behavior.

Changes:

• Adds per-shard Services + status plumbing to discover and publish externally reachable node ports, and wires those ports into per-node announce flags.
• Introduces externalAccess.preferredEndpointType (ip default / hostname) and renders cluster-preferred-endpoint-type hostname cluster-wide when selected.
• Updates unit/e2e tests and documentation to cover the external-access workflow and examples.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/e2e/valkeycluster_external_access_test.go	New e2e coverage for shard Services + status ports + announced hostname behavior.
internal/valkey/clusterstate_test.go	Adds regression test for parsing CLUSTER NODES address field with appended hostname.
internal/controller/valkeynode_resources.go	Announces human nodename, shard hostname, and external client(-tls) port when external access is enabled.
internal/controller/valkeynode_resources_test.go	Unit tests for new announce flags, port naming, and TLS vs non-TLS client-port announce selection.
internal/controller/valkeycluster_controller.go	Reconciles per-shard Services, publishes status.externalEndpoints, and plumbs per-node external client port into ValkeyNode specs.
internal/controller/utils.go	Adds helpers for per-node client port names and shard hostname formatting.
internal/controller/shard_services_unit_test.go	Unit tests for ServicePort generation and endpoint extraction logic.
internal/controller/shard_services_test.go	Controller-level tests for shard Service reconciliation lifecycle and NodePort stability.
internal/controller/config.go	Conditionally renders cluster-preferred-endpoint-type hostname into the base config.
internal/controller/config_test.go	Tests default omission and hostname-only rendering of cluster-preferred-endpoint-type.
docs/valkeycluster.md	Documents externalAccess fields, operational notes, and TLS considerations.
config/samples/v1alpha1_valkeycluster-external-access.yaml	Adds a sample ValkeyCluster manifest enabling external access + hostname endpoint preference.
config/samples/kustomization.yaml	Includes the new external-access sample in kustomize samples.
config/crd/bases/valkey.io_valkeynodes.yaml	CRD schema updates for ValkeyNode externalAccess + externalAccessClientPort fields.
config/crd/bases/valkey.io_valkeyclusters.yaml	CRD schema updates for ValkeyCluster externalAccess + status.externalEndpoints + CEL validation.
api/v1alpha1/zz_generated.deepcopy.go	Generated deepcopy updates for new API types/fields.
api/v1alpha1/valkeynode_types.go	Adds spec.externalAccess and spec.externalAccessClientPort to ValkeyNode.
api/v1alpha1/valkeycluster_types.go	Adds externalAccess API, ShardEndpoint status type, and CEL rule for domain requirement.

Files not reviewed (1)

• api/v1alpha1/zz_generated.deepcopy.go: Generated file

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

This PR completes the external-cluster-access series by directing external clients to the correct per-shard endpoints. Each node announces its --cluster-announce-client-port (or --cluster-announce-client-tls-port under TLS) from the NodePort allocated by the per-shard Service, and cluster-preferred-endpoint-type hostname is injected into the shared base config when selected.

• Adds ExternalAccessSpec to ValkeyClusterSpec and ValkeyNodeSpec with a CEL cross-field rule enforcing domain when preferredEndpointType: hostname is chosen.
• Introduces reconcileShardServices which creates one NodePort/LoadBalancer Service per shard (one uniquely-named port per node so the EndpointSlice controller routes each external port to exactly one pod), cleans up on scale-in/disable, and returns allocated endpoints for cluster status.
• externalClientPort reads from cluster.Status.ExternalEndpoints populated in-memory by reconcileShardServices before reconcileValkeyNodes runs, ensuring ValkeyNodes get the correct external port on their first reconcile.

Confidence Score: 4/5

Safe to merge with the annotation-merge nit addressed; all core routing logic is sound and the reconcile ordering guarantees ValkeyNodes receive the correct external port on their first creation.

The reconcile ordering and named-port scheme are correctly implemented end-to-end. Two findings: shard Service annotations are replaced rather than merged on every reconcile (could cause churn with cloud LB controllers), and the container port rename silently produces the wrong name when LabelNodeIndex is missing (a label the cluster controller always sets in practice). Neither blocks correctness on the happy path.

internal/controller/valkeycluster_controller.go (annotation replacement in reconcileShardServices) and internal/controller/valkeynode_resources.go (silent wrong port name on missing label).

Important Files Changed

Filename	Overview
internal/controller/valkeycluster_controller.go	Adds reconcileShardServices and externalClientPort; wires endpoints into cluster status before ValkeyNode reconciliation so the port is available on first create.
internal/controller/valkeynode_resources.go	Adds external-access announce flags and renames the client container port to the node-unique name required by shard Service targetPort matching.
api/v1alpha1/valkeycluster_types.go	Adds ExternalAccessSpec, ShardEndpoint, and PreferredEndpointType with CEL cross-field validation.
internal/controller/config.go	Injects cluster-preferred-endpoint-type hostname into base config; correctly guarded behind Enabled check.
internal/controller/shard_services_test.go	Integration tests cover NodePort creation, scale-in cleanup, and idempotency.
internal/controller/shard_services_unit_test.go	Unit tests verify NodePort preservation and NodePort-vs-LoadBalancer branching.
internal/controller/utils.go	Adds shardClientPortName and shardHostname helpers with correct port-name length comments.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Rec as ClusterController
    participant K8s as Kubernetes API
    participant VNRec as ValkeyNodeController
    participant Pod as Valkey Pod

    Rec->>K8s: CreateOrUpdate shard Service (NodePort)
    K8s-->>Rec: svc with allocated NodePorts
    Rec->>Rec: "cluster.Status.ExternalEndpoints = endpoints"
    Rec->>K8s: CreateOrUpdate ValkeyNode with ExternalAccessClientPort
    Rec->>K8s: updateStatus persists ExternalEndpoints
    VNRec->>K8s: reconcile ValkeyNode update StatefulSet
    K8s->>Pod: restart with cluster-announce-client-port NodePort
    note over Pod,K8s: Named port vk-nN routes each NodePort to exactly one pod

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Rec as ClusterController
    participant K8s as Kubernetes API
    participant VNRec as ValkeyNodeController
    participant Pod as Valkey Pod

    Rec->>K8s: CreateOrUpdate shard Service (NodePort)
    K8s-->>Rec: svc with allocated NodePorts
    Rec->>Rec: "cluster.Status.ExternalEndpoints = endpoints"
    Rec->>K8s: CreateOrUpdate ValkeyNode with ExternalAccessClientPort
    Rec->>K8s: updateStatus persists ExternalEndpoints
    VNRec->>K8s: reconcile ValkeyNode update StatefulSet
    K8s->>Pod: restart with cluster-announce-client-port NodePort
    note over Pod,K8s: Named port vk-nN routes each NodePort to exactly one pod

Loading

_{Reviews (1): Last reviewed commit: "feat: direct clients to external shard e..." | Re-trigger Greptile}

[greptile-apps]

Annotation replacement on every reconcile wipes externally-managed annotations. maps.Clone(ea.ServiceAnnotations) unconditionally replaces the entire annotation map on every CreateOrUpdate pass. For LoadBalancer services, cloud controllers (e.g. GKE NEG, AWS LB controller) add status annotations that will be cleared on each operator reconcile, causing those controllers to churn. Merging into the existing map rather than replacing it would preserve externally-added annotations while still keeping user-specified ones current.

Suggested change

	_, err := controllerutil.CreateOrUpdate(ctx, r.Client, svc, func() error {
	svc.Labels = labels(cluster)
	svc.Labels[LabelCluster] = cluster.Name
	svc.Labels[LabelShardIndex] = shardSelector
	svc.Annotations = maps.Clone(ea.ServiceAnnotations)
	svc.Spec.Type = serviceType
	_, err := controllerutil.CreateOrUpdate(ctx, r.Client, svc, func() error {
	svc.Labels = labels(cluster)
	svc.Labels[LabelCluster] = cluster.Name
	svc.Labels[LabelShardIndex] = shardSelector
	if svc.Annotations == nil {
	svc.Annotations = make(map[string]string, len(ea.ServiceAnnotations))
	}
	for k, v := range ea.ServiceAnnotations {
	svc.Annotations[k] = v
	}
	svc.Spec.Type = serviceType

[greptile-apps]

Silent mismatch when LabelNodeIndex label is absent. node.Labels[LabelNodeIndex] returns "" for a ValkeyNode missing the label, producing the port name "vk-n" instead of (e.g.) "vk-n0". The corresponding shard Service port has targetPort: "vk-n0", so the EndpointSlice controller cannot match the container port and external traffic never reaches the right pod. In practice the cluster controller always stamps this label, but a defensive check would surface the misconfiguration early rather than silently dropping external traffic.

[greptile-apps]

externalClientPort convergence comment. On the initial reconcile cluster.Status.ExternalEndpoints starts empty (fetched from API); reconcileShardServices populates it in-memory just before this call so the NodePorts are available. The comment that "the port is normally available on the same reconcile that creates the node" implicitly relies on controllerutil.CreateOrUpdate populating svc.Spec.Ports[i].NodePort with the API-server-assigned value after Create. The downstream guard if node.Spec.ExternalAccessClientPort > 0 handles the zero case gracefully by skipping the announcement flag, so the cluster converges after the next reconcile — a short comment to that effect would make this two-phase convergence easier to follow during review.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!