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

feat(appliance): local developer mode#63417

Merged
craigfurman merged 3 commits into
mainfrom
craig/rel-21-appliance-has-a-dev-mode-flag-that-sets-no-resources
Jun 24, 2024
Merged

feat(appliance): local developer mode#63417
craigfurman merged 3 commits into
mainfrom
craig/rel-21-appliance-has-a-dev-mode-flag-that-sets-no-resources

Conversation

@craigfurman

@craigfurman craigfurman commented Jun 21, 2024

Copy link
Copy Markdown
Contributor

feat(appliance): local developer mode

  • Expose a toggle in the web UI to enable dev mode
  • dev mode is currently defined as: no container resource
    requests/limits

fix(appliance): fix misconfigurations to 2 services

Gitserver: the configured probe timeouts were too aggressive.
Indexed-search: the image name was wrong

Both of these were drift from Helm that we didn't catch. Luckily the
appliance is still pre-release!


Closes https://linear.app/sourcegraph/issue/REL-199/populate-accurate-list-of-versions-to-install

Draft until base branch merged, but should be reviewable independently

Test plan

The behavior of the dev mode toggle is unit tested and golden tests cover its integration with the reconcile loop.

Changelog

  • Local developer mode for appliance config authors.

@cla-bot cla-bot Bot added the cla-signed label Jun 21, 2024
@craigfurman craigfurman force-pushed the craig/rel-21-appliance-has-a-dev-mode-flag-that-sets-no-resources branch from ca4f157 to 4b91680 Compare June 21, 2024 09:49
@craigfurman craigfurman force-pushed the craig/rel-199-populate-accurate-list-of-versions-to-install branch from d53b5ae to 64ce31e Compare June 21, 2024 09:52
@craigfurman craigfurman force-pushed the craig/rel-21-appliance-has-a-dev-mode-flag-that-sets-no-resources branch from 4b91680 to 1104857 Compare June 21, 2024 09:53
Base automatically changed from craig/rel-199-populate-accurate-list-of-versions-to-install to main June 24, 2024 09:48
Craig Furman added 2 commits June 24, 2024 10:50
- Expose a toggle in the web UI to enable dev mode
- dev mode is currently defined as: no container resource
  requests/limits
Gitserver: the configured probe timeouts were too aggressive.
Indexed-search: the image name was wrong

Both of these were drift from Helm that we didn't catch. Luckily the
appliance is still pre-release!
@craigfurman craigfurman force-pushed the craig/rel-21-appliance-has-a-dev-mode-flag-that-sets-no-resources branch from 1104857 to 80e6a2c Compare June 24, 2024 09:50
@craigfurman craigfurman marked this pull request as ready for review June 24, 2024 09:50
@craigfurman

Copy link
Copy Markdown
Contributor Author

Fun fact: if we apply the following patch on top of this PR, the appliance can deploy all resources with the dev-mode toggle switched on without errors!

diff --git a/internal/appliance/reconciler/frontend.go b/internal/appliance/reconciler/frontend.go
index fdb5bcbd528..6f41191da55 100644
--- a/internal/appliance/reconciler/frontend.go
+++ b/internal/appliance/reconciler/frontend.go
@@ -269,7 +269,7 @@ func (r *Reconciler) reconcileFrontendIngress(ctx context.Context, sg *config.So
 
 func frontendEnvVars(sg *config.Sourcegraph) []corev1.EnvVar {
 	vars := []corev1.EnvVar{
-		{Name: "DEPLOY_TYPE", Value: "appliance"},
+		{Name: "DEPLOY_TYPE", Value: "helm"},
 	}
 	if !sg.Spec.Grafana.Disabled {
 		vars = append(vars, corev1.EnvVar{Name: "GRAFANA_SERVER_URL", Value: "http://grafana:30070"})

Why do we need the patch, I hear you say? Because the error preventing startup without it is fixed by the first commit in https://github.com/sourcegraph/sourcegraph/pull/63158, and we've not cut a release from main since. I probably can't easily backport it due to the fact that it's squash-merged with the other commit in the same PR, which is larger and would probably cause cherry-pick conflicts 🙈

We'll be cutting a minor release in July, so I'm not too worried. Just a note to fellow developers for testing. cc @sourcegraph/release - please kick the tires on the appliance!

@craigfurman craigfurman merged commit e5be8af into main Jun 24, 2024
@craigfurman craigfurman deleted the craig/rel-21-appliance-has-a-dev-mode-flag-that-sets-no-resources branch June 24, 2024 15:19
// the BestEffortQOS flag to influence the behavior in that file.
// While this is admittedly a bit of a confusing leaky abstraction,
// the golden tests should catch any regressions in the interactions
// between these 2 bits of code.

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.

Nice comment 👍🏼

@craigfurman craigfurman Jun 24, 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.

Yeah, real comment of last resort. The tl;dr is "I'm sorry for writing confusing code. In this case, I can't figure out how to make it easier to understand without making something else significantly worse" 😂

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.

3 participants