Conversation
There was a problem hiding this comment.
Yikes! This one is my bad from a previous PR. Shows how error-prone it can be doing all the grunt work of service configs... the golden tests didn't catch this (and are unchanged here), for a complicated and fragile reason: NewService() takes the optional standard config element and checks for a prometheus port, optionally adding the relevant scrape directive annotations.
Symbols and RepoUpdater happen to have the same prometheus port. If an admin had overridden this in symbols' config, they would have exposed a bug, since the repo-updater config is being checked here. But, overriding this in config wouldn't make any sense, since the service unconditionally listens on some port to expose prometheus metrics - that aspect can't be overridden. Even though there is probably little chance of this bug leaking out, it is something to keep in mind...
There was a problem hiding this comment.
Annotated output of:
❯ go run ./internal/appliance/dev/compare-helm \
-deploy-sourcegraph-helm-path ../../deploy-sourcegraph-helm \
-component searcher \
-golden-file internal/appliance/reconciler/testdata/golden-fixtures/searcher/default.yaml \
-helm-template-extra-args '--set searcher.serviceAccount.create=true'
One thing I've been forgetting to note lately in these compare-helm diffs: the
commit that is checked out in my deploy-sourcegraph-helm directory! I'm using
v5.3.9104, the same as the requested version-under-test.
In
https://linear.app/sourcegraph/issue/REL-122/run-compare-helm-against-all-services-and-squash-any-problematic
we can bump that version to latest and double-check our configs against helm.
I can't see much interesting here, but as usual please take a look.
2c2
< # helm: ServiceAccount/searcher
---
> # golden: ServiceAccount/searcher
5a6,8
> annotations:
> appliance.sourcegraph.com/configHash: 2fec30f60a396508a28da4a0d7c03ecbe530b8370ddb0ae38b355e52bd844446
> creationTimestamp: "2024-04-19T00:00:00Z"
7,8d9
< app.kubernetes.io/component: searcher
< category: rbac
10a12,21
> namespace: NORMALIZED_FOR_TESTING
> ownerReferences:
> - apiVersion: v1
> blockOwnerDeletion: true
> controller: true
> kind: ConfigMap
> name: sg
> uid: NORMALIZED_FOR_TESTING
> resourceVersion: NORMALIZED_FOR_TESTING
> uid: NORMALIZED_FOR_TESTING
12c23
< # helm: Service/searcher
---
> # golden: Service/searcher
16a28
> appliance.sourcegraph.com/configHash: 2fec30f60a396508a28da4a0d7c03ecbe530b8370ddb0ae38b355e52bd844446
18a31
> creationTimestamp: "2024-04-19T00:00:00Z"
23a37,46
> namespace: NORMALIZED_FOR_TESTING
> ownerReferences:
> - apiVersion: v1
> blockOwnerDeletion: true
> controller: true
> kind: ConfigMap
> name: sg
> uid: NORMALIZED_FOR_TESTING
> resourceVersion: NORMALIZED_FOR_TESTING
> uid: NORMALIZED_FOR_TESTING
25c48,54
< clusterIP: None
---
> clusterIP: NORMALIZED_FOR_TESTING
> clusterIPs:
> - NORMALIZED_FOR_TESTING
> internalTrafficPolicy: Cluster
> ipFamilies:
> - IPv4
> ipFamilyPolicy: SingleStack
28a58
> protocol: TCP
31a62
> protocol: TCP
35,36c66,69
< app.kubernetes.io/instance: release-name
< app.kubernetes.io/name: sourcegraph
---
> sessionAffinity: None
> type: ClusterIP
> status:
> loadBalancer: {}
38c71
< # helm: StatefulSet/searcher
---
> # golden: StatefulSet/searcher
43c76,78
< description: Backend for text search operations.
---
> appliance.sourcegraph.com/configHash: 2fec30f60a396508a28da4a0d7c03ecbe530b8370ddb0ae38b355e52bd844446
> creationTimestamp: "2024-04-19T00:00:00Z"
> generation: 1
46,47d80
< app.kubernetes.io/instance: release-name
< app.kubernetes.io/managed-by: Helm
51d83
< helm.sh/chart: sourcegraph-5.3.9104
52a85,94
> namespace: NORMALIZED_FOR_TESTING
> ownerReferences:
> - apiVersion: v1
> blockOwnerDeletion: true
> controller: true
> kind: ConfigMap
> name: sg
> uid: NORMALIZED_FOR_TESTING
> resourceVersion: NORMALIZED_FOR_TESTING
> uid: NORMALIZED_FOR_TESTING
53a96,100
> minReadySeconds: 10
> persistentVolumeClaimRetentionPolicy:
> whenDeleted: Retain
> whenScaled: Retain
> podManagementPolicy: OrderedReady
59,60d105
< app.kubernetes.io/instance: release-name
< app.kubernetes.io/name: sourcegraph
65d109
< checksum/redis: 63b58e05a2640417d599c4aee6d866cb9063e3a9aa452dc08dbfff836b7781b7
66a111
> creationTimestamp: null
69,70d113
< app.kubernetes.io/instance: release-name
< app.kubernetes.io/name: sourcegraph
71a115
> name: searcher
73d116
< affinity: null
87c130
< value: "23400"
---
> value: "23961"1024 MB to a GiB vs 1000 to a GB difference, probably. This code backing this
parses the storage size and sets the value of this var to 90% of it.
90a134
> apiVersion: v1
96a141
> apiVersion: v1
100c145
< image: index.docker.io/sourcegraph/searcher:5.3.9104@sha256:abf63df1d39a5dce8d506551b798050b7ad008faefd5f319128bd0355736a79a
---
> image: index.docker.io/sourcegraph/searcher:5.3.9104
105a151
> protocol: TCP
107a154
> protocol: TCP
114a162
> successThreshold: 1
127a176
> terminationMessagePath: /dev/termination-log
134c183,185
< nodeSelector: null
---
> dnsPolicy: ClusterFirst
> restartPolicy: Always
> schedulerName: default-scheduler
137a189
> runAsGroup: 101
138a191
> serviceAccount: searcher
140c193
< tolerations: null
---
> terminationGracePeriodSeconds: 30
149c202,207
< - metadata:
---
> - apiVersion: v1
> kind: PersistentVolumeClaim
> metadata:
> creationTimestamp: null
> labels:
> deploy: sourcegraph
150a209
> namespace: NORMALIZED_FOR_TESTING
157c216,291
< storageClassName: sourcegraph
---
> volumeMode: Filesystem
> status:
> phase: Pending
> status:
> availableReplicas: 0
> replicas: 0
> ---
> # golden: ConfigMap/sg
> apiVersion: v1
> data:
> spec: |
> spec:
> requestedVersion: "5.3.9104"
>
> blobstore:
> disabled: true
>
> codeInsights:
> disabled: true
>
> codeIntel:
> disabled: true
>
> frontend:
> disabled: true
>
> gitServer:
> disabled: true
>
> indexedSearch:
> disabled: true
>
> indexedSearchIndexer:
> disabled: true
>
> pgsql:
> disabled: true
>
> postgresExporter:
> disabled: true
>
> preciseCodeIntel:
> disabled: true
>
> redisCache:
> disabled: true
>
> redisStore:
> disabled: true
>
> repoUpdater:
> disabled: true
>
> searcher: {}
>
> symbols:
> disabled: true
>
> syntectServer:
> disabled: true
>
> worker:
> disabled: true
>
> prometheus:
> disabled: true
> kind: ConfigMap
> metadata:
> annotations:
> appliance.sourcegraph.com/currentVersion: 5.3.9104
> appliance.sourcegraph.com/managed: "true"
> creationTimestamp: "2024-04-19T00:00:00Z"
> name: sg
> namespace: NORMALIZED_FOR_TESTING
> resourceVersion: NORMALIZED_FOR_TESTING
> uid: NORMALIZED_FOR_TESTINGThere was a problem hiding this comment.
One thing I've been forgetting to note lately in these compare-helm diffs: the
commit that is checked out in my deploy-sourcegraph-helm directory! I'm using
v5.3.9104, the same as the requested version-under-test.
👍
There was a problem hiding this comment.
❯ diff internal/appliance/reconciler/testdata/golden-fixtures/searcher/default.yaml internal/appliance/reconciler/testdata/golden-fixtures/searcher/with-storage.yaml
6c6
< appliance.sourcegraph.com/configHash: 2fec30f60a396508a28da4a0d7c03ecbe530b8370ddb0ae38b355e52bd844446
---
> appliance.sourcegraph.com/configHash: 25b03d6fa226ee5e00548da8fffd9b6a70d3f14b8b1ea254b174105c425c2cdd
60c60
< value: "23961"
---
> value: "921600"
145c145
< storage: 26Gi
---
> storage: 1000Gi
197c197,199
< searcher: {}
---
> searcher:
> persistentVolumeConfig:
> storageSize: "1000Gi"
224c226
< appliance.sourcegraph.com/configHash: 2fec30f60a396508a28da4a0d7c03ecbe530b8370ddb0ae38b355e52bd844446
---
> appliance.sourcegraph.com/configHash: 25b03d6fa226ee5e00548da8fffd9b6a70d3f14b8b1ea254b174105c425c2cdd
243c245
< appliance.sourcegraph.com/configHash: 2fec30f60a396508a28da4a0d7c03ecbe530b8370ddb0ae38b355e52bd844446
---
> appliance.sourcegraph.com/configHash: 25b03d6fa226ee5e00548da8fffd9b6a70d3f14b8b1ea254b174105c425c2cddThere was a problem hiding this comment.
❯ diff internal/appliance/reconciler/testdata/golden-fixtures/searcher/default.yaml internal/appliance/reconciler/testdata/golden-fixtures/searcher/with-replicas.yaml
6c6
< appliance.sourcegraph.com/configHash: 2fec30f60a396508a28da4a0d7c03ecbe530b8370ddb0ae38b355e52bd844446
---
> appliance.sourcegraph.com/configHash: c878c6daa974c7ef411448ebabf07a65e9eb202152898e1f786066a15de2209a
31c31
< replicas: 1
---
> replicas: 8
197c197,198
< searcher: {}
---
> searcher:
> replicas: 8
224c225
< appliance.sourcegraph.com/configHash: 2fec30f60a396508a28da4a0d7c03ecbe530b8370ddb0ae38b355e52bd844446
---
> appliance.sourcegraph.com/configHash: c878c6daa974c7ef411448ebabf07a65e9eb202152898e1f786066a15de2209a
243c244
< appliance.sourcegraph.com/configHash: 2fec30f60a396508a28da4a0d7c03ecbe530b8370ddb0ae38b355e52bd844446
---
> appliance.sourcegraph.com/configHash: c878c6daa974c7ef411448ebabf07a65e9eb202152898e1f786066a15de2209a725b233 to
f5f977a
Compare
Fix reference to incorrect service in appliance config
feat(appliance): deploy searcher
Draft until base branch merged but can be reviewed independently.
Closes https://linear.app/sourcegraph/issue/REL-82/service-definition-searcher
Test plan
Golden tests included.
Changelog