feat(api): expose Type/LoadBalancerIP/ExternalTrafficPolicy on ServiceOverrides (closes #305)

[aagarwal-apexanalytix]

Summary

Resolves #305 — Expose vtgate service by nodeport or load balancer.

Adds three fields to the existing ServiceOverrides struct so operators can expose vtgate (and any other operator-managed Service that already uses ServiceOverrides — gateway, vtctld, vtadmin, etcd-lockserver, vttablet headless) via NodePort or LoadBalancer directly through the VitessCluster / VitessCell CR, eliminating the layered-Service workaround discussed in #305.

API addition

type ServiceOverrides struct {
    Annotations map[string]string                  `json:"annotations,omitempty"`
    ClusterIP   string                             `json:"clusterIP,omitempty"`
    // NEW:
    Type                  corev1.ServiceType               `json:"type,omitempty"`
    LoadBalancerIP        string                           `json:"loadBalancerIP,omitempty"`
    ExternalTrafficPolicy corev1.ServiceExternalTrafficPolicy `json:"externalTrafficPolicy,omitempty"`
}

Mutability matrix (matches Kubernetes' rules):

Field	Create-time	In-place
Annotations	✓	✓
ClusterIP	✓	— (immutable on Service)
Type	✓	✓ (k8s supports ClusterIP↔NodePort↔LoadBalancer transitions)
LoadBalancerIP	✓	— (immutable on Service)
ExternalTrafficPolicy	✓	✓

Example usage

apiVersion: planetscale.com/v2
kind: VitessCluster
spec:
  gatewayService:
    type: LoadBalancer
    loadBalancerIP: 192.0.2.42
    externalTrafficPolicy: Local
    annotations:
      metallb.universe.tf/address-pool: prod-vips

Why this approach

The same field placement is reused so every caller that already wires ServiceOverrides (cluster-aggregate vtgate, per-cell vtgate, vtctld, vtadmin, etcd-lockserver, vttablet headless) picks up the new fields automatically — no controller-side changes needed. The change is confined to:

• pkg/apis/planetscale/v2/vitesscluster_types.go — three new field declarations on ServiceOverrides.
• pkg/operator/update/service.go — wire the new fields through ServiceOverrides() (all fields) and InPlaceServiceOverrides() (only the mutable ones).
• pkg/operator/update/service_test.go — new unit-tests covering nil, all-fields-applied, empty-fields-don't-clobber, in-place-skips-immutable.
• deploy/crds/*.yaml — regenerated by make generate. No DeepCopy changes — all new fields are simple-type aliases (string, corev1.ServiceType, corev1.ServiceExternalTrafficPolicy) handled by the existing shallow copy.

Test plan

• go build ./... passes
• go test ./pkg/operator/update/ -run TestServiceOverrides -v — 4 new tests pass
• make generate produces the expected CRD diff (5 CRDs updated; extraVolumeMounts-style placement at every service: / gatewayService: / tabletService: etc.)
• Verified by hand: applying a VitessCluster with gatewayService.type: LoadBalancer to a live cluster with the patched operator produces a LoadBalancer Service (vs ClusterIP without the patch).

Compatibility

• Existing manifests without the new fields are unaffected — empty values fall through the if so.Foo != "" guards.
• The CRD additions are all +optional with omitempty tags, so existing tooling that doesn't know about them continues to validate.
• Behavioural change for the in-place reconcile path: Type and ExternalTrafficPolicy are now updated on subsequent reconciles. Operators that previously relied on the operator-managed Service staying ClusterIP regardless of the spec will see the Service flip type if they add a type: override. This is the desired behaviour.

Open questions for reviewers

1. Should LoadBalancerIP be applied in-place even though k8s rejects it on existing Services? Current PR skips it to avoid noisy reconcile errors; happy to add a one-time create-detection if you'd prefer the apply attempt.
2. The PR exposes the new fields uniformly on every callsite that already uses ServiceOverrides. Want me to gate any of them (e.g., disallow Type on the vttablet headless Service, since headless + LoadBalancer is a nonsense combination)?

Signed-off-by

DCO: Signed-off-by: Akhilesh Agarwal <akhilesh.agarwal@gmail.com> on the single commit.

[aagarwal-apexanalytix]

Hi @mattlord — opened this following your invitation on #305. Single commit, DCO-signed, tests included. Happy to iterate on the open questions in the PR body (in-place LoadBalancerIP handling + scoping the new fields per callsite) or anything else.

[mattlord]

In pkg/operator/update/service.go:43-50 I don't think that we should apply external Service fields to every ServiceOverrides target blindly. I say that as ServiceOverrides is reused by "headless" Services too, notably the global vttablet Service and the etcd peer Service. With this change, tabletService.type: LoadBalancer or peerService.type: NodePort leaves clusterIP: None in place and Kubernetes rejects the Service because headless Services cannot be NodePort/LoadBalancer. Separately, externalTrafficPolicy is applied even when the effective Service type is still ClusterIP, despite the API comment saying it is ignored; Kubernetes rejects that as only valid for externally-accessible Services. Unless I'm missing something? If not then I think we should probably split/gate these fields so they are only available for non-headless exposable Services, or validate/ignore invalid combinations before applying them.

You also should run make generate in your branch to update both the deploy/crds/* and the generated API docs as it looks like this PR currently only includes the CRD changes. That should then address the failing test.