sourcegraph: render frontend and migrator command as a list - #1005

Kubernetes does not allow changing a workload's `spec.selector` or a
StatefulSet's `volumeClaimTemplates`. The chart hard-codes both, so a
Helm release cannot take over workloads that were created another way,
such as with Kustomize, unless they are deleted first. A customer moving
from Kustomize to this chart has extra labels in every selector and
volume claim template.

### Changes

Three opt-in values, all empty by default:

- `sourcegraph.selectorLabelsOverride` replaces the default
`app.kubernetes.io/name` and `app.kubernetes.io/instance` selector
labels on every component.
- `<serviceName>.extraSelectorLabels` adds selector labels to one
component.
- `storageLabels` on `gitserver`, `grafana`, `indexedSearch`, and
`searcher` adds labels to the volume claim template.

```yaml
sourcegraph:
  selectorLabelsOverride:
    example.com/project: sourcegraph
gitserver:
  extraSelectorLabels:
    example.com/service: gitserver
  storageLabels:
    example.com/project: sourcegraph
    example.com/service: gitserver
```

How it works:

- `sourcegraph.selectorLabels` now takes the component's values, `(list
. .Values.gitserver)`, so one helper renders a component's workload
selector, pod labels, and Service and PodDisruptionBudget selectors, and
they cannot drift apart. Most of the diff is that argument change at 70
call sites.
- Object metadata labels (`sourcegraph.labels`) keep
`app.kubernetes.io/name` and `app.kubernetes.io/instance` when the
selector labels are replaced.
- The render fails if either value sets a label the chart sets itself
(`app`, `deploy`, `group`, `type`, `worker-replica`), because that would
stop a selector from matching its pods.
- The `grafana` StatefulSet selector is only `app: grafana`. It now also
gets the custom labels when they are set, so an existing `grafana` can
be adopted too.
- The README has a new "Adopting existing workloads" section.

Default values render the same manifests as `main`. Out of scope:
`jaeger`, which has its own selector helper, and the migrator and
executor charts.

This touches most workload, Service, and PodDisruptionBudget templates,
so it will conflict with other open PRs that edit the same lines.

### Checklist

- [x] Follow the [manual testing
process](https://github.com/sourcegraph/deploy-sourcegraph-helm/blob/main/TEST.md):
on an EKS cluster instead of kind, see "Tested on a cluster" below
- [x] Update
[changelog](https://github.com/sourcegraph/deploy-sourcegraph-helm/blob/main/charts/sourcegraph/CHANGELOG.md)
- [ ] Update [Kubernetes update
doc](https://docs.sourcegraph.com/admin/updates/kubernetes): not needed,
no upgrade steps

### Test plan

- New `charts/sourcegraph/tests/selectorLabels_test.yaml`: 9 of its 13
tests fail on `main` and all pass here. The 4 that pass on `main` check
that defaults are unchanged.
- `helm unittest charts/sourcegraph`: 247 tests pass.
- `helm lint charts/sourcegraph` passes.
- `helm template` with default values, and with every optional
component, all PodDisruptionBudgets, and worker replicas enabled, is
byte-identical to `main`.
- With the new values set on every component, a script checked the full
render: every Service and PodDisruptionBudget selector matches the pods
of its own component and no other, the same as with default values.
- Compared a render against the customer's production Kustomize
manifests (8 Deployments, 3 StatefulSets, 14 Services): `spec.selector`,
and for StatefulSets `volumeClaimTemplates`, `serviceName`, and
`podManagementPolicy`, are equal in all 48 fields. The same comparison
against `main` differs in 28.
- Adopted a running Kustomize install into a Helm release on a cluster,
without deleting anything. Details below.

### Tested on a cluster

A Helm release took over a running Kustomize install. No object was
deleted or recreated, and the data survived.

**Setup**

- EKS Auto Mode, Kubernetes v1.37.1, Helm v4.3.1, Sourcegraph 8.1.4873.
- Chart: `main` at `0868743` plus this PR and #1005 (the only conflict
between them is `CHANGELOG.md`). Lint passes, 252 unit tests pass, and
the default render equals `main`.
- Existing install:
[`deploy-sourcegraph-k8s`](https://github.com/sourcegraph/deploy-sourcegraph-k8s)
`v8.1.4873`, applied with `kubectl apply -k`, with three extra labels in
every workload selector, pod template, Service selector, and volume
claim template. Two labels are the same everywhere. The third is per
service, and its value is not always the component name: both Redis
Deployments share one value, and so do the `otel-agent` DaemonSet and
`otel-collector` Deployment.
- Workloads the chart also renders: 9 Deployments, 7 StatefulSets, 1
DaemonSet, and 19 Services, including the bundled databases, `grafana`,
`prometheus`, and OpenTelemetry, with 11 PersistentVolumeClaims (7
standalone, 4 from `volumeClaimTemplates`). One replica each. One
repository was cloned, indexed, and searched before the adoption.

**Results**

| Check | `main` | This PR |
|---|---|---|
| Rendered fields that differ from the live objects (selectors,
`volumeClaimTemplates`, `serviceName`, `podManagementPolicy`) | 39: 19
Service selectors, 9 Deployment selectors, 7 StatefulSet selectors, 4
`volumeClaimTemplates` | 0 |
| `kubectl apply --dry-run=server` of the render over the live objects |
17 workloads rejected: `spec.selector` is immutable on all 17, and
`volumeClaimTemplates` on 4 StatefulSets | 66 objects accepted, 0 errors
|

The adoption itself, with this PR:

```sh
helm upgrade --install sourcegraph charts/sourcegraph \
  --take-ownership --server-side=false \
  --namespace <namespace> --values override.yaml
```

- The release is `deployed`.
- All 70 objects that existed before kept their UID and creation time,
and the PersistentVolumes bound to the 11 claims are unchanged.
- Helm now owns every workload, 20 Services, the 7 standalone
PersistentVolumeClaims, the Ingress, and the RBAC objects.
- Every Service has ready endpoints, and every live selector matches its
pods.
- Unindexed and indexed (`index:only`) searches return the same 5
matches as before, and the access token created before the adoption
still works, so the database, the cloned repository, and the search
index survived.
- A later `helm upgrade` (revision 3, the values from #1005) applied
cleanly.
- The `sourcegraph-executor/k8s` chart, which this PR does not change,
installed next to the adopted release. Its executor registered on the
`batches` and `codeintel` queues, a server-side batch spec completed (1
workspace, 1 line added), and a `scip-typescript` auto-indexing job
completed.

### Lessons learned: check these before adopting a Kustomize install

Each one was observed on the test cluster unless it says otherwise. The
selector labels in this PR are needed, but they were not enough: the
first two attempts failed for the reasons in 1 and 2.

1. **Helm 4 needs `--server-side=false`.** Helm 4 uses server-side apply
for a new release. With only `--take-ownership` it stopped on conflicts
with the `kubectl-client-side-apply` field manager on 38 objects (15
Services, 9 Deployments, 7 StatefulSets, 6 ConfigMaps, 1 DaemonSet).
Client-side apply worked. Helm 3 always applies client-side, and has
`--take-ownership` since 3.17.
2. **An environment variable set with `value` on one side and
`valueFrom` on the other blocks the upgrade.** Kustomize sets
`POSTGRES_PASSWORD` and `POSTGRES_USER` on `codeinsights-db` with
`value`; the chart sets them with `valueFrom.secretKeyRef`. Helm's merge
keeps the old `value` and adds `valueFrom`, and the API server rejects
that: ``valueFrom: may not be specified when `value` is not empty``. I
patched the live StatefulSet's two variables to the chart's
`secretKeyRef` first, then the upgrade passed. The same applies to any
variable a Kustomize patch sets one way and the chart sets the other.
3. **A failed attempt is not rolled back.** The failed server-side
attempt had already created the one object that did not exist yet
(`syntactic-code-intel-worker`). The failed client-side attempt had
already patched every other workload, and their pods restarted at that
point. Plan the first attempt as the real change. No single dry run
predicted it:
- `helm template | kubectl apply --dry-run=server -f -` caught the
immutable selector and `volumeClaimTemplates` errors on `main`, but
accepted `codeinsights-db`, because `kubectl` removes the old `value`
and Helm does not.
- `helm upgrade --install --dry-run=server` reported `Dry run complete`
30 seconds before the real run failed on the 38 conflicts in 1.
- `helm template | kubectl apply --server-side --force-conflicts
--dry-run=server -f -` caught the error in 2.
4. **Every pod restarts.** The chart's pod templates differ from
Kustomize's, so every workload rolled once. Plan for downtime on
single-replica services.
5. **Storage must match too.** `storageClass.name` and every
`storageSize` have to equal the existing claims, because they are part
of the immutable `volumeClaimTemplates`.
6. **What only Kustomize set stays on the objects.** Helm did not remove
fields that are missing from the chart's render. Still on the live
objects after the adoption: extra environment variables
(`PRECISE_CODE_INTEL_UPLOAD_BACKEND` and
`PRECISE_CODE_INTEL_UPLOAD_AWS_ENDPOINT` on `worker` and
`precise-code-intel-worker`, `DATA_SOURCE_NAME` on the three
`pgsql-exporter` sidecars, `USE_ROCKSKIP` and
`ROCKSKIP_MIN_REPO_SIZE_MB` on `searcher`), port 8888 on `otel-agent`,
and Kustomize's object labels. They keep working, but they are not in
the values file, so a later rebuild from the chart alone would not have
them. Compare `kubectl get -o yaml` with `helm get manifest` afterwards,
and move what is still needed into values.
7. **Objects the chart names differently, or does not render, are not
adopted.** Left behind, unowned: the `syntactic-code-intel` Deployment
and Service (the chart creates `syntactic-code-intel-worker` next to
them, so both ran until I deleted the old pair), and the ConfigMaps
`otel-collector`, `prometheus-rbac`, `sourcegraph-frontend-env`,
`sourcegraph-upload`, and `sourcegraph-kustomize-build-config`. The 4
PersistentVolumeClaims created from `volumeClaimTemplates` are also
unowned, which is normal for a StatefulSet. List what the release does
not own afterwards and delete what is no longer used.
8. **Cluster-scoped objects are adopted too.** The release took over the
`prometheus` ClusterRole and ClusterRoleBinding. Deleting the namespace
does not delete them.
9. **A failed first revision did not need a reset.** Running the same
`helm upgrade --install` again moved the release from failed revision 1
to deployed revision 2.

Not tested:

- Helm 3. All runs used Helm 4.3.1.
- Helm 4 server-side apply with `--force-conflicts`. Only a `kubectl
apply --server-side --force-conflicts --dry-run=server` of the render
was run: it accepted 65 objects and rejected `codeinsights-db` for the
reason in 2.
- `helm uninstall` or `helm rollback` on an adopted release. Expect
`helm uninstall` to delete the adopted objects, including the 7
standalone PersistentVolumeClaims.
- Extra containers or volumes that only Kustomize added. From 6, expect
them to stay on the objects, unmanaged.
- More than one replica per service, PodDisruptionBudgets, `jaeger`, and
external databases or object storage.
- The customer's own cluster and manifests. Their manifests were only
compared offline (see the test plan), so their patches can hit cases of
2, 6, and 7 that this test did not.

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: Michael Lin <mlzc@hey.com>