From ac64be27d58cf9a1865f46acf91832b6cab5b5d2 Mon Sep 17 00:00:00 2001 From: Guilherme Teixeira <4645845+gateixeira@users.noreply.github.com> Date: Wed, 11 Feb 2026 10:34:33 +0100 Subject: [PATCH] feat: add default linux nodeSelector to listener pod The listener is a Linux-only binary. In mixed-OS clusters (e.g. with Windows node pools), the listener pod can be scheduled on a Windows node where it fails with a platform mismatch. Issue #2652 was closed by #2758 which gave users a workaround via listenerTemplate, but the default behavior still allows the listener to be scheduled on Windows nodes. This PR adds a safe default (kubernetes.io/os: linux) so users don't need to explicitly configure listenerTemplate just to prevent the listener from landing on a non-Linux node. Changes: - Add default nodeSelector kubernetes.io/os: linux to listener pod spec - Define LabelKeyKubernetesOS constant alongside existing k8s labels - Make mergeListenerPodWithTemplate preserve the default when the listenerTemplate does not explicitly set a nodeSelector (nil check) - Add unit tests for all nodeSelector merge scenarios Refs: #2652 --- controllers/actions.github.com/constants.go | 3 + .../actions.github.com/resourcebuilder.go | 8 +- .../resourcebuilder_test.go | 108 ++++++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/controllers/actions.github.com/constants.go b/controllers/actions.github.com/constants.go index 6619a504..ad1841fc 100644 --- a/controllers/actions.github.com/constants.go +++ b/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" diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index 98b894a6..3c66b1cb 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/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 diff --git a/controllers/actions.github.com/resourcebuilder_test.go b/controllers/actions.github.com/resourcebuilder_test.go index b4c11466..24af4ca5 100644 --- a/controllers/actions.github.com/resourcebuilder_test.go +++ b/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") + }) +}