From 6fe80086405199af73b20abf1234e10974986823 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Wed, 5 Jul 2023 21:06:42 +0200 Subject: [PATCH] Add configurable log format to values.yaml and propagate it to listener (#2686) --- .../templates/deployment.yaml | 3 + .../tests/template_test.go | 72 ++++++++++++------- .../values.yaml | 15 ++-- cmd/githubrunnerscalesetlistener/main.go | 24 +++++-- controllers/actions.github.com/constants.go | 11 ++- .../actions.github.com/resourcebuilder.go | 30 ++++++++ main.go | 8 ++- 7 files changed, 124 insertions(+), 39 deletions(-) diff --git a/charts/gha-runner-scale-set-controller/templates/deployment.yaml b/charts/gha-runner-scale-set-controller/templates/deployment.yaml index dc0b88a7..41034df8 100644 --- a/charts/gha-runner-scale-set-controller/templates/deployment.yaml +++ b/charts/gha-runner-scale-set-controller/templates/deployment.yaml @@ -56,6 +56,9 @@ spec: {{- with .Values.flags.logLevel }} - "--log-level={{ . }}" {{- end }} + {{- with .Values.flags.logFormat }} + - "--log-format={{ . }}" + {{- end }} {{- with .Values.flags.watchSingleNamespace }} - "--watch-single-namespace={{ . }}" {{- end }} 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 73c61699..f7107fbb 100644 --- a/charts/gha-runner-scale-set-controller/tests/template_test.go +++ b/charts/gha-runner-scale-set-controller/tests/template_test.go @@ -244,6 +244,7 @@ func TestTemplate_CreateManagerListenerRole(t *testing.T) { assert.Equal(t, namespaceName, managerListenerRole.Namespace, "Role should have a namespace") assert.Equal(t, "test-arc-gha-runner-scale-set-controller-manager-listener-role", managerListenerRole.Name) + assert.Equal(t, 4, len(managerListenerRole.Rules)) assert.Equal(t, "pods", managerListenerRole.Rules[0].Resources[0]) assert.Equal(t, "pods/status", managerListenerRole.Rules[1].Resources[0]) @@ -356,10 +357,13 @@ func TestTemplate_ControllerDeployment_Defaults(t *testing.T) { assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Command, 1) assert.Equal(t, "/manager", deployment.Spec.Template.Spec.Containers[0].Command[0]) - assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Args, 3) - assert.Equal(t, "--auto-scaling-runner-set-only", deployment.Spec.Template.Spec.Containers[0].Args[0]) - assert.Equal(t, "--log-level=debug", deployment.Spec.Template.Spec.Containers[0].Args[1]) - assert.Equal(t, "--update-strategy=immediate", deployment.Spec.Template.Spec.Containers[0].Args[2]) + expectedArgs := []string{ + "--auto-scaling-runner-set-only", + "--log-level=debug", + "--log-format=text", + "--update-strategy=immediate", + } + assert.ElementsMatch(t, expectedArgs, deployment.Spec.Template.Spec.Containers[0].Args) 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) @@ -420,6 +424,8 @@ func TestTemplate_ControllerDeployment_Customize(t *testing.T) { "affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].operator": "bar", "priorityClassName": "test-priority-class", "flags.updateStrategy": "eventual", + "flags.logLevel": "info", + "flags.logFormat": "json", }, KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), } @@ -484,11 +490,15 @@ func TestTemplate_ControllerDeployment_Customize(t *testing.T) { assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Command, 1) assert.Equal(t, "/manager", deployment.Spec.Template.Spec.Containers[0].Command[0]) - assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Args, 4) - assert.Equal(t, "--auto-scaling-runner-set-only", deployment.Spec.Template.Spec.Containers[0].Args[0]) - 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.Equal(t, "--update-strategy=eventual", deployment.Spec.Template.Spec.Containers[0].Args[3]) + expectArgs := []string{ + "--auto-scaling-runner-set-only", + "--auto-scaler-image-pull-secrets=dockerhub", + "--log-level=info", + "--log-format=json", + "--update-strategy=eventual", + } + + assert.ElementsMatch(t, expectArgs, deployment.Spec.Template.Spec.Containers[0].Args) assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 4) assert.Equal(t, "CONTROLLER_MANAGER_CONTAINER_IMAGE", deployment.Spec.Template.Spec.Containers[0].Env[0].Name) @@ -605,12 +615,16 @@ func TestTemplate_EnableLeaderElection(t *testing.T) { assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Command, 1) assert.Equal(t, "/manager", deployment.Spec.Template.Spec.Containers[0].Command[0]) - assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Args, 5) - assert.Equal(t, "--auto-scaling-runner-set-only", deployment.Spec.Template.Spec.Containers[0].Args[0]) - assert.Equal(t, "--enable-leader-election", deployment.Spec.Template.Spec.Containers[0].Args[1]) - assert.Equal(t, "--leader-election-id=test-arc-gha-runner-scale-set-controller", deployment.Spec.Template.Spec.Containers[0].Args[2]) - assert.Equal(t, "--log-level=debug", deployment.Spec.Template.Spec.Containers[0].Args[3]) - assert.Equal(t, "--update-strategy=immediate", deployment.Spec.Template.Spec.Containers[0].Args[4]) + expectedArgs := []string{ + "--auto-scaling-runner-set-only", + "--enable-leader-election", + "--leader-election-id=test-arc-gha-runner-scale-set-controller", + "--log-level=debug", + "--log-format=text", + "--update-strategy=immediate", + } + + assert.ElementsMatch(t, expectedArgs, deployment.Spec.Template.Spec.Containers[0].Args) } func TestTemplate_ControllerDeployment_ForwardImagePullSecrets(t *testing.T) { @@ -639,11 +653,15 @@ func TestTemplate_ControllerDeployment_ForwardImagePullSecrets(t *testing.T) { assert.Equal(t, namespaceName, deployment.Namespace) - assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Args, 4) - assert.Equal(t, "--auto-scaling-runner-set-only", deployment.Spec.Template.Spec.Containers[0].Args[0]) - assert.Equal(t, "--auto-scaler-image-pull-secrets=dockerhub,ghcr", deployment.Spec.Template.Spec.Containers[0].Args[1]) - assert.Equal(t, "--log-level=debug", deployment.Spec.Template.Spec.Containers[0].Args[2]) - assert.Equal(t, "--update-strategy=immediate", deployment.Spec.Template.Spec.Containers[0].Args[3]) + expectedArgs := []string{ + "--auto-scaling-runner-set-only", + "--auto-scaler-image-pull-secrets=dockerhub,ghcr", + "--log-level=debug", + "--log-format=text", + "--update-strategy=immediate", + } + + assert.ElementsMatch(t, expectedArgs, deployment.Spec.Template.Spec.Containers[0].Args) } func TestTemplate_ControllerDeployment_WatchSingleNamespace(t *testing.T) { @@ -721,11 +739,15 @@ func TestTemplate_ControllerDeployment_WatchSingleNamespace(t *testing.T) { assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Command, 1) assert.Equal(t, "/manager", deployment.Spec.Template.Spec.Containers[0].Command[0]) - assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Args, 4) - assert.Equal(t, "--auto-scaling-runner-set-only", deployment.Spec.Template.Spec.Containers[0].Args[0]) - assert.Equal(t, "--log-level=debug", deployment.Spec.Template.Spec.Containers[0].Args[1]) - assert.Equal(t, "--watch-single-namespace=demo", deployment.Spec.Template.Spec.Containers[0].Args[2]) - assert.Equal(t, "--update-strategy=immediate", deployment.Spec.Template.Spec.Containers[0].Args[3]) + expectedArgs := []string{ + "--auto-scaling-runner-set-only", + "--log-level=debug", + "--log-format=text", + "--watch-single-namespace=demo", + "--update-strategy=immediate", + } + + assert.ElementsMatch(t, expectedArgs, deployment.Spec.Template.Spec.Containers[0].Args) 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) diff --git a/charts/gha-runner-scale-set-controller/values.yaml b/charts/gha-runner-scale-set-controller/values.yaml index b0405609..1da0aac6 100644 --- a/charts/gha-runner-scale-set-controller/values.yaml +++ b/charts/gha-runner-scale-set-controller/values.yaml @@ -79,6 +79,9 @@ flags: ## Log level can be set here with one of the following values: "debug", "info", "warn", "error". ## Defaults to "debug". logLevel: "debug" + ## Log format can be set with one of the following values: "text", "json" + ## Defaults to "text" + logFormat: "text" ## Restricts the controller to only watch resources in the desired namespace. ## Defaults to watch all namespaces when unset. @@ -88,14 +91,14 @@ flags: ## ## The srategies available are: ## - "immediate": (default) The controller will immediately apply the change causing the - ## recreation of the listener and ephemeral runner set. This can lead to an + ## recreation of the listener and ephemeral runner set. This can lead to an ## overprovisioning of runners, if there are pending / running jobs. This should not - ## be a problem at a small scale, but it could lead to a significant increase of + ## be a problem at a small scale, but it could lead to a significant increase of ## resources if you have a lot of jobs running concurrently. - ## - ## - "eventual": The controller will remove the listener and ephemeral runner set - ## immediately, but will not recreate them (to apply changes) until all + ## + ## - "eventual": The controller will remove the listener and ephemeral runner set + ## immediately, but will not recreate them (to apply changes) until all ## pending / running jobs have completed. ## This can lead to a longer time to apply the change but it will ensure ## that you don't have any overprovisioning of runners. - updateStrategy: "immediate" \ No newline at end of file + updateStrategy: "immediate" diff --git a/cmd/githubrunnerscalesetlistener/main.go b/cmd/githubrunnerscalesetlistener/main.go index 64abf6cf..deff9a68 100644 --- a/cmd/githubrunnerscalesetlistener/main.go +++ b/cmd/githubrunnerscalesetlistener/main.go @@ -46,18 +46,30 @@ type RunnerScaleSetListenerConfig struct { MinRunners int `split_words:"true"` RunnerScaleSetId int `split_words:"true"` ServerRootCA string `split_words:"true"` + LogLevel string `split_words:"true"` + LogFormat string `split_words:"true"` } func main() { - logger, err := logging.NewLogger(logging.LogLevelDebug, logging.LogFormatText) - if err != nil { - fmt.Fprintf(os.Stderr, "Error: creating logger: %v\n", err) + var rc RunnerScaleSetListenerConfig + if err := envconfig.Process("github", &rc); err != nil { + fmt.Fprintf(os.Stderr, "Error: processing environment variables for RunnerScaleSetListenerConfig: %v\n", err) os.Exit(1) } - var rc RunnerScaleSetListenerConfig - if err := envconfig.Process("github", &rc); err != nil { - logger.Error(err, "Error: processing environment variables for RunnerScaleSetListenerConfig") + logLevel := string(logging.LogLevelDebug) + if rc.LogLevel != "" { + logLevel = rc.LogLevel + } + + logFormat := string(logging.LogFormatText) + if rc.LogFormat != "" { + logFormat = rc.LogFormat + } + + logger, err := logging.NewLogger(logLevel, logFormat) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: creating logger: %v\n", err) os.Exit(1) } diff --git a/controllers/actions.github.com/constants.go b/controllers/actions.github.com/constants.go index 339c39d9..5e8445ce 100644 --- a/controllers/actions.github.com/constants.go +++ b/controllers/actions.github.com/constants.go @@ -1,6 +1,9 @@ package actionsgithubcom -import corev1 "k8s.io/api/core/v1" +import ( + "github.com/actions/actions-runner-controller/logging" + corev1 "k8s.io/api/core/v1" +) const ( LabelKeyRunnerTemplateHash = "runner-template-hash" @@ -60,5 +63,11 @@ const ( // to the listener when ImagePullPolicy is not specified const DefaultScaleSetListenerImagePullPolicy = corev1.PullIfNotPresent +// DefaultScaleSetListenerLogLevel is the default log level applied +const DefaultScaleSetListenerLogLevel = string(logging.LogLevelDebug) + +// DefaultScaleSetListenerLogFormat is the default log format applied +const DefaultScaleSetListenerLogFormat = string(logging.LogFormatText) + // ownerKey is field selector matching the owner name of a particular resource const resourceOwnerKey = ".metadata.controller" diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index 5be1163b..d9369bd5 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -10,6 +10,7 @@ import ( "github.com/actions/actions-runner-controller/build" "github.com/actions/actions-runner-controller/github/actions" "github.com/actions/actions-runner-controller/hash" + "github.com/actions/actions-runner-controller/logging" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,6 +47,27 @@ func SetListenerImagePullPolicy(pullPolicy string) bool { } } +var scaleSetListenerLogLevel = DefaultScaleSetListenerLogLevel +var scaleSetListenerLogFormat = DefaultScaleSetListenerLogFormat + +func SetListenerLoggingParameters(level string, format string) bool { + switch level { + case logging.LogLevelDebug, logging.LogLevelInfo, logging.LogLevelWarn, logging.LogLevelError: + default: + return false + } + + switch format { + case logging.LogFormatJSON, logging.LogFormatText: + default: + return false + } + + scaleSetListenerLogLevel = level + scaleSetListenerLogFormat = format + return true +} + type resourceBuilder struct{} func (b *resourceBuilder) newAutoScalingListener(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, namespace, image string, imagePullSecrets []corev1.LocalObjectReference) (*v1alpha1.AutoscalingListener, error) { @@ -128,6 +150,14 @@ func (b *resourceBuilder) newScaleSetListenerPod(autoscalingListener *v1alpha1.A Name: "GITHUB_RUNNER_SCALE_SET_ID", Value: strconv.Itoa(autoscalingListener.Spec.RunnerScaleSetId), }, + { + Name: "GITHUB_RUNNER_LOG_LEVEL", + Value: scaleSetListenerLogLevel, + }, + { + Name: "GITHUB_RUNNER_LOG_FORMAT", + Value: scaleSetListenerLogFormat, + }, } listenerEnv = append(listenerEnv, envs...) diff --git a/main.go b/main.go index 9cc1bbdd..689a71b3 100644 --- a/main.go +++ b/main.go @@ -182,12 +182,18 @@ func main() { } listenerPullPolicy := os.Getenv("CONTROLLER_MANAGER_LISTENER_IMAGE_PULL_POLICY") - if ok := actionsgithubcom.SetListenerImagePullPolicy(listenerPullPolicy); ok { + if actionsgithubcom.SetListenerImagePullPolicy(listenerPullPolicy) { log.Info("AutoscalingListener image pull policy changed", "ImagePullPolicy", listenerPullPolicy) } else { log.Info("Using default AutoscalingListener image pull policy", "ImagePullPolicy", actionsgithubcom.DefaultScaleSetListenerImagePullPolicy) } + if actionsgithubcom.SetListenerLoggingParameters(logLevel, logFormat) { + log.Info("AutoscalingListener logging parameters changed", "LogLevel", logLevel, "LogFormat", logFormat) + } else { + log.Info("Using default AutoscalingListener logging parameters", "LogLevel", actionsgithubcom.DefaultScaleSetListenerLogLevel, "LogFormat", actionsgithubcom.DefaultScaleSetListenerLogFormat) + } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, NewCache: newCache,