actions-metrics: Do our best not to fail the whole event processing on no API creds (#2459)

Co-authored-by: Tingluo Huang <tingluohuang@github.com>

.github/workflows/publish-chart.yaml

@@ -20,7 +20,7 @@ env:
   HELM_VERSION: v3.8.0
 
 permissions:
-  contents: read
+  contents: write
 
 jobs:
   lint-chart:
@@ -173,10 +173,28 @@ jobs:
             --pages-branch 'gh-pages' \
             --pages-index-path 'index.yaml'
 
+      # This step is required to not throw away changes made to the index.yaml on every new chart release.
+      #
+      # We update the index.yaml in the actions-runner-controller.github.io repo
+      # by appending the new chart version to the index.yaml saved in actions-runner-controller repo
+      # and copying and commiting the updated index.yaml to the github.io one.
+      # See below for more context:
+      # - https://github.com/actions-runner-controller/actions-runner-controller.github.io/pull/2
+      # - https://github.com/actions/actions-runner-controller/pull/2452
+      - name: Commit and push to actions/actions-runner-controller
+        run: |
+          git checkout gh-pages
+          git config user.name "$GITHUB_ACTOR"
+          git config user.email "$GITHUB_ACTOR@users.noreply.github.com"
+          git add .
+          git commit -m "Update index.yaml"
+          git push
+        working-directory: ${{ github.workspace }}
+
       # Chart Release was never intended to publish to a different repo
       # this workaround is intended to move the index.yaml to the target repo
       # where the github pages are hosted
-      - name: Checkout pages repository
+      - name: Checkout target repository
         uses: actions/checkout@v3
         with:
           repository: ${{ env.CHART_TARGET_ORG }}/${{ env.CHART_TARGET_REPO }}
@@ -188,7 +206,7 @@ jobs:
         run: |
           cp ${{ github.workspace }}/index.yaml ${{ env.CHART_TARGET_REPO }}/actions-runner-controller/index.yaml
       
-      - name: Commit and push
+      - name: Commit and push to target repository
         run: |
           git config user.name "$GITHUB_ACTOR"
           git config user.email "$GITHUB_ACTOR@users.noreply.github.com"

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 }}

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()
 

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: ""
+  ## Restricts the controller to only watch resources in the desired namespace.
+  ## Defaults to watch all namespaces when unset.
+  # watchSingleNamespace: ""

pkg/actionsmetrics/event_reader.go

@@ -136,12 +136,27 @@ func (reader *EventReader) ProcessWorkflowJobEvent(ctx context.Context, event in
 		// job_conclusion -> (neutral, success, skipped, cancelled, timed_out, action_required, failure)
 		githubWorkflowJobConclusionsTotal.With(extraLabel("job_conclusion", *e.WorkflowJob.Conclusion, labels)).Inc()
 
-		parseResult, err := reader.fetchAndParseWorkflowJobLogs(ctx, e)
-		if err != nil {
-			log.Error(err, "reading workflow job log")
-			return
-		} else {
-			log.Info("reading workflow_job logs", keysAndValues...)
+		var (
+			exitCode       = "na"
+			runTimeSeconds *float64
+		)
+
+		// We need to do our best not to fail the whole event processing
+		// when the user provided no GitHub API credentials.
+		// See https://github.com/actions/actions-runner-controller/issues/2424
+		if reader.GitHubClient != nil {
+			parseResult, err := reader.fetchAndParseWorkflowJobLogs(ctx, e)
+			if err != nil {
+				log.Error(err, "reading workflow job log")
+				return
+			}
+
+			exitCode = parseResult.ExitCode
+
+			s := parseResult.RunTime.Seconds()
+			runTimeSeconds = &s
+
+			log.WithValues(keysAndValues...).Info("reading workflow_job logs", "exit_code", exitCode)
 		}
 
 		if *e.WorkflowJob.Conclusion == "failure" {
@@ -167,18 +182,20 @@ func (reader *EventReader) ProcessWorkflowJobEvent(ctx context.Context, event in
 				}
 				if *conclusion == "timed_out" {
 					failedStep = fmt.Sprint(i)
-					parseResult.ExitCode = "timed_out"
+					exitCode = "timed_out"
 					break
 				}
 			}
 			githubWorkflowJobFailuresTotal.With(
 				extraLabel("failed_step", failedStep,
-					extraLabel("exit_code", parseResult.ExitCode, labels),
+					extraLabel("exit_code", exitCode, labels),
 				),
 			).Inc()
 		}
 
-		githubWorkflowJobRunDurationSeconds.With(extraLabel("job_conclusion", *e.WorkflowJob.Conclusion, labels)).Observe(parseResult.RunTime.Seconds())
+		if runTimeSeconds != nil {
+			githubWorkflowJobRunDurationSeconds.With(extraLabel("job_conclusion", *e.WorkflowJob.Conclusion, labels)).Observe(*runTimeSeconds)
+		}
 	}
 }