From 02d9add3228e737fe721bc37d1f693f7acf53d68 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Thu, 30 Mar 2023 15:40:28 +0200 Subject: [PATCH] Fix bug preventing env variables from being specified (#2450) Co-authored-by: Tingluo Huang --- .../templates/deployment.yaml | 9 +-- .../tests/template_test.go | 56 ++++++++++++++++++- .../values.yaml | 51 ++++++++++------- 3 files changed, 88 insertions(+), 28 deletions(-) diff --git a/charts/gha-runner-scale-set-controller/templates/deployment.yaml b/charts/gha-runner-scale-set-controller/templates/deployment.yaml index b239040a..b624d963 100644 --- a/charts/gha-runner-scale-set-controller/templates/deployment.yaml +++ b/charts/gha-runner-scale-set-controller/templates/deployment.yaml @@ -69,13 +69,8 @@ spec: fieldRef: fieldPath: metadata.namespace {{- with .Values.env }} - {{- if kindIs "slice" .Values.env }} - {{- toYaml .Values.env | nindent 8 }} - {{- else }} - {{- range $key, $val := .Values.env }} - - name: {{ $key }} - value: {{ $val | quote }} - {{- end }} + {{- if kindIs "slice" . }} + {{- toYaml . | nindent 8 }} {{- end }} {{- end }} {{- with .Values.resources }} diff --git a/charts/gha-runner-scale-set-controller/tests/template_test.go b/charts/gha-runner-scale-set-controller/tests/template_test.go index 96b671eb..3ee12f7d 100644 --- a/charts/gha-runner-scale-set-controller/tests/template_test.go +++ b/charts/gha-runner-scale-set-controller/tests/template_test.go @@ -390,6 +390,8 @@ func TestTemplate_ControllerDeployment_Customize(t *testing.T) { "imagePullSecrets[0].name": "dockerhub", "nameOverride": "gha-runner-scale-set-controller-override", "fullnameOverride": "gha-runner-scale-set-controller-fullname-override", + "env[0].name": "ENV_VAR_NAME_1", + "env[0].value": "ENV_VAR_VALUE_1", "serviceAccount.name": "gha-runner-scale-set-controller-sa", "podAnnotations.foo": "bar", "podSecurityContext.fsGroup": "1000", @@ -432,6 +434,9 @@ func TestTemplate_ControllerDeployment_Customize(t *testing.T) { assert.Equal(t, "bar", deployment.Spec.Template.Annotations["foo"]) assert.Equal(t, "manager", deployment.Spec.Template.Annotations["kubectl.kubernetes.io/default-container"]) + assert.Equal(t, "ENV_VAR_NAME_1", deployment.Spec.Template.Spec.Containers[0].Env[2].Name) + assert.Equal(t, "ENV_VAR_VALUE_1", deployment.Spec.Template.Spec.Containers[0].Env[2].Value) + assert.Len(t, deployment.Spec.Template.Spec.ImagePullSecrets, 1) assert.Equal(t, "dockerhub", deployment.Spec.Template.Spec.ImagePullSecrets[0].Name) assert.Equal(t, "gha-runner-scale-set-controller-sa", deployment.Spec.Template.Spec.ServiceAccountName) @@ -467,10 +472,13 @@ func TestTemplate_ControllerDeployment_Customize(t *testing.T) { assert.Equal(t, "--auto-scaler-image-pull-secrets=dockerhub", deployment.Spec.Template.Spec.Containers[0].Args[1]) assert.Equal(t, "--log-level=debug", deployment.Spec.Template.Spec.Containers[0].Args[2]) - assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 2) + assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 3) assert.Equal(t, "CONTROLLER_MANAGER_CONTAINER_IMAGE", deployment.Spec.Template.Spec.Containers[0].Env[0].Name) assert.Equal(t, managerImage, deployment.Spec.Template.Spec.Containers[0].Env[0].Value) + assert.Equal(t, "ENV_VAR_NAME_1", deployment.Spec.Template.Spec.Containers[0].Env[2].Name) + assert.Equal(t, "ENV_VAR_VALUE_1", deployment.Spec.Template.Spec.Containers[0].Env[2].Value) + assert.Equal(t, "CONTROLLER_MANAGER_POD_NAMESPACE", deployment.Spec.Template.Spec.Containers[0].Env[1].Name) assert.Equal(t, "metadata.namespace", deployment.Spec.Template.Spec.Containers[0].Env[1].ValueFrom.FieldRef.FieldPath) @@ -704,6 +712,52 @@ func TestTemplate_ControllerDeployment_WatchSingleNamespace(t *testing.T) { assert.Equal(t, "/tmp", deployment.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath) } +func TestTemplate_ControllerContainerEnvironmentVariables(t *testing.T) { + t.Parallel() + + // Path to the helm chart we will test + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set-controller") + require.NoError(t, err) + + releaseName := "test-arc" + namespaceName := "test-" + strings.ToLower(random.UniqueId()) + + options := &helm.Options{ + SetValues: map[string]string{ + "env[0].Name": "ENV_VAR_NAME_1", + "env[0].Value": "ENV_VAR_VALUE_1", + "env[1].Name": "ENV_VAR_NAME_2", + "env[1].ValueFrom.SecretKeyRef.Key": "ENV_VAR_NAME_2", + "env[1].ValueFrom.SecretKeyRef.Name": "secret-name", + "env[1].ValueFrom.SecretKeyRef.Optional": "true", + "env[2].Name": "ENV_VAR_NAME_3", + "env[2].Value": "", + "env[3].Name": "ENV_VAR_NAME_4", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + output := helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/deployment.yaml"}) + + var deployment appsv1.Deployment + helm.UnmarshalK8SYaml(t, output, &deployment) + + assert.Equal(t, namespaceName, deployment.Namespace) + assert.Equal(t, "test-arc-gha-runner-scale-set-controller", deployment.Name) + + assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 6) + assert.Equal(t, "ENV_VAR_NAME_1", deployment.Spec.Template.Spec.Containers[0].Env[2].Name) + assert.Equal(t, "ENV_VAR_VALUE_1", deployment.Spec.Template.Spec.Containers[0].Env[2].Value) + assert.Equal(t, "ENV_VAR_NAME_2", deployment.Spec.Template.Spec.Containers[0].Env[3].Name) + assert.Equal(t, "secret-name", deployment.Spec.Template.Spec.Containers[0].Env[3].ValueFrom.SecretKeyRef.Name) + assert.Equal(t, "ENV_VAR_NAME_2", deployment.Spec.Template.Spec.Containers[0].Env[3].ValueFrom.SecretKeyRef.Key) + assert.True(t, *deployment.Spec.Template.Spec.Containers[0].Env[3].ValueFrom.SecretKeyRef.Optional) + assert.Equal(t, "ENV_VAR_NAME_3", deployment.Spec.Template.Spec.Containers[0].Env[4].Name) + assert.Empty(t, deployment.Spec.Template.Spec.Containers[0].Env[4].Value) + assert.Equal(t, "ENV_VAR_NAME_4", deployment.Spec.Template.Spec.Containers[0].Env[5].Name) + assert.Empty(t, deployment.Spec.Template.Spec.Containers[0].Env[5].ValueFrom) +} + func TestTemplate_WatchSingleNamespace_NotCreateManagerClusterRole(t *testing.T) { t.Parallel() diff --git a/charts/gha-runner-scale-set-controller/values.yaml b/charts/gha-runner-scale-set-controller/values.yaml index 055d68e9..03359b5f 100644 --- a/charts/gha-runner-scale-set-controller/values.yaml +++ b/charts/gha-runner-scale-set-controller/values.yaml @@ -18,6 +18,17 @@ imagePullSecrets: [] nameOverride: "" fullnameOverride: "" +env: +## Define environment variables for the controller pod +# - name: "ENV_VAR_NAME_1" +# value: "ENV_VAR_VALUE_1" +# - name: "ENV_VAR_NAME_2" +# valueFrom: +# secretKeyRef: +# key: ENV_VAR_NAME_2 +# name: secret-name +# optional: true + serviceAccount: # Specifies whether a service account should be created for running the controller pod create: true @@ -31,27 +42,27 @@ serviceAccount: podAnnotations: {} podSecurityContext: {} - # fsGroup: 2000 +# fsGroup: 2000 securityContext: {} - # capabilities: - # drop: - # - ALL - # readOnlyRootFilesystem: true - # runAsNonRoot: true - # runAsUser: 1000 +# capabilities: +# drop: +# - ALL +# readOnlyRootFilesystem: true +# runAsNonRoot: true +# runAsUser: 1000 resources: {} - # We usually recommend not to specify default resources and to leave this as a conscious - # choice for the user. This also increases chances charts run on environments with little - # resources, such as Minikube. If you do want to specify resources, uncomment the following - # lines, adjust them as necessary, and remove the curly braces after 'resources:'. - # limits: - # cpu: 100m - # memory: 128Mi - # requests: - # cpu: 100m - # memory: 128Mi +## We usually recommend not to specify default resources and to leave this as a conscious +## choice for the user. This also increases chances charts run on environments with little +## resources, such as Minikube. If you do want to specify resources, uncomment the following +## lines, adjust them as necessary, and remove the curly braces after 'resources:'. +# limits: +# cpu: 100m +# memory: 128Mi +# requests: +# cpu: 100m +# memory: 128Mi nodeSelector: {} @@ -69,6 +80,6 @@ flags: # Defaults to "debug". logLevel: "debug" - # Restricts the controller to only watch resources in the desired namespace. - # Defaults to watch all namespaces when unset. - # watchSingleNamespace: "" \ No newline at end of file + ## Restricts the controller to only watch resources in the desired namespace. + ## Defaults to watch all namespaces when unset. + # watchSingleNamespace: ""