feat(appliance): local developer mode (#63417)

Craig Furman · web-flow · commit e5be8af7243f · 2024-06-24T16:19:27.000+01:00
**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
diff --git a/internal/appliance/appliance.go b/internal/appliance/appliance.go
@@ -21,7 +21,7 @@ type Appliance struct {
 	client                 client.Client
 	namespace              string
 	status                 Status
-	sourcegraph            config.Sourcegraph
+	sourcegraph            *config.Sourcegraph
 	releaseRegistryClient  *releaseregistry.Client
 	latestSupportedVersion string
 	logger                 log.Logger
@@ -58,7 +58,7 @@ func NewAppliance(
 		latestSupportedVersion: latestSupportedVersion,
 		namespace:              namespace,
 		status:                 StatusSetup,
-		sourcegraph:            config.Sourcegraph{},
+		sourcegraph:            &config.Sourcegraph{},
 		logger:                 logger,
 	}
 }
diff --git a/internal/appliance/config/BUILD.bazel b/internal/appliance/config/BUILD.bazel
@@ -32,5 +32,9 @@ go_test(
     srcs = ["dev_mode_test.go"],
     embed = [":config"],
     tags = [TAG_INFRA_RELEASE],
-    deps = ["@com_github_stretchr_testify//assert"],
+    deps = [
+        "@com_github_stretchr_testify//assert",
+        "@io_k8s_api//core/v1:core",
+        "@io_k8s_apimachinery//pkg/api/resource",
+    ],
 )
diff --git a/internal/appliance/config/dev_mode.go b/internal/appliance/config/dev_mode.go
@@ -1,22 +1,41 @@
 package config
 
-func SetLocalDevMode(sg *Sourcegraph) {
+func (sg *Sourcegraph) SetLocalDevMode() {
 	sg.Spec.Blobstore.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.Blobstore.ContainerConfig, "blobstore")
+	sg.Spec.Cadvisor.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.Cadvisor.ContainerConfig, "cadvisor")
+
+	sg.Spec.CodeInsights.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.CodeInsights.ContainerConfig, "correct-data-dir-permissions")
+	sg.Spec.CodeInsights.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.CodeInsights.ContainerConfig, "codeinsights")
+	sg.Spec.CodeInsights.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.CodeInsights.ContainerConfig, "pgsql-exporter")
+
+	sg.Spec.CodeIntel.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.CodeIntel.ContainerConfig, "correct-data-dir-permissions")
+	sg.Spec.CodeIntel.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.CodeIntel.ContainerConfig, "codeintel-db")
+	sg.Spec.CodeIntel.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.CodeIntel.ContainerConfig, "pgsql-exporter")
+
+	sg.Spec.Frontend.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.Frontend.ContainerConfig, "frontend")
+	sg.Spec.Frontend.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.Frontend.ContainerConfig, "migrator")
+
 	sg.Spec.GitServer.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.GitServer.ContainerConfig, "gitserver")
 
+	sg.Spec.IndexedSearch.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.IndexedSearch.ContainerConfig, "zoekt-webserver")
+	sg.Spec.IndexedSearch.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.IndexedSearch.ContainerConfig, "zoekt-indexserver")
+
 	sg.Spec.PGSQL.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.PGSQL.ContainerConfig, "correct-data-dir-permissions")
 	sg.Spec.PGSQL.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.PGSQL.ContainerConfig, "pgsql")
 	sg.Spec.PGSQL.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.PGSQL.ContainerConfig, "pgsql-exporter")
 
+	sg.Spec.PreciseCodeIntel.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.PreciseCodeIntel.ContainerConfig, "precise-code-intel-worker")
+	sg.Spec.Prometheus.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.Prometheus.ContainerConfig, "prometheus")
+	sg.Spec.RepoUpdater.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.RepoUpdater.ContainerConfig, "repo-updater")
+
 	sg.Spec.RedisCache.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.RedisCache.ContainerConfig, "redis-cache")
 	sg.Spec.RedisCache.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.RedisCache.ContainerConfig, "redis-exporter")
 	sg.Spec.RedisStore.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.RedisStore.ContainerConfig, "redis-store")
 	sg.Spec.RedisStore.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.RedisStore.ContainerConfig, "redis-exporter")
 
-	sg.Spec.PreciseCodeIntel.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.PreciseCodeIntel.ContainerConfig, "precise-code-intel-worker")
 	sg.Spec.Symbols.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.Symbols.ContainerConfig, "symbols")
 	sg.Spec.SyntectServer.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.SyntectServer.ContainerConfig, "syntect-server")
-	sg.Spec.RepoUpdater.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.RepoUpdater.ContainerConfig, "repo-updater")
+	sg.Spec.Worker.ContainerConfig = setBestEffortQOSOnContainer(sg.Spec.Worker.ContainerConfig, "worker")
 }
 
 func setBestEffortQOSOnContainer(ctrConfigs map[string]ContainerConfig, container string) map[string]ContainerConfig {
@@ -25,7 +44,6 @@ func setBestEffortQOSOnContainer(ctrConfigs map[string]ContainerConfig, containe
 	}
 	ctrConfig := ctrConfigs[container]
 	ctrConfig.BestEffortQOS = true
-	ctrConfig.Resources = nil
 	ctrConfigs[container] = ctrConfig
 	return ctrConfigs
 }
diff --git a/internal/appliance/config/dev_mode_test.go b/internal/appliance/config/dev_mode_test.go
@@ -4,19 +4,21 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 )
 
 func TestSetLocalDevMode_CreatesContainerConfigWhereNoneExistedBefore(t *testing.T) {
-	sg := Sourcegraph{}
+	sg := &Sourcegraph{}
 
-	SetLocalDevMode(&sg)
+	sg.SetLocalDevMode()
 	assert.Equal(t, sg.Spec.Blobstore.ContainerConfig, map[string]ContainerConfig{
 		"blobstore": {BestEffortQOS: true},
 	})
 }
 
 func TestSetLocalDevMode_PreservesContainerConfigOtherThanBestEffortQOS(t *testing.T) {
-	sg := Sourcegraph{
+	sg := &Sourcegraph{
 		Spec: SourcegraphSpec{
 			Blobstore: BlobstoreSpec{
 				StandardConfig: StandardConfig{
@@ -25,20 +27,54 @@ func TestSetLocalDevMode_PreservesContainerConfigOtherThanBestEffortQOS(t *testi
 							EnvVars: map[string]string{
 								"foo": "bar",
 							},
+							Resources: &corev1.ResourceRequirements{
+								Requests: corev1.ResourceList{
+									corev1.ResourceCPU:              resource.MustParse("2"),
+									corev1.ResourceMemory:           resource.MustParse("2G"),
+									corev1.ResourceEphemeralStorage: resource.MustParse("4Gi"),
+								},
+								Limits: corev1.ResourceList{
+									corev1.ResourceCPU:              resource.MustParse("2"),
+									corev1.ResourceMemory:           resource.MustParse("4G"),
+									corev1.ResourceEphemeralStorage: resource.MustParse("8Gi"),
+								},
+							},
 						},
 					},
 				},
 			},
 		},
 	}
 
-	SetLocalDevMode(&sg)
+	sg.SetLocalDevMode()
 	assert.Equal(t, sg.Spec.Blobstore.ContainerConfig, map[string]ContainerConfig{
 		"blobstore": {
 			BestEffortQOS: true,
 			EnvVars: map[string]string{
 				"foo": "bar",
 			},
+
+			// We really do want to preserve the resources block, if its already
+			// set, for one reason: in case the admin has overriden
+			// ephemeral-resources. Dev mode only affects CPU and memory, but
+			// this is implemented in
+			// internal/k8s/resource/container/container.go, not here. We set
+			// 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.
+			Resources: &corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU:              resource.MustParse("2"),
+					corev1.ResourceMemory:           resource.MustParse("2G"),
+					corev1.ResourceEphemeralStorage: resource.MustParse("4Gi"),
+				},
+				Limits: corev1.ResourceList{
+					corev1.ResourceCPU:              resource.MustParse("2"),
+					corev1.ResourceMemory:           resource.MustParse("4G"),
+					corev1.ResourceEphemeralStorage: resource.MustParse("8Gi"),
+				},
+			},
 		},
 	})
 }
diff --git a/internal/appliance/config/spec.go b/internal/appliance/config/spec.go
@@ -1,7 +1,6 @@
 package config
 
 import (
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -100,11 +99,6 @@ type PGSQLSpec struct {
 	DatabaseConnection *DatabaseConnectionSpec `json:"database,omitempty"`
 }
 
-type PostgresExporterSpec struct {
-	// Resources allows for custom resource limits and requests.
-	Resources *corev1.ResourceList `json:"resources,omitempty"`
-}
-
 type PreciseCodeIntelSpec struct {
 	StandardConfig
 
@@ -232,9 +226,6 @@ type SourcegraphSpec struct {
 	// PGSQL defines the desired state of the PostgreSQL database.
 	PGSQL PGSQLSpec `json:"pgsql,omitempty"`
 
-	// PostgresExporter defines the desired state of the Postgres exporter service.
-	PostgresExporter PostgresExporterSpec `json:"postgresExporter,omitempty"`
-
 	// PreciseCodeIntel defines the desired state of the Precise Code Intel service.
 	PreciseCodeIntel PreciseCodeIntelSpec `json:"preciseCodeIntel,omitempty"`
 
diff --git a/internal/appliance/dev/integrationtest/integration_test.go b/internal/appliance/dev/integrationtest/integration_test.go
@@ -66,10 +66,10 @@ func srcValidate(t *testing.T, namespace string) {
 }
 
 func createConfigMap(t *testing.T, namespace string, k8sClient *kubernetes.Clientset, cfg config.SourcegraphSpec) {
-	sg := config.Sourcegraph{
+	sg := &config.Sourcegraph{
 		Spec: cfg,
 	}
-	config.SetLocalDevMode(&sg)
+	sg.SetLocalDevMode()
 	cfgBytes, err := yaml.Marshal(sg)
 	require.NoError(t, err)
 	cfgMap := configmap.NewConfigMap("sg", namespace)
diff --git a/internal/appliance/html.go b/internal/appliance/html.go
@@ -16,6 +16,7 @@ import (
 
 const (
 	templateSetup = "web/template/setup.gohtml"
+	formValueOn   = "on"
 )
 
 var (
@@ -84,7 +85,7 @@ func (a *Appliance) postSetupHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	a.sourcegraph.Spec.RequestedVersion = r.FormValue("version")
-	if r.FormValue("external_database") == "yes" {
+	if r.FormValue("external_database") == formValueOn {
 		a.sourcegraph.Spec.PGSQL.DatabaseConnection = &config.DatabaseConnectionSpec{
 			Host:     r.FormValue("pgsqlDBHost"),
 			Port:     r.FormValue("pgsqlDBPort"),
@@ -109,6 +110,10 @@ func (a *Appliance) postSetupHandler(w http.ResponseWriter, r *http.Request) {
 	}
 	// TODO validate user input
 
+	if r.FormValue("dev_mode") == formValueOn {
+		a.sourcegraph.SetLocalDevMode()
+	}
+
 	_, err = a.CreateConfigMap(r.Context(), "sourcegraph-appliance")
 	if err != nil {
 		a.logger.Error("failed to create configMap sourcegraph-appliance", log.Error(err))
diff --git a/internal/appliance/reconciler/codeintel.go b/internal/appliance/reconciler/codeintel.go
@@ -227,6 +227,6 @@ func (r *Reconciler) reconcileCodeIntelService(ctx context.Context, sg *config.S
 
 func (r *Reconciler) reconcileCodeIntelServiceAccount(ctx context.Context, sg *config.Sourcegraph, owner client.Object) error {
 	cfg := sg.Spec.CodeIntel
-	sa := serviceaccount.NewServiceAccount("codeintel", sg.Namespace, cfg)
+	sa := serviceaccount.NewServiceAccount("codeintel-db", sg.Namespace, cfg)
 	return reconcileObject(ctx, r, sg.Spec.CodeIntel, &sa, &corev1.ServiceAccount{}, sg, owner)
 }
diff --git a/internal/appliance/reconciler/gitserver.go b/internal/appliance/reconciler/gitserver.go
@@ -67,8 +67,6 @@ func (r *Reconciler) reconcileGitServerStatefulSet(ctx context.Context, sg *conf
 				Port: intstr.FromString("rpc"),
 			},
 		},
-		TimeoutSeconds:      5,
-		InitialDelaySeconds: 5,
 	}
 
 	ctr.VolumeMounts = []corev1.VolumeMount{
diff --git a/internal/appliance/reconciler/indexed_search.go b/internal/appliance/reconciler/indexed_search.go
@@ -40,7 +40,7 @@ func (r *Reconciler) reconcileIndexedSearcherStatefulSet(ctx context.Context, sg
 	cfg := sg.Spec.IndexedSearch
 
 	webServer := container.NewContainer("zoekt-webserver", cfg, config.ContainerConfig{
-		Image: config.GetDefaultImage(sg, name),
+		Image: config.GetDefaultImage(sg, "indexed-searcher"),
 		Resources: &corev1.ResourceRequirements{
 			Requests: corev1.ResourceList{
 				corev1.ResourceCPU:    resource.MustParse("500m"),
diff --git a/internal/appliance/reconciler/standard_config_test.go b/internal/appliance/reconciler/standard_config_test.go
@@ -8,6 +8,7 @@ func (suite *ApplianceTestSuite) TestStandardFeatures() {
 		name string
 	}{
 		{name: "standard/blobstore-with-named-storage-class"},
+		{name: "standard/frontend-with-no-cpu-memory-resources"},
 		{name: "standard/precise-code-intel-with-env-vars"},
 		{name: "standard/redis-with-multiple-custom-images"},
 		{name: "standard/redis-with-storage"},
diff --git a/internal/appliance/reconciler/testdata/golden-fixtures/codeintel/default.yaml b/internal/appliance/reconciler/testdata/golden-fixtures/codeintel/default.yaml
@@ -435,7 +435,7 @@ resources:
       creationTimestamp: "2024-04-19T00:00:00Z"
       labels:
         deploy: sourcegraph
-      name: codeintel
+      name: codeintel-db
       namespace: NORMALIZED_FOR_TESTING
       ownerReferences:
         - apiVersion: v1
diff --git a/internal/appliance/reconciler/testdata/golden-fixtures/gitserver/default.yaml b/internal/appliance/reconciler/testdata/golden-fixtures/gitserver/default.yaml
@@ -69,12 +69,11 @@ resources:
               imagePullPolicy: IfNotPresent
               livenessProbe:
                 failureThreshold: 3
-                initialDelaySeconds: 5
                 periodSeconds: 10
                 successThreshold: 1
                 tcpSocket:
                   port: rpc
-                timeoutSeconds: 5
+                timeoutSeconds: 1
               name: gitserver
               ports:
                 - containerPort: 3178
diff --git a/internal/appliance/reconciler/testdata/golden-fixtures/indexed-search/default.yaml b/internal/appliance/reconciler/testdata/golden-fixtures/indexed-search/default.yaml
@@ -55,7 +55,7 @@ resources:
                   value: http://$(OTEL_AGENT_HOST):4317
                 - name: OPENTELEMETRY_DISABLED
                   value: "false"
-              image: index.docker.io/sourcegraph/indexed-search:5.3.9104
+              image: index.docker.io/sourcegraph/indexed-searcher:5.3.9104
               imagePullPolicy: IfNotPresent
               name: zoekt-webserver
               ports:
diff --git a/internal/appliance/reconciler/testdata/golden-fixtures/indexed-search/with-replicas.yaml b/internal/appliance/reconciler/testdata/golden-fixtures/indexed-search/with-replicas.yaml
@@ -55,7 +55,7 @@ resources:
                   value: http://$(OTEL_AGENT_HOST):4317
                 - name: OPENTELEMETRY_DISABLED
                   value: "false"
-              image: index.docker.io/sourcegraph/indexed-search:5.3.9104
+              image: index.docker.io/sourcegraph/indexed-searcher:5.3.9104
               imagePullPolicy: IfNotPresent
               name: zoekt-webserver
               ports:
diff --git a/internal/appliance/reconciler/testdata/golden-fixtures/standard/frontend-with-no-cpu-memory-resources.yaml b/internal/appliance/reconciler/testdata/golden-fixtures/standard/frontend-with-no-cpu-memory-resources.yaml
diff --git a/internal/appliance/reconciler/testdata/sg/standard/frontend-with-no-cpu-memory-resources.yaml b/internal/appliance/reconciler/testdata/sg/standard/frontend-with-no-cpu-memory-resources.yaml
diff --git a/internal/appliance/web/template/setup.gohtml b/internal/appliance/web/template/setup.gohtml
diff --git a/internal/k8s/resource/container/container.go b/internal/k8s/resource/container/container.go

Original file line number	Diff line number	Diff line change
`@@ -66,10 +66,10 @@ func srcValidate(t *testing.T, namespace string) {`
`66`	`66`	`}`
`67`	`67`
`68`	`68`	`func createConfigMap(t testing.T, namespace string, k8sClient kubernetes.Clientset, cfg config.SourcegraphSpec) {`
`69`		`- sg := config.Sourcegraph{`
	`69`	`+ sg := &config.Sourcegraph{`
`70`	`70`	`Spec: cfg,`
`71`	`71`	`}`
`72`		`- config.SetLocalDevMode(&sg)`
	`72`	`+ sg.SetLocalDevMode()`
`73`	`73`	`cfgBytes, err := yaml.Marshal(sg)`
`74`	`74`	`require.NoError(t, err)`
`75`	`75`	`cfgMap := configmap.NewConfigMap("sg", namespace)`
Original file line number	Diff line number	Diff line change
`@@ -227,6 +227,6 @@ func (r Reconciler) reconcileCodeIntelService(ctx context.Context, sg config.S`
`227`	`227`
`228`	`228`	`func (r Reconciler) reconcileCodeIntelServiceAccount(ctx context.Context, sg config.Sourcegraph, owner client.Object) error {`
`229`	`229`	`cfg := sg.Spec.CodeIntel`
`230`		`- sa := serviceaccount.NewServiceAccount("codeintel", sg.Namespace, cfg)`
	`230`	`+ sa := serviceaccount.NewServiceAccount("codeintel-db", sg.Namespace, cfg)`
`231`	`231`	`return reconcileObject(ctx, r, sg.Spec.CodeIntel, &sa, &corev1.ServiceAccount{}, sg, owner)`
`232`	`232`	`}`
Original file line number	Diff line number	Diff line change
`@@ -67,8 +67,6 @@ func (r Reconciler) reconcileGitServerStatefulSet(ctx context.Context, sg conf`
`67`	`67`	`Port: intstr.FromString("rpc"),`
`68`	`68`	`},`
`69`	`69`	`},`
`70`		`- TimeoutSeconds: 5,`
`71`		`- InitialDelaySeconds: 5,`
`72`	`70`	`}`
`73`	`71`
`74`	`72`	`ctr.VolumeMounts = []corev1.VolumeMount{`