sourcegraph: Drop redundant PRECISE_CODE_INTEL_UPLOAD_* env (#992)

Follow-up to sourcegraph/deploy-sourcegraph-docker#1461, same cleanup
for the Helm chart.

Four templates still set `PRECISE_CODE_INTEL_UPLOAD_BACKEND=blobstore`
and `PRECISE_CODE_INTEL_UPLOAD_AWS_ENDPOINT=http://blobstore:9000` under
`{{ if .Values.blobstore.enabled }}`: `frontend`, `worker`,
`precise-code-intel-worker`, and `syntactic-code-intel-worker`. A
customer reading the templates could reasonably think both
`PRECISE_CODE_INTEL_UPLOAD_*` and `SOURCEGRAPH_UPLOAD_*` need changing
to move to S3 or GCS. This removes the legacy lines so
`SOURCEGRAPH_UPLOAD_*` in `*.env` is the only object storage
configuration in the chart, matching the `external-object-storage`
examples.

The removed values did nothing:

- They are the built-in defaults: `PRECISE_CODE_INTEL_UPLOAD_BACKEND`
defaults to `blobstore`
([lsifuploadstore/config.go](https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/codeintel/shared/lsifuploadstore/config.go?L31)),
and the endpoint defaults to `http://blobstore:9000` outside the
single-container deployment
([deploy/endpoints.go](https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/conf/deploy/endpoints.go)).
- When the legacy backend is `blobstore`,
[`resolveLegacyCompatStorage`](https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/object/sourcegraphobjects/backcompat.go?L20-L44)
uses the shared `SOURCEGRAPH_UPLOAD_*` config and keeps only the legacy
bucket name, so the legacy endpoint was never read.

## Impact on existing deployments

- **Default (bundled blobstore):** no change. The rendered env loses two
variables whose values equal the compiled-in defaults, so pods restart
on upgrade (env changed) but behave identically: bundled blobstore,
`lsif-uploads` bucket.
- **`blobstore.enabled: false` with `SOURCEGRAPH_UPLOAD_*` in `*.env`
(the documented path):** no change at all. The removed block never
rendered for these deployments, so the manifests are byte-identical.
- **`blobstore.enabled: true` with `SOURCEGRAPH_UPLOAD_BACKEND=S3|GCS`
in `*.env`:** no change. Code-intel uploads already went to the shared
bucket under the `lsif-uploads/` prefix via the compat shim, and still
do.
- **`blobstore.enabled: true` with
`PRECISE_CODE_INTEL_UPLOAD_BACKEND=S3|GCS` in `*.env`:** behaviour fix,
one upgrade later. The chart's hardcoded `blobstore` rendered *after*
`*.env`, and Kubernetes takes the last duplicate env name, so these
overrides were silently ignored and uploads went to the bundled
blobstore. On the upgrade to this chart, Helm's three-way merge deletes
every env entry with those names, the `*.env` override included, so the
live Deployments carry neither value and runtime behaviour is unchanged
(the deleted values equal the built-in defaults). On the *next* `helm
upgrade`, even one with no changes, the override is rendered once, takes
effect, and uploads switch to the dedicated bucket the customer
configured. Before that second upgrade, make sure the bucket exists and
is reachable, or move the settings to `SOURCEGRAPH_UPLOAD_*`, or remove
them. Running a second `helm upgrade` straight away makes the switch
happen at a known time. Existing uploads in the bundled blobstore are
not migrated. Verified on a kind cluster by @bobheadxi's agent, see the
comment below.

## Impact on new deployments

None beyond less to read. The `external-object-storage` examples already
disable blobstore and set only `SOURCEGRAPH_UPLOAD_*`.

## Test plan

- `helm lint` and `helm unittest ./charts/sourcegraph` pass (218 tests,
12 snapshots).
- `helm template` of `origin/main` vs this branch with default values:
the only difference is the two variables removed from the three services
that render by default (`syntactic-code-intel-worker` is disabled by
default).
- `helm template --set
frontend.env.PRECISE_CODE_INTEL_UPLOAD_BACKEND.value=S3` on
`origin/main` renders `S3` followed by a second `blobstore` entry; on
this branch it renders `S3` once.
- CI.

---------

Co-authored-by: Amp <amp@ampcode.com>

Author: marcleblanc2

charts/sourcegraph/CHANGELOG.md

@@ -8,6 +8,7 @@ Use `**BREAKING**:` to denote a breaking change
 
 ## Unreleased
 
+- Removed the hardcoded `PRECISE_CODE_INTEL_UPLOAD_BACKEND=blobstore` and `PRECISE_CODE_INTEL_UPLOAD_AWS_ENDPOINT=http://blobstore:9000` environment variables from `frontend`, `worker`, `precise-code-intel-worker`, and `syntactic-code-intel-worker`. They matched Sourcegraph's built-in defaults, so default deployments are unchanged. `SOURCEGRAPH_UPLOAD_*` in `*.env` is now the only object storage configuration in the chart, as in the `external-object-storage` examples. Previously, with `blobstore.enabled: true`, the chart's values were rendered after `*.env` and overrode any `PRECISE_CODE_INTEL_UPLOAD_*` set there. Helm's three-way merge removes every duplicate of those names on this upgrade, including the `*.env` override, and the override is applied on the next `helm upgrade` after this one, when code-intel uploads switch to its bucket. Before that, make sure the bucket exists, or move the settings to `SOURCEGRAPH_UPLOAD_*`, or remove them. Run a second `helm upgrade` right away to make the switch at a known time.
 - Added `syntacticCodeIntel.storageType`, `syntacticCodeIntel.storageSize`, and `syntacticCodeIntel.storageClassName`, so the `syntactic-code-intel-worker` `/tmp` scratch volume can be a per-pod PVC (generic ephemeral volume) instead of an `emptyDir` on the node boot disk, matching `preciseCodeIntel`. The default remains an unbounded `emptyDir`.
 - Added an `outbound-http-proxy` example, which routes outbound traffic from frontend, worker, and migrator through an HTTP proxy with `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY`
 - Fixed the `codeintel-db` PersistentVolumeClaim being renamed by `codeIntelDB.name`, which left the StatefulSet pointing at a `codeintel-db` claim that no longer existed and stuck the pod in `Pending`. The PVC is now always named `codeintel-db`, matching the other components and the documented behaviour of `*.name`. If you worked around this by creating a `codeintel-db` PVC by hand, label it `app.kubernetes.io/managed-by: Helm` and annotate it with `meta.helm.sh/release-name` and `meta.helm.sh/release-namespace` before upgrading so Helm can adopt it. [#612](https://github.com/sourcegraph/deploy-sourcegraph-helm/issues/612)

charts/sourcegraph/templates/_worker.tpl

@@ -82,12 +82,6 @@ spec:
         - name: {{ $name }}
           {{- $item | toYaml | nindent 10 }}
         {{- end }}
-        {{- if $top.Values.blobstore.enabled }}
-        - name: PRECISE_CODE_INTEL_UPLOAD_BACKEND
-          value: blobstore
-        - name: PRECISE_CODE_INTEL_UPLOAD_AWS_ENDPOINT
-          value: http://blobstore:9000
-        {{- end }}
         - name: POD_NAME
           valueFrom:
             fieldRef:

charts/sourcegraph/templates/frontend/sourcegraph-frontend.Deployment.yaml

@@ -99,12 +99,6 @@ spec:
         {{- end }}
         {{- include "sourcegraph.redisConnection" .| nindent 8 }}
         {{- include "sourcegraph.openTelemetryEnv" . | nindent 8 }}
-        {{- if .Values.blobstore.enabled }}
-        - name: PRECISE_CODE_INTEL_UPLOAD_BACKEND
-          value: blobstore
-        - name: PRECISE_CODE_INTEL_UPLOAD_AWS_ENDPOINT
-          value: http://blobstore:9000
-        {{- end }}
         terminationMessagePolicy: FallbackToLogsOnError
         livenessProbe:
           httpGet:

charts/sourcegraph/templates/precise-code-intel/worker.Deployment.yaml

@@ -62,12 +62,6 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.name
-        {{- if .Values.blobstore.enabled }}
-        - name: PRECISE_CODE_INTEL_UPLOAD_BACKEND
-          value: blobstore
-        - name: PRECISE_CODE_INTEL_UPLOAD_AWS_ENDPOINT
-          value: http://blobstore:9000
-        {{- end }}
         {{- include "sourcegraph.openTelemetryEnv" . | nindent 8 }}
         image: {{ include "sourcegraph.image" (list . "preciseCodeIntel") }}
         imagePullPolicy: {{ .Values.sourcegraph.image.pullPolicy }}

charts/sourcegraph/templates/syntactic-code-intel/worker.Deployment.yaml

@@ -63,12 +63,6 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.name
-        {{- if .Values.blobstore.enabled }}
-        - name: PRECISE_CODE_INTEL_UPLOAD_BACKEND
-          value: blobstore
-        - name: PRECISE_CODE_INTEL_UPLOAD_AWS_ENDPOINT
-          value: http://blobstore:9000
-        {{- end }}
         {{- include "sourcegraph.openTelemetryEnv" . | nindent 8 }}
         image: {{ include "sourcegraph.image" (list . "syntacticCodeIntel") }}
         imagePullPolicy: {{ .Values.sourcegraph.image.pullPolicy }}