Remove inheritance of imagePullPolicy from manager to listeners (#3009)

.github/workflows/arc-publish-chart.yaml

@@ -63,7 +63,7 @@ jobs:
         python-version: '3.11'
 
     - name: Set up chart-testing
-      uses: helm/chart-testing-action@v2.3.1
+      uses: helm/chart-testing-action@v2.6
 
     - name: Run chart-testing (list-changed)
       id: list-changed

.github/workflows/arc-validate-chart.yaml

@@ -28,7 +28,7 @@ permissions:
   contents: read
 
 concurrency:
-  # This will make sure we only apply the concurrency limits on pull requests 
+  # This will make sure we only apply the concurrency limits on pull requests
   # but not pushes to master branch by making the concurrency group name unique
   # for pushes
   group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
@@ -69,10 +69,10 @@ jobs:
       # python is a requirement for the chart-testing action below (supports yamllint among other tests)
       - uses: actions/setup-python@v4
         with:
-          python-version: '3.7'
+          python-version: '3.11'
 
       - name: Set up chart-testing
-        uses: helm/chart-testing-action@v2.4.0
+        uses: helm/chart-testing-action@v2.6.0
 
       - name: Run chart-testing (list-changed)
         id: list-changed

.github/workflows/gha-validate-chart.yaml

@@ -24,7 +24,7 @@ permissions:
   contents: read
 
 concurrency:
-  # This will make sure we only apply the concurrency limits on pull requests 
+  # This will make sure we only apply the concurrency limits on pull requests
   # but not pushes to master branch by making the concurrency group name unique
   # for pushes
   group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
@@ -65,10 +65,10 @@ jobs:
       # python is a requirement for the chart-testing action below (supports yamllint among other tests)
       - uses: actions/setup-python@v4
         with:
-          python-version: '3.7'
+          python-version: '3.11'
 
       - name: Set up chart-testing
-        uses: helm/chart-testing-action@v2.4.0
+        uses: helm/chart-testing-action@v2.6.0
 
       - name: Run chart-testing (list-changed)
         id: list-changed

charts/gha-runner-scale-set-controller/templates/NOTES.txt

@@ -2,4 +2,3 @@ Thank you for installing {{ .Chart.Name }}.
 
 Your release is named {{ .Release.Name }}.
 
-WARNING: value specified under image.pullPolicy will be ignored and no longer be applied to the listener pod spec as of gha-runner-scale-set-0.7.0. Please use the listenerTemplate in the gha-runner-scale-set chart to control the image pull policy of the listener.

charts/gha-runner-scale-set-controller/templates/deployment.yaml

@@ -94,8 +94,6 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
-        - name: CONTROLLER_MANAGER_LISTENER_IMAGE_PULL_POLICY
-          value: "{{ .Values.image.pullPolicy | default "IfNotPresent" }}"
         {{- with .Values.env }}
           {{- if kindIs "slice" . }}
             {{- toYaml . | nindent 8 }}

charts/gha-runner-scale-set-controller/tests/template_test.go

@@ -368,14 +368,12 @@ func TestTemplate_ControllerDeployment_Defaults(t *testing.T) {
 	}
 	assert.ElementsMatch(t, expectedArgs, deployment.Spec.Template.Spec.Containers[0].Args)
 
-	assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 3)
+	assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 2)
 	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, "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)
-	assert.Equal(t, "CONTROLLER_MANAGER_LISTENER_IMAGE_PULL_POLICY", deployment.Spec.Template.Spec.Containers[0].Env[2].Name)
-	assert.Equal(t, "IfNotPresent", deployment.Spec.Template.Spec.Containers[0].Env[2].Value) // default value. Needs to align with controllers/actions.github.com/resourcebuilder.go
 
 	assert.Empty(t, deployment.Spec.Template.Spec.Containers[0].Resources)
 	assert.Nil(t, deployment.Spec.Template.Spec.Containers[0].SecurityContext)
@@ -459,8 +457,8 @@ 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[3].Name)
+	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[3].Value)
+	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)
@@ -505,17 +503,15 @@ func TestTemplate_ControllerDeployment_Customize(t *testing.T) {
 
 	assert.ElementsMatch(t, expectArgs, deployment.Spec.Template.Spec.Containers[0].Args)
 
-	assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 4)
+	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, "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)
-	assert.Equal(t, "CONTROLLER_MANAGER_LISTENER_IMAGE_PULL_POLICY", deployment.Spec.Template.Spec.Containers[0].Env[2].Name)
-	assert.Equal(t, "Always", deployment.Spec.Template.Spec.Containers[0].Env[2].Value) // default value. Needs to align with controllers/actions.github.com/resourcebuilder.go
 
-	assert.Equal(t, "ENV_VAR_NAME_1", deployment.Spec.Template.Spec.Containers[0].Env[3].Name)
+	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[3].Value)
+	assert.Equal(t, "ENV_VAR_VALUE_1", deployment.Spec.Template.Spec.Containers[0].Env[2].Value)
 
 	assert.Equal(t, "500m", deployment.Spec.Template.Spec.Containers[0].Resources.Limits.Cpu().String())
 	assert.True(t, *deployment.Spec.Template.Spec.Containers[0].SecurityContext.RunAsNonRoot)
@@ -762,14 +758,12 @@ func TestTemplate_ControllerDeployment_WatchSingleNamespace(t *testing.T) {
 
 	assert.ElementsMatch(t, expectedArgs, deployment.Spec.Template.Spec.Containers[0].Args)
 
-	assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 3)
+	assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 2)
 	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, "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)
-	assert.Equal(t, "CONTROLLER_MANAGER_LISTENER_IMAGE_PULL_POLICY", deployment.Spec.Template.Spec.Containers[0].Env[2].Name)
-	assert.Equal(t, "IfNotPresent", deployment.Spec.Template.Spec.Containers[0].Env[2].Value) // default value. Needs to align with controllers/actions.github.com/resourcebuilder.go
 
 	assert.Empty(t, deployment.Spec.Template.Spec.Containers[0].Resources)
 	assert.Nil(t, deployment.Spec.Template.Spec.Containers[0].SecurityContext)
@@ -812,17 +806,17 @@ func TestTemplate_ControllerContainerEnvironmentVariables(t *testing.T) {
 	assert.Equal(t, namespaceName, deployment.Namespace)
 	assert.Equal(t, "test-arc-gha-rs-controller", deployment.Name)
 
-	assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 7)
+	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[3].Name)
+	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[3].Value)
+	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[4].Name)
+	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[4].ValueFrom.SecretKeyRef.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[4].ValueFrom.SecretKeyRef.Key)
+	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[4].ValueFrom.SecretKeyRef.Optional)
+	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[5].Name)
+	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[5].Value)
+	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[6].Name)
+	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[6].ValueFrom)
+	assert.Empty(t, deployment.Spec.Template.Spec.Containers[0].Env[5].ValueFrom)
 }
 
 func TestTemplate_WatchSingleNamespace_NotCreateManagerClusterRole(t *testing.T) {

controllers/actions.github.com/constants.go

@@ -2,7 +2,6 @@ package actionsgithubcom
 
 import (
 	"github.com/actions/actions-runner-controller/logging"
-	corev1 "k8s.io/api/core/v1"
 )
 
 const (
@@ -59,10 +58,6 @@ const (
 	AnnotationKeyNoPermissionServiceAccountName   = "actions.github.com/cleanup-no-permission-service-account-name"
 )
 
-// DefaultScaleSetListenerImagePullPolicy is the default pull policy applied
-// to the listener when ImagePullPolicy is not specified
-const DefaultScaleSetListenerImagePullPolicy = corev1.PullIfNotPresent
-
 // DefaultScaleSetListenerLogLevel is the default log level applied
 const DefaultScaleSetListenerLogLevel = string(logging.LogLevelDebug)
 

controllers/actions.github.com/resourcebuilder.go

@@ -41,18 +41,6 @@ const labelValueKubernetesPartOf = "gha-runner-scale-set"
 var scaleSetListenerLogLevel = DefaultScaleSetListenerLogLevel
 var scaleSetListenerLogFormat = DefaultScaleSetListenerLogFormat
 
-var scaleSetListenerImagePullPolicy = DefaultScaleSetListenerImagePullPolicy
-
-func SetListenerImagePullPolicy(pullPolicy string) bool {
-	switch p := corev1.PullPolicy(pullPolicy); p {
-	case corev1.PullAlways, corev1.PullIfNotPresent, corev1.PullNever:
-		scaleSetListenerImagePullPolicy = p
-		return true
-	default:
-		return false
-	}
-}
-
 func SetListenerLoggingParameters(level string, format string) bool {
 	switch level {
 	case logging.LogLevelDebug, logging.LogLevelInfo, logging.LogLevelWarn, logging.LogLevelError:
@@ -233,10 +221,9 @@ func (b *resourceBuilder) newScaleSetListenerPod(autoscalingListener *v1alpha1.A
 		ServiceAccountName: serviceAccount.Name,
 		Containers: []corev1.Container{
 			{
-				Name:            autoscalingListenerContainerName,
+				Name:  autoscalingListenerContainerName,
-				Image:           autoscalingListener.Spec.Image,
+				Image: autoscalingListener.Spec.Image,
-				Env:             listenerEnv,
+				Env:   listenerEnv,
-				ImagePullPolicy: scaleSetListenerImagePullPolicy,
 				Command: []string{
 					"/github-runnerscaleset-listener",
 				},
@@ -380,10 +367,7 @@ func mergeListenerContainer(base, from *corev1.Container) {
 
 	base.Env = append(base.Env, from.Env...)
 
-	if from.ImagePullPolicy != "" {
+	base.ImagePullPolicy = from.ImagePullPolicy
-		base.ImagePullPolicy = from.ImagePullPolicy
-	}
-
 	base.Args = append(base.Args, from.Args...)
 	base.WorkingDir = from.WorkingDir
 	base.Ports = append(base.Ports, from.Ports...)

main.go

@@ -185,13 +185,6 @@ func main() {
 		}
 	}
 
-	listenerPullPolicy := os.Getenv("CONTROLLER_MANAGER_LISTENER_IMAGE_PULL_POLICY")
-	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 {