Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

feat(appliance): deploy worker#62964

Merged
craigfurman merged 1 commit into
mainfrom
craig/rel-83-service-definition-worker
Jun 4, 2024
Merged

feat(appliance): deploy worker#62964
craigfurman merged 1 commit into
mainfrom
craig/rel-83-service-definition-worker

Conversation

@craigfurman

@craigfurman craigfurman commented May 29, 2024

Copy link
Copy Markdown
Contributor

Closes https://linear.app/sourcegraph/issue/REL-83/service-definition-worker

Test plan

Golden tests included.

Changelog

  • Deploys the Worker service.

@cla-bot cla-bot Bot added the cla-signed label May 29, 2024
@craigfurman craigfurman force-pushed the craig/rel-83-service-definition-worker branch from 8edbc25 to 8019999 Compare May 29, 2024 14:55

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annotated output of:

go run ./internal/appliance/dev/compare-helm \
  -deploy-sourcegraph-helm-path ../../deploy-sourcegraph-helm \
  -component worker \
  -golden-file internal/appliance/reconciler/testdata/golden-fixtures/worker/default.yaml \
  -helm-template-extra-args '--set worker.serviceAccount.create=true'

Nothing much stands out to me.

2c2
< # helm: ServiceAccount/worker
---
> # golden: ServiceAccount/worker
5a6,8
>   annotations:
>     appliance.sourcegraph.com/configHash: 24debcd57654ec9bad2bc69454c6c9eb407fa072782198b3410f88874cd2eb25
>   creationTimestamp: "2024-04-19T00:00:00Z"
7,8d9
<     app.kubernetes.io/component: worker
<     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/worker-executors
---
> # golden: Service/worker-executors
16a28
>     appliance.sourcegraph.com/configHash: 24debcd57654ec9bad2bc69454c6c9eb407fa072782198b3410f88874cd2eb25
18a31
>   creationTimestamp: "2024-04-19T00:00:00Z"
20,21c33,34
<     app: worker
<     app.kubernetes.io/component: worker
---
>     app: worker-executors
>     app.kubernetes.io/component: worker-executors
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
24a48,54
>   clusterIP: NORMALIZED_FOR_TESTING
>   clusterIPs:
>     - NORMALIZED_FOR_TESTING
>   internalTrafficPolicy: Cluster
>   ipFamilies:
>     - IPv4
>   ipFamilyPolicy: SingleStack
27a58
>       protocol: TCP
31,32c62
<     app.kubernetes.io/instance: release-name
<     app.kubernetes.io/name: sourcegraph
---
>   sessionAffinity: None
33a64,65
> status:
>   loadBalancer: {}
35c67
< # helm: Service/worker
---
> # golden: Service/worker
39a72
>     appliance.sourcegraph.com/configHash: 24debcd57654ec9bad2bc69454c6c9eb407fa072782198b3410f88874cd2eb25
41a75
>   creationTimestamp: "2024-04-19T00:00:00Z"
46a81,90
>   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
47a92,98
>   clusterIP: NORMALIZED_FOR_TESTING
>   clusterIPs:
>     - NORMALIZED_FOR_TESTING
>   internalTrafficPolicy: Cluster
>   ipFamilies:
>     - IPv4
>   ipFamilyPolicy: SingleStack
50a102
>       protocol: TCP
53a106
>       protocol: TCP
57,58c110
<     app.kubernetes.io/instance: release-name
<     app.kubernetes.io/name: sourcegraph
---
>   sessionAffinity: None
59a112,113
> status:
>   loadBalancer: {}
61c115
< # helm: Deployment/worker
---
> # golden: Deployment/worker
66c120,122
<     description: Manages background processes.
---
>     appliance.sourcegraph.com/configHash: 24debcd57654ec9bad2bc69454c6c9eb407fa072782198b3410f88874cd2eb25
>   creationTimestamp: "2024-04-19T00:00:00Z"
>   generation: 1
69,70d124
<     app.kubernetes.io/instance: release-name
<     app.kubernetes.io/managed-by: Helm
72c126
<     app.kubernetes.io/version: 5.3.2
---
>     app.kubernetes.io/version: 5.3.9104
74d127
<     helm.sh/chart: sourcegraph-5.3.2
75a129,138
>   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
77a141
>   progressDeadlineSeconds: 600
83,84d146
<       app.kubernetes.io/instance: release-name
<       app.kubernetes.io/name: sourcegraph
93d154
<         checksum/redis: 63b58e05a2640417d599c4aee6d866cb9063e3a9aa452dc08dbfff836b7781b7
94a156
>       creationTimestamp: null
97,98d158
<         app.kubernetes.io/instance: release-name
<         app.kubernetes.io/name: sourcegraph
99a160
>       name: worker
101d161
<       affinity: null
114,117d173
<             - name: PRECISE_CODE_INTEL_UPLOAD_BACKEND
<               value: blobstore
<             - name: PRECISE_CODE_INTEL_UPLOAD_AWS_ENDPOINT
<               value: http://blobstore:9000

Compare the golden fixture for this test to with-blobstore.yaml - you can see these vars present when we enable the blobstore.

120a177
>                   apiVersion: v1
124a182
>                   apiVersion: v1
130a189
>             failureThreshold: 3
135a195,196
>             periodSeconds: 10
>             successThreshold: 1
140a202
>               protocol: TCP
142a205
>               protocol: TCP
144a208
>               protocol: TCP
145a210
>             failureThreshold: 3
150a216
>             successThreshold: 1
163a230
>           terminationMessagePath: /dev/termination-log
165,170c232,314
<           volumeMounts: null
<       nodeSelector: null
<       securityContext: {}
<       serviceAccountName: worker
<       tolerations: null
<       volumes: null
---
>       dnsPolicy: ClusterFirst
>       restartPolicy: Always
>       schedulerName: default-scheduler
>       securityContext:
>         fsGroup: 101
>         fsGroupChangePolicy: OnRootMismatch
>         runAsGroup: 101
>         runAsUser: 100
>       terminationGracePeriodSeconds: 30
> status: {}
> ---
> # 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:
>         disabled: true
> 
>       symbols:
>         disabled: true
> 
>       syntectServer:
>         disabled: true
> 
>       worker: {}
> 
>       prometheus:
>         disabled: true
> 
>       embeddings:
>         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_TESTING

Comment thread internal/appliance/reconciler/worker.go Outdated
Comment on lines 62 to 68

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this logic is a little different to what we do in the helm chart: https://github.com/sourcegraph/deploy-sourcegraph-helm/blob/76581d9e2d4c6cfe4e92b2b58c8f5dac87a97b07/charts/sourcegraph/templates/worker/worker.Deployment.yaml#L63-L70

AFAICT, we want to set these vars only when we are using the bundled blobstore - otherwise that hostname makes no sense! Users of external blobstores will have to configure these vars using the containerConfig (see config subpackage, and standard_config_test.go).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with the logic here. Although I'm too familiar with the external blobstore stuff.

@craigfurman craigfurman Jun 4, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's actually an issue to rip out embeddings, only opened long after I opened this PR 😅 https://linear.app/sourcegraph/issue/REL-125/remove-references-to-embeddings

Let's just merge this for now and defer making sure we really do want to remove embeddings for that issue

@craigfurman craigfurman requested review from a team and sourcegraph-release-bot and removed request for sourcegraph-release-bot May 29, 2024 15:01
@craigfurman craigfurman force-pushed the craig/rel-83-service-definition-worker branch from 8019999 to 2b14e43 Compare May 29, 2024 15:03
@craigfurman craigfurman requested review from a team and DaedalusG and removed request for a team May 29, 2024 15:03
Base automatically changed from appliance-embeddings-scaffolding to main May 30, 2024 14:24
@craigfurman craigfurman force-pushed the craig/rel-83-service-definition-worker branch from 2b14e43 to 5705890 Compare May 30, 2024 14:25
@craigfurman craigfurman marked this pull request as ready for review May 30, 2024 14:25
@craigfurman craigfurman force-pushed the craig/rel-83-service-definition-worker branch from 5705890 to 23d426b Compare June 3, 2024 09:03
@craigfurman craigfurman force-pushed the craig/rel-83-service-definition-worker branch from 23d426b to 9791082 Compare June 3, 2024 09:05
return reconcileObject(ctx, r, cfg, &sa, &corev1.ServiceAccount{}, sg, owner)
}

func addPreciseCodeIntelBlobstoreVars(env []corev1.EnvVar, sg *config.Sourcegraph) []corev1.EnvVar {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a huge conceptual change -- here is the *config.Sourcegraph type mapped to the configMap the customer is using similar to a values.yaml file in helm? Looks like the API 👀 - chat more later

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm not quite understanding this comment. This is a refactor in this file, extracting a function to add some env vars that are useful in 2 places, and calling it from the new worker.go. We can see that it's a refactor because the precise-code-intel golden fixtures haven't changed and the tests are still passing.

is the *config.Sourcegraph type mapped to the configMap the customer is using similar to a values.yaml file in helm?

Yep! 👍

Looks like the API 👀

I'm not 100% sure what you mean, but indeed the ConfigMap schema is the API for deploying sourcegraph here. The appliance frontend will craft that configmap in response to user input, but users can ignore the web app and craft a configmap directly if they like 🙂

@DaedalusG DaedalusG left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, very similar to how we were doing things with CRDs

@craigfurman craigfurman merged commit 79c912d into main Jun 4, 2024
@craigfurman craigfurman deleted the craig/rel-83-service-definition-worker branch June 4, 2024 09:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants