feat: add default linux nodeSelector to listener pod (#4377)

Co-authored-by: Nikola Jokic <[email protected]>

controllers/actions.github.com/constants.go

@@ -28,6 +28,9 @@ const (
 	LabelKeyKubernetesComponent = "app.kubernetes.io/component"
 	LabelKeyKubernetesVersion   = "app.kubernetes.io/version"
 
+	// Well-known Kubernetes node labels
+	LabelKeyKubernetesOS = "kubernetes.io/os"
+
 	// Github labels
 	LabelKeyGitHubScaleSetName      = "actions.github.com/scale-set-name"
 	LabelKeyGitHubScaleSetNamespace = "actions.github.com/scale-set-namespace"

controllers/actions.github.com/resourcebuilder.go

@@ -244,6 +244,9 @@ func (b *ResourceBuilder) newScaleSetListenerPod(autoscalingListener *v1alpha1.A
 	terminationGracePeriodSeconds := int64(60)
 	podSpec := corev1.PodSpec{
 		ServiceAccountName: serviceAccount.Name,
+		NodeSelector: map[string]string{
+			LabelKeyKubernetesOS: "linux",
+		},
 		Containers: []corev1.Container{
 			{
 				Name:  autoscalingListenerContainerName,
@@ -353,13 +356,16 @@ func mergeListenerPodWithTemplate(pod *corev1.Pod, tmpl *corev1.PodTemplateSpec)
 		pod.Spec.ImagePullSecrets = tmpl.Spec.ImagePullSecrets
 	}
 
+	if tmpl.Spec.NodeSelector != nil {
+		pod.Spec.NodeSelector = tmpl.Spec.NodeSelector
+	}
+
 	pod.Spec.Volumes = append(pod.Spec.Volumes, tmpl.Spec.Volumes...)
 	pod.Spec.InitContainers = tmpl.Spec.InitContainers
 	pod.Spec.EphemeralContainers = tmpl.Spec.EphemeralContainers
 	pod.Spec.TerminationGracePeriodSeconds = tmpl.Spec.TerminationGracePeriodSeconds
 	pod.Spec.ActiveDeadlineSeconds = tmpl.Spec.ActiveDeadlineSeconds
 	pod.Spec.DNSPolicy = tmpl.Spec.DNSPolicy
-	pod.Spec.NodeSelector = tmpl.Spec.NodeSelector
 	pod.Spec.NodeName = tmpl.Spec.NodeName
 	pod.Spec.HostNetwork = tmpl.Spec.HostNetwork
 	pod.Spec.HostPID = tmpl.Spec.HostPID

controllers/actions.github.com/resourcebuilder_test.go

@@ -242,3 +242,111 @@ func TestOwnershipRelationships(t *testing.T) {
 	assert.Equal(t, true, *ownerRef.Controller, "Controller flag should be true")
 	assert.Equal(t, true, *ownerRef.BlockOwnerDeletion, "BlockOwnerDeletion flag should be true")
 }
+
+func TestListenerPodNodeSelector(t *testing.T) {
+	autoscalingRunnerSet := v1alpha1.AutoscalingRunnerSet{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-scale-set",
+			Namespace: "test-ns",
+			Labels: map[string]string{
+				LabelKeyKubernetesPartOf:  labelValueKubernetesPartOf,
+				LabelKeyKubernetesVersion: "0.2.0",
+			},
+			Annotations: map[string]string{
+				runnerScaleSetIDAnnotationKey:         "1",
+				AnnotationKeyGitHubRunnerGroupName:    "test-group",
+				AnnotationKeyGitHubRunnerScaleSetName: "test-scale-set",
+			},
+		},
+		Spec: v1alpha1.AutoscalingRunnerSetSpec{
+			GitHubConfigUrl: "https://github.com/org/repo",
+		},
+	}
+
+	b := ResourceBuilder{}
+	ephemeralRunnerSet, err := b.newEphemeralRunnerSet(&autoscalingRunnerSet)
+	require.NoError(t, err)
+
+	listener, err := b.newAutoScalingListener(&autoscalingRunnerSet, ephemeralRunnerSet, autoscalingRunnerSet.Namespace, "test:latest", nil)
+	require.NoError(t, err)
+
+	listenerServiceAccount := &corev1.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test",
+		},
+	}
+
+	t.Run("default listener pod has linux nodeSelector", func(t *testing.T) {
+		pod, err := b.newScaleSetListenerPod(listener, &corev1.Secret{}, listenerServiceAccount, nil)
+		require.NoError(t, err)
+		require.NotNil(t, pod.Spec.NodeSelector)
+		assert.Equal(t, "linux", pod.Spec.NodeSelector[LabelKeyKubernetesOS],
+			"listener pod should default to linux nodeSelector")
+	})
+
+	t.Run("nil listenerTemplate preserves linux nodeSelector", func(t *testing.T) {
+		listenerNoTemplate := listener.DeepCopy()
+		listenerNoTemplate.Spec.Template = nil
+
+		pod, err := b.newScaleSetListenerPod(listenerNoTemplate, &corev1.Secret{}, listenerServiceAccount, nil)
+		require.NoError(t, err)
+		require.NotNil(t, pod.Spec.NodeSelector)
+		assert.Equal(t, "linux", pod.Spec.NodeSelector[LabelKeyKubernetesOS],
+			"listener pod should keep linux nodeSelector when no template is provided")
+	})
+
+	t.Run("listenerTemplate with nil nodeSelector preserves linux default", func(t *testing.T) {
+		listenerWithTemplate := listener.DeepCopy()
+		listenerWithTemplate.Spec.Template = &corev1.PodTemplateSpec{
+			Spec: corev1.PodSpec{
+				// NodeSelector intentionally nil
+				Tolerations: []corev1.Toleration{
+					{Key: "example.com/test", Operator: corev1.TolerationOpExists},
+				},
+			},
+		}
+
+		pod, err := b.newScaleSetListenerPod(listenerWithTemplate, &corev1.Secret{}, listenerServiceAccount, nil)
+		require.NoError(t, err)
+		require.NotNil(t, pod.Spec.NodeSelector,
+			"linux nodeSelector should not be cleared by template with nil nodeSelector")
+		assert.Equal(t, "linux", pod.Spec.NodeSelector[LabelKeyKubernetesOS])
+		assert.Len(t, pod.Spec.Tolerations, 1, "other template fields should still be applied")
+	})
+
+	t.Run("listenerTemplate with explicit nodeSelector overrides default", func(t *testing.T) {
+		listenerWithTemplate := listener.DeepCopy()
+		listenerWithTemplate.Spec.Template = &corev1.PodTemplateSpec{
+			Spec: corev1.PodSpec{
+				NodeSelector: map[string]string{
+					LabelKeyKubernetesOS: "linux",
+					"custom-label/pool":  "listeners",
+				},
+			},
+		}
+
+		pod, err := b.newScaleSetListenerPod(listenerWithTemplate, &corev1.Secret{}, listenerServiceAccount, nil)
+		require.NoError(t, err)
+		require.NotNil(t, pod.Spec.NodeSelector)
+		assert.Equal(t, "linux", pod.Spec.NodeSelector[LabelKeyKubernetesOS])
+		assert.Equal(t, "listeners", pod.Spec.NodeSelector["custom-label/pool"],
+			"explicit template nodeSelector should be applied")
+	})
+
+	t.Run("listenerTemplate with empty nodeSelector overrides default", func(t *testing.T) {
+		listenerWithTemplate := listener.DeepCopy()
+		listenerWithTemplate.Spec.Template = &corev1.PodTemplateSpec{
+			Spec: corev1.PodSpec{
+				NodeSelector: map[string]string{},
+			},
+		}
+
+		pod, err := b.newScaleSetListenerPod(listenerWithTemplate, &corev1.Secret{}, listenerServiceAccount, nil)
+		require.NoError(t, err)
+		// An explicitly set empty map is non-nil, so it overrides the default.
+		// This is intentional: the user explicitly opted out of nodeSelector constraints.
+		assert.NotNil(t, pod.Spec.NodeSelector)
+		assert.Empty(t, pod.Spec.NodeSelector,
+			"explicitly empty nodeSelector should override the linux default")
+	})
+}