From 8d6d32e53db520803dce146bdb5825f53fc32e87 Mon Sep 17 00:00:00 2001
From: Matthias Bertschy <matthias.bertschy@gmail.com>
Date: Mon, 29 Jun 2026 11:53:42 +0200
Subject: [PATCH 1/4] feat(autoscaler): deploy node-agent on nodes missing the
 grouping label

The node-agent autoscaler grouped nodes by NodeGroupLabel
(default node.kubernetes.io/instance-type) and skipped any node that did
not carry it, so on on-prem/custom clusters no node-agent DaemonSet was
created and the autoscaler effectively did nothing.

Add a configurable DefaultNodeGroup (default "default", empty = strict
skip). Label-less nodes now fall into this default group, flagged via
NodeGroup.IsDefault and exposed to the template as IsDefaultGroup. Since a
nodeSelector cannot match an absent label, the default group's DaemonSet
is targeted with a "DoesNotExist" node affinity instead (the Helm chart
provides the template branch).

Sizing is unchanged (first node's allocatable per group).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
---
 config/config.go                             | 10 +-
 config/config_test.go                        |  5 +-
 docs/node-agent-autoscaler.md                | 30 +++++-
 nodeagentautoscaler/autoscaler_test.go       | 79 ++++++++++++++++
 nodeagentautoscaler/integration_test.go      | 49 ++++++++++
 nodeagentautoscaler/nodegrouper.go           | 31 +++++--
 nodeagentautoscaler/templaterenderer.go      |  6 +-
 nodeagentautoscaler/templaterenderer_test.go | 97 ++++++++++++++++++++
 8 files changed, 294 insertions(+), 13 deletions(-)

diff --git a/config/config.go b/config/config.go
index 8232e2a..9a73b3a 100644
--- a/config/config.go
+++ b/config/config.go
@@ -66,8 +66,13 @@ type NodeAgentAutoscalerResourceBounds struct {
 
 // NodeAgentAutoscalerConfig defines the configuration for node agent autoscaling
 type NodeAgentAutoscalerConfig struct {
-	Enabled                bool                                   `json:"enabled" mapstructure:"enabled"`
-	NodeGroupLabel         string                                 `json:"nodeGroupLabel" mapstructure:"nodeGroupLabel"`
+	Enabled        bool   `json:"enabled" mapstructure:"enabled"`
+	NodeGroupLabel string `json:"nodeGroupLabel" mapstructure:"nodeGroupLabel"`
+	// DefaultNodeGroup is the group value assigned to nodes that do not carry the
+	// NodeGroupLabel. This ensures a node-agent DaemonSet is still deployed on such
+	// nodes (e.g. on-prem or custom clusters that do not populate the label).
+	// If set to an empty string, nodes without the label are skipped.
+	DefaultNodeGroup       string                                 `json:"defaultNodeGroup" mapstructure:"defaultNodeGroup"`
 	ResourcePercentages    NodeAgentAutoscalerResourcePercentages `json:"resourcePercentages" mapstructure:"resourcePercentages"`
 	MinResources           NodeAgentAutoscalerResourceBounds      `json:"minResources" mapstructure:"minResources"`
 	MaxResources           NodeAgentAutoscalerResourceBounds      `json:"maxResources" mapstructure:"maxResources"`
@@ -306,6 +311,7 @@ func LoadConfig(path string) (Config, error) {
 	// Node agent autoscaler defaults
 	viper.SetDefault("nodeAgentAutoscaler.enabled", false)
 	viper.SetDefault("nodeAgentAutoscaler.nodeGroupLabel", "node.kubernetes.io/instance-type")
+	viper.SetDefault("nodeAgentAutoscaler.defaultNodeGroup", "default")
 	viper.SetDefault("nodeAgentAutoscaler.resourcePercentages.requestCPU", 2)
 	viper.SetDefault("nodeAgentAutoscaler.resourcePercentages.requestMemory", 2)
 	viper.SetDefault("nodeAgentAutoscaler.resourcePercentages.limitCPU", 5)
diff --git a/config/config_test.go b/config/config_test.go
index f58a64b..30889a3 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -103,8 +103,9 @@ func TestLoadConfig(t *testing.T) {
 					Namespace: "default",
 				},
 				NodeAgentAutoscaler: NodeAgentAutoscalerConfig{
-					Enabled:        false,
-					NodeGroupLabel: "node.kubernetes.io/instance-type",
+					Enabled:          false,
+					NodeGroupLabel:   "node.kubernetes.io/instance-type",
+					DefaultNodeGroup: "default",
 					ResourcePercentages: NodeAgentAutoscalerResourcePercentages{
 						RequestCPU:    2,
 						RequestMemory: 2,
diff --git a/docs/node-agent-autoscaler.md b/docs/node-agent-autoscaler.md
index 5c11961..9500e65 100644
--- a/docs/node-agent-autoscaler.md
+++ b/docs/node-agent-autoscaler.md
@@ -79,6 +79,31 @@ annotations:
   argocd.argoproj.io/sync-options: Prune=false
 ```
 
+**Node Targeting:**
+
+Each DaemonSet selects the nodes it runs on based on the grouping label:
+
+- **Normal groups** use a `nodeSelector` matching the grouping label value, e.g.
+  `node.kubernetes.io/instance-type: m5.large`.
+- **The default group** (`IsDefaultGroup`) targets nodes that are *missing* the
+  grouping label. A `nodeSelector` cannot match an absent label, so its template
+  branch instead uses a `DoesNotExist` node affinity:
+
+  ```yaml
+  affinity:
+    nodeAffinity:
+      requiredDuringSchedulingIgnoredDuringExecution:
+        nodeSelectorTerms:
+        - matchExpressions:
+          - key: node.kubernetes.io/instance-type   # the grouping label key
+            operator: DoesNotExist
+  ```
+
+  This ensures a `node-agent` is deployed on label-less nodes (on-prem / custom
+  clusters) without requiring the cluster operator to label nodes manually. The
+  branch is driven by `IsDefaultGroup` in the template data; the DaemonSet
+  template must therefore include both branches.
+
 ### NodeGrouper (`nodegrouper.go`)
 
 Handles node discovery and resource calculation.
@@ -139,6 +164,7 @@ Manages DaemonSet template loading and rendering.
 type TemplateData struct {
     Name           string            // e.g., "node-agent-m5-large"
     NodeGroupLabel string            // e.g., "m5.large"
+    IsDefaultGroup bool              // true for the fallback group of label-less nodes
     Resources      TemplateResources // Requests and limits
 }
 
@@ -193,6 +219,7 @@ The autoscaler is configured via the operator's ConfigMap:
 type NodeAgentAutoscalerConfig struct {
     Enabled             bool          // Enable/disable autoscaler
     NodeGroupLabel      string        // Label to group nodes by
+    DefaultNodeGroup    string        // Group value for nodes missing NodeGroupLabel (default: "default"; empty = skip)
     ResourcePercentages struct {
         RequestCPU    int  // % of allocatable CPU for requests
         RequestMemory int  // % of allocatable memory for requests
@@ -295,7 +322,8 @@ kubectl get events -n kubescape --field-selector reason=Created,reason=Deleted
 
 | Scenario | Behavior |
 |----------|----------|
-| Node missing group label | Log error, skip node (no DaemonSet for it) |
+| Node missing group label (`DefaultNodeGroup` set) | Assign node to the default group (targeted via `DoesNotExist` affinity) so a node-agent is still deployed |
+| Node missing group label (`DefaultNodeGroup` empty) | Log error, skip node (no DaemonSet for it) |
 | Template parse error | Fatal error on startup |
 | Template render error | Log error, skip node group |
 | API error (create/update/delete) | Log error, continue reconciliation |
diff --git a/nodeagentautoscaler/autoscaler_test.go b/nodeagentautoscaler/autoscaler_test.go
index bec5033..44e20ad 100644
--- a/nodeagentautoscaler/autoscaler_test.go
+++ b/nodeagentautoscaler/autoscaler_test.go
@@ -286,6 +286,85 @@ func TestNodeGrouper_GetNodeGroups_SkipsNodesWithoutLabel(t *testing.T) {
 	assert.Equal(t, 1, groups[0].NodeCount)
 }
 
+func TestNodeGrouper_GetNodeGroups_DefaultNodeGroup(t *testing.T) {
+	ctx := context.Background()
+
+	// Two nodes without the grouping label, one with it.
+	nodes := []runtime.Object{
+		&corev1.Node{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "node-with-label",
+				Labels: map[string]string{
+					"node.kubernetes.io/instance-type": "m5.large",
+				},
+			},
+			Status: corev1.NodeStatus{
+				Conditions: []corev1.NodeCondition{
+					{Type: corev1.NodeReady, Status: corev1.ConditionTrue},
+				},
+				Allocatable: corev1.ResourceList{
+					corev1.ResourceCPU:    resource.MustParse("4"),
+					corev1.ResourceMemory: resource.MustParse("16Gi"),
+				},
+			},
+		},
+		&corev1.Node{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "node-without-label-1",
+				Labels: map[string]string{},
+			},
+			Status: corev1.NodeStatus{
+				Conditions: []corev1.NodeCondition{
+					{Type: corev1.NodeReady, Status: corev1.ConditionTrue},
+				},
+				Allocatable: corev1.ResourceList{
+					corev1.ResourceCPU:    resource.MustParse("8"),
+					corev1.ResourceMemory: resource.MustParse("32Gi"),
+				},
+			},
+		},
+		&corev1.Node{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "node-without-label-2",
+				Labels: map[string]string{},
+			},
+			Status: corev1.NodeStatus{
+				Conditions: []corev1.NodeCondition{
+					{Type: corev1.NodeReady, Status: corev1.ConditionTrue},
+				},
+				Allocatable: corev1.ResourceList{
+					corev1.ResourceCPU:    resource.MustParse("8"),
+					corev1.ResourceMemory: resource.MustParse("32Gi"),
+				},
+			},
+		},
+	}
+
+	client := fake.NewSimpleClientset(nodes...)
+
+	cfg := config.NodeAgentAutoscalerConfig{
+		Enabled:          true,
+		NodeGroupLabel:   "node.kubernetes.io/instance-type",
+		DefaultNodeGroup: "default",
+	}
+
+	ng := NewNodeGrouper(client, cfg, "kubescape")
+	groups, err := ng.GetNodeGroups(ctx)
+
+	require.NoError(t, err)
+	// Both unlabeled nodes collapse into the "default" group alongside the labeled one.
+	assert.Len(t, groups, 2)
+
+	byLabel := make(map[string]NodeGroup, len(groups))
+	for _, g := range groups {
+		byLabel[g.LabelValue] = g
+	}
+	require.Contains(t, byLabel, "default")
+	assert.Equal(t, 2, byLabel["default"].NodeCount)
+	require.Contains(t, byLabel, "m5.large")
+	assert.Equal(t, 1, byLabel["m5.large"].NodeCount)
+}
+
 func TestNodeGrouper_CalculateResources(t *testing.T) {
 	cfg := config.NodeAgentAutoscalerConfig{
 		ResourcePercentages: config.NodeAgentAutoscalerResourcePercentages{
diff --git a/nodeagentautoscaler/integration_test.go b/nodeagentautoscaler/integration_test.go
index 7552bd5..a5b2b85 100644
--- a/nodeagentautoscaler/integration_test.go
+++ b/nodeagentautoscaler/integration_test.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 )
 
@@ -77,3 +78,51 @@ func TestIntegration_HelmGeneratedTemplate(t *testing.T) {
 		container.Resources.Limits.Cpu().String(),
 		container.Resources.Limits.Memory().String())
 }
+
+// TestIntegration_HelmGeneratedTemplate_DefaultGroup verifies the default group
+// (nodes missing the grouping label) renders with a DoesNotExist node affinity and
+// without an instance-type nodeSelector, using the actual Helm-generated template.
+func TestIntegration_HelmGeneratedTemplate_DefaultGroup(t *testing.T) {
+	templatePath := "/tmp/test-daemonset-template.yaml"
+
+	if _, err := os.Stat(templatePath); os.IsNotExist(err) {
+		t.Skip("Integration test requires template file. Run Helm extraction first. See test comments for instructions.")
+	}
+
+	renderer, err := NewTemplateRenderer(templatePath, 0.8)
+	require.NoError(t, err)
+
+	group := NodeGroup{
+		LabelValue:    "default",
+		SanitizedName: "default",
+		IsDefault:     true,
+	}
+	resources := CalculatedResources{
+		Requests: ResourcePair{CPU: resource.MustParse("100m"), Memory: resource.MustParse("200Mi")},
+		Limits:   ResourcePair{CPU: resource.MustParse("500m"), Memory: resource.MustParse("1Gi")},
+	}
+
+	ds, err := renderer.RenderDaemonSet(group, resources)
+	require.NoError(t, err, "Failed to render default-group DaemonSet - check template YAML structure")
+
+	assert.Equal(t, "node-agent-default", ds.Name)
+
+	// Must not pin to an instance-type value (the nodes lack the label).
+	assert.NotContains(t, ds.Spec.Template.Spec.NodeSelector, "node.kubernetes.io/instance-type")
+
+	// Must select nodes where the grouping label does not exist.
+	require.NotNil(t, ds.Spec.Template.Spec.Affinity)
+	require.NotNil(t, ds.Spec.Template.Spec.Affinity.NodeAffinity)
+	terms := ds.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution
+	require.NotNil(t, terms)
+	require.NotEmpty(t, terms.NodeSelectorTerms)
+
+	var foundDoesNotExist bool
+	for _, expr := range terms.NodeSelectorTerms[0].MatchExpressions {
+		if expr.Key == "node.kubernetes.io/instance-type" {
+			assert.Equal(t, corev1.NodeSelectorOpDoesNotExist, expr.Operator)
+			foundDoesNotExist = true
+		}
+	}
+	assert.True(t, foundDoesNotExist, "expected a DoesNotExist match expression on the grouping label")
+}
diff --git a/nodeagentautoscaler/nodegrouper.go b/nodeagentautoscaler/nodegrouper.go
index 0d1d220..4abf9c9 100644
--- a/nodeagentautoscaler/nodegrouper.go
+++ b/nodeagentautoscaler/nodegrouper.go
@@ -28,6 +28,10 @@ type NodeGroup struct {
 	AllocatableMemory resource.Quantity
 	// NodeCount is the number of nodes in this group
 	NodeCount int
+	// IsDefault is true when this group was formed from nodes missing the
+	// NodeGroupLabel. Such a group cannot be targeted by a nodeSelector on the
+	// (absent) label, so its DaemonSet uses a "DoesNotExist" node affinity instead.
+	IsDefault bool
 }
 
 // CalculatedResources represents the calculated resource requests and limits
@@ -75,18 +79,31 @@ func (ng *NodeGrouper) GetNodeGroups(ctx context.Context) ([]NodeGroup, error) {
 		}
 
 		labelValue, ok := node.Labels[ng.config.NodeGroupLabel]
+		isDefault := false
 		if !ok {
-			// If label doesn't exist, log an error and skip this node
-			// Cloud managed Kubernetes (AKS, EKS, GKE) populate this field automatically
-			// For on-prem or custom clusters, the infrastructure must be configured to set this label
-			logger.L().Ctx(ctx).Error("node missing required label for autoscaler, node-agent will not be deployed on this node",
+			// Cloud managed Kubernetes (AKS, EKS, GKE) populate this field automatically.
+			// For on-prem or custom clusters the label may be absent, so fall back to the
+			// configured default group to ensure a node-agent is still deployed on this node.
+			if ng.config.DefaultNodeGroup == "" {
+				// Default group disabled: preserve the strict behavior and skip the node.
+				logger.L().Ctx(ctx).Error("node missing required label for autoscaler, node-agent will not be deployed on this node",
+					helpers.String("node", node.Name),
+					helpers.String("requiredLabel", ng.config.NodeGroupLabel))
+				continue
+			}
+			logger.L().Debug("node missing node group label, assigning default node group",
 				helpers.String("node", node.Name),
-				helpers.String("requiredLabel", ng.config.NodeGroupLabel))
-			continue
+				helpers.String("requiredLabel", ng.config.NodeGroupLabel),
+				helpers.String("defaultNodeGroup", ng.config.DefaultNodeGroup))
+			labelValue = ng.config.DefaultNodeGroup
+			isDefault = true
 		}
 
 		if group, exists := groupMap[labelValue]; exists {
 			group.NodeCount++
+			// A label value can be shared by labelled and label-less nodes; if any
+			// member lacks the label, the group must be targeted via the affinity path.
+			group.IsDefault = group.IsDefault || isDefault
 		} else {
 			groupMap[labelValue] = &NodeGroup{
 				LabelValue:        labelValue,
@@ -94,6 +111,7 @@ func (ng *NodeGrouper) GetNodeGroups(ctx context.Context) ([]NodeGroup, error) {
 				AllocatableCPU:    *node.Status.Allocatable.Cpu(),
 				AllocatableMemory: *node.Status.Allocatable.Memory(),
 				NodeCount:         1,
+				IsDefault:         isDefault,
 			}
 		}
 	}
@@ -259,4 +277,3 @@ func isNodeReady(node *corev1.Node) bool {
 	}
 	return false
 }
-
diff --git a/nodeagentautoscaler/templaterenderer.go b/nodeagentautoscaler/templaterenderer.go
index 25a7006..fcbf347 100644
--- a/nodeagentautoscaler/templaterenderer.go
+++ b/nodeagentautoscaler/templaterenderer.go
@@ -23,6 +23,10 @@ type TemplateData struct {
 	Name string
 	// NodeGroupLabel is the value of the node group label
 	NodeGroupLabel string
+	// IsDefaultGroup is true for the fallback group of nodes that do not carry the
+	// grouping label. The template should select these nodes via a "DoesNotExist"
+	// node affinity rather than a nodeSelector matching NodeGroupLabel.
+	IsDefaultGroup bool
 	// Resources contains the calculated resource requests and limits
 	Resources TemplateResources
 	// GoMemLimit is the GOMEMLIMIT value derived from the memory limit and percentage (e.g., "360MiB")
@@ -214,6 +218,7 @@ func (tr *TemplateRenderer) RenderDaemonSet(group NodeGroup, resources Calculate
 	data := TemplateData{
 		Name:           fmt.Sprintf("node-agent-%s", group.SanitizedName),
 		NodeGroupLabel: group.LabelValue,
+		IsDefaultGroup: group.IsDefault,
 		Resources: TemplateResources{
 			Requests: TemplateResourcePair{
 				CPU:    resources.Requests.CPU.String(),
@@ -258,4 +263,3 @@ func (tr *TemplateRenderer) RenderDaemonSet(group NodeGroup, resources Calculate
 func GenerateDaemonSetName(group NodeGroup) string {
 	return fmt.Sprintf("node-agent-%s", group.SanitizedName)
 }
-
diff --git a/nodeagentautoscaler/templaterenderer_test.go b/nodeagentautoscaler/templaterenderer_test.go
index 441cec1..b40d0ce 100644
--- a/nodeagentautoscaler/templaterenderer_test.go
+++ b/nodeagentautoscaler/templaterenderer_test.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 )
 
@@ -98,9 +99,21 @@ spec:
           limits:
             cpu: "{{ .Resources.Limits.CPU }}"
             memory: "{{ .Resources.Limits.Memory }}"
+{{- if .IsDefaultGroup }}
+      affinity:
+        nodeAffinity:
+          requiredDuringSchedulingIgnoredDuringExecution:
+            nodeSelectorTerms:
+            - matchExpressions:
+              - key: node.kubernetes.io/instance-type
+                operator: DoesNotExist
+      nodeSelector:
+        kubernetes.io/os: linux
+{{- else }}
       nodeSelector:
         kubernetes.io/os: linux
         node.kubernetes.io/instance-type: "{{ .NodeGroupLabel }}"
+{{- end }}
 `
 
 	// Create temp file
@@ -152,6 +165,90 @@ spec:
 	assert.Equal(t, "1Gi", container.Resources.Limits.Memory().String())
 }
 
+func TestTemplateRenderer_RenderDaemonSet_DefaultGroup(t *testing.T) {
+	// Template branches between a nodeSelector (normal groups) and a
+	// DoesNotExist node affinity (the default group of label-less nodes).
+	templateContent := `apiVersion: apps/v1
+kind: DaemonSet
+metadata:
+  name: "{{ .Name }}"
+  namespace: kubescape
+  labels:
+    kubescape.io/node-group: "{{ .NodeGroupLabel }}"
+spec:
+  selector:
+    matchLabels:
+      app.kubernetes.io/name: node-agent
+  template:
+    metadata:
+      labels:
+        app.kubernetes.io/name: node-agent
+    spec:
+      containers:
+      - name: node-agent
+        image: "quay.io/kubescape/node-agent:v0.3.3"
+        resources:
+          requests:
+            cpu: "{{ .Resources.Requests.CPU }}"
+            memory: "{{ .Resources.Requests.Memory }}"
+          limits:
+            cpu: "{{ .Resources.Limits.CPU }}"
+            memory: "{{ .Resources.Limits.Memory }}"
+{{- if .IsDefaultGroup }}
+      affinity:
+        nodeAffinity:
+          requiredDuringSchedulingIgnoredDuringExecution:
+            nodeSelectorTerms:
+            - matchExpressions:
+              - key: node.kubernetes.io/instance-type
+                operator: DoesNotExist
+      nodeSelector:
+        kubernetes.io/os: linux
+{{- else }}
+      nodeSelector:
+        kubernetes.io/os: linux
+        node.kubernetes.io/instance-type: "{{ .NodeGroupLabel }}"
+{{- end }}
+`
+
+	tmpDir := t.TempDir()
+	templatePath := filepath.Join(tmpDir, "daemonset-template.yaml")
+	require.NoError(t, os.WriteFile(templatePath, []byte(templateContent), 0644))
+
+	renderer, err := NewTemplateRenderer(templatePath, 0.8)
+	require.NoError(t, err)
+
+	group := NodeGroup{
+		LabelValue:    "default",
+		SanitizedName: "default",
+		IsDefault:     true,
+	}
+	resources := CalculatedResources{
+		Requests: ResourcePair{CPU: resource.MustParse("100m"), Memory: resource.MustParse("200Mi")},
+		Limits:   ResourcePair{CPU: resource.MustParse("500m"), Memory: resource.MustParse("1Gi")},
+	}
+
+	ds, err := renderer.RenderDaemonSet(group, resources)
+	require.NoError(t, err)
+
+	assert.Equal(t, "node-agent-default", ds.Name)
+
+	// The default group must NOT pin to an instance-type value (the nodes lack the label).
+	assert.Equal(t, "linux", ds.Spec.Template.Spec.NodeSelector["kubernetes.io/os"])
+	assert.NotContains(t, ds.Spec.Template.Spec.NodeSelector, "node.kubernetes.io/instance-type")
+
+	// Instead it selects nodes where the grouping label does not exist.
+	require.NotNil(t, ds.Spec.Template.Spec.Affinity)
+	require.NotNil(t, ds.Spec.Template.Spec.Affinity.NodeAffinity)
+	terms := ds.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution
+	require.NotNil(t, terms)
+	require.Len(t, terms.NodeSelectorTerms, 1)
+	require.Len(t, terms.NodeSelectorTerms[0].MatchExpressions, 1)
+	expr := terms.NodeSelectorTerms[0].MatchExpressions[0]
+	assert.Equal(t, "node.kubernetes.io/instance-type", expr.Key)
+	assert.Equal(t, corev1.NodeSelectorOpDoesNotExist, expr.Operator)
+}
+
 func TestTemplateRenderer_RenderDaemonSet_InvalidTemplate(t *testing.T) {
 	// Create an invalid template
 	tmpDir := t.TempDir()

From 75ea4f505f1cce9cd5116619305ad517ec2c3a01 Mon Sep 17 00:00:00 2001
From: Matthias Bertschy <matthias.bertschy@gmail.com>
Date: Mon, 29 Jun 2026 13:48:01 +0200
Subject: [PATCH 2/4] fix(autoscaler): address review on default node-group
 handling

Resolve the blockers and nitpicks from #389 review:

- Track label-less nodes under an internal sentinel bucket so a node
  legitimately labelled "<groupingLabel>=default" forms its own normal
  (nodeSelector-targeted) group and is never merged onto the affinity path.
  Salt the fallback group's collision hash so it cannot collapse onto a real
  same-named group.
- Key DaemonSet reconciliation (existence + orphan deletion) off the unique
  DaemonSet name instead of the node-group label value, which is not unique
  once the default group shares a value with a real group.
- Pass the configured grouping label key through TemplateData
  (NodeGroupLabelKey) so the nodeSelector and the default group's DoesNotExist
  affinity key off the operator's actual grouping label rather than a
  hardcoded constant; the Helm template consumes it as the single source.
- Run the integration tests in CI: extract the real DaemonSet template from
  the helm-charts chart so the tests run instead of skipping.
- Tests: assert IsDefault on the grouper contract, assert os=linux survives in
  the default branch, add the =default collision test and a Reconcile
  create/orphan test; switch to fake.NewClientset.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
---
 .github/workflows/pr-created.yaml            |  26 ++-
 docs/node-agent-autoscaler.md                |  21 ++-
 nodeagentautoscaler/autoscaler.go            |  46 ++---
 nodeagentautoscaler/autoscaler_test.go       | 189 ++++++++++++++++++-
 nodeagentautoscaler/integration_test.go      |   7 +-
 nodeagentautoscaler/nodegrouper.go           |  27 ++-
 nodeagentautoscaler/templaterenderer.go      |  16 +-
 nodeagentautoscaler/templaterenderer_test.go |  22 +--
 8 files changed, 294 insertions(+), 60 deletions(-)

diff --git a/.github/workflows/pr-created.yaml b/.github/workflows/pr-created.yaml
index 0f5bf69..f6a4fab 100644
--- a/.github/workflows/pr-created.yaml
+++ b/.github/workflows/pr-created.yaml
@@ -26,8 +26,30 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
+        with:
+          path: operator
       - uses: actions/setup-go@v5
         with:
           go-version: "1.25"
-      - name: Build with integration tag
-        run: go test -tags=integration -run=^$ ./...
+      # The node-agent autoscaler integration tests render the actual DaemonSet
+      # template that lives in the helm-charts repo. Check it out and extract the
+      # template so the tests run for real instead of skipping (see
+      # nodeagentautoscaler/integration_test.go).
+      - name: Checkout helm-charts
+        uses: actions/checkout@v4
+        with:
+          repository: kubescape/helm-charts
+          ref: main
+          path: helm-charts
+      - uses: azure/setup-helm@v4
+      - name: Extract node-agent daemonset template
+        run: |
+          helm template test helm-charts/charts/kubescape-operator \
+            --kube-version 1.29.0 \
+            --set nodeAgent.autoscaler.enabled=true --set clusterName=test \
+          | grep -A 400 "daemonset-template.yaml:" | tail -n +2 | sed 's/^    //' \
+          | awk '/^---/{exit} {print}' > /tmp/test-daemonset-template.yaml
+          test -s /tmp/test-daemonset-template.yaml || { echo "template extraction produced an empty file"; exit 1; }
+      - name: Build and run integration tests
+        working-directory: operator
+        run: go test -tags=integration ./...
diff --git a/docs/node-agent-autoscaler.md b/docs/node-agent-autoscaler.md
index 9500e65..7c37733 100644
--- a/docs/node-agent-autoscaler.md
+++ b/docs/node-agent-autoscaler.md
@@ -95,14 +95,22 @@ Each DaemonSet selects the nodes it runs on based on the grouping label:
       requiredDuringSchedulingIgnoredDuringExecution:
         nodeSelectorTerms:
         - matchExpressions:
-          - key: node.kubernetes.io/instance-type   # the grouping label key
+          - key: <NodeGroupLabelKey>   # the configured grouping label key
             operator: DoesNotExist
   ```
 
   This ensures a `node-agent` is deployed on label-less nodes (on-prem / custom
   clusters) without requiring the cluster operator to label nodes manually. The
   branch is driven by `IsDefaultGroup` in the template data; the DaemonSet
-  template must therefore include both branches.
+  template must therefore include both branches. Both the `nodeSelector` and this
+  affinity key off `NodeGroupLabelKey` (the operator's configured grouping label),
+  so they always match the label the operator actually groups by — even when it is
+  overridden from the default `node.kubernetes.io/instance-type`.
+
+The label-less population is tracked under an internal sentinel key, so a node
+that legitimately carries `<groupingLabel>=<defaultNodeGroup>` forms its own
+normal (nodeSelector-targeted) group and is never merged into the affinity-based
+default group.
 
 ### NodeGrouper (`nodegrouper.go`)
 
@@ -162,10 +170,11 @@ Manages DaemonSet template loading and rendering.
 
 ```go
 type TemplateData struct {
-    Name           string            // e.g., "node-agent-m5-large"
-    NodeGroupLabel string            // e.g., "m5.large"
-    IsDefaultGroup bool              // true for the fallback group of label-less nodes
-    Resources      TemplateResources // Requests and limits
+    Name              string            // e.g., "node-agent-m5-large"
+    NodeGroupLabel    string            // the group's label value, e.g., "m5.large"
+    NodeGroupLabelKey string            // the configured grouping label key, e.g., "node.kubernetes.io/instance-type"
+    IsDefaultGroup    bool              // true for the fallback group of label-less nodes
+    Resources         TemplateResources // Requests and limits
 }
 
 type TemplateResources struct {
diff --git a/nodeagentautoscaler/autoscaler.go b/nodeagentautoscaler/autoscaler.go
index 4bb575c..5800c3a 100644
--- a/nodeagentautoscaler/autoscaler.go
+++ b/nodeagentautoscaler/autoscaler.go
@@ -41,20 +41,20 @@ const (
 
 // Autoscaler manages node-agent DaemonSets based on node groups
 type Autoscaler struct {
-	client               kubernetes.Interface
-	config               config.NodeAgentAutoscalerConfig
-	namespace            string
-	nodeGrouper          *NodeGrouper
-	templateRenderer     *TemplateRenderer
-	stopCh               chan struct{}
-	operatorDeployment   string // Name of the operator deployment (for owner references)
-	ownerRef             *metav1.OwnerReference
-	eventRecorder        record.EventRecorder
+	client             kubernetes.Interface
+	config             config.NodeAgentAutoscalerConfig
+	namespace          string
+	nodeGrouper        *NodeGrouper
+	templateRenderer   *TemplateRenderer
+	stopCh             chan struct{}
+	operatorDeployment string // Name of the operator deployment (for owner references)
+	ownerRef           *metav1.OwnerReference
+	eventRecorder      record.EventRecorder
 }
 
 // NewAutoscaler creates a new Autoscaler instance
 func NewAutoscaler(client kubernetes.Interface, cfg config.NodeAgentAutoscalerConfig, namespace string, operatorDeploymentName string) (*Autoscaler, error) {
-	templateRenderer, err := NewTemplateRenderer(cfg.TemplatePath, cfg.GoMemLimitPercentage)
+	templateRenderer, err := NewTemplateRenderer(cfg.TemplatePath, cfg.GoMemLimitPercentage, cfg.NodeGroupLabel)
 	if err != nil {
 		return nil, err
 	}
@@ -177,21 +177,23 @@ func (a *Autoscaler) Reconcile(ctx context.Context) error {
 		return err
 	}
 
-	// Build a map of existing DaemonSets by node group
-	existingByNodeGroup := make(map[string]*appsv1.DaemonSet)
+	// Build a map of existing DaemonSets by name. The DaemonSet name (derived from
+	// the group's sanitized, collision-resolved name) is the unique per-group
+	// identity. The kubescape.io/node-group label value is NOT unique: the default
+	// group and a real group can legitimately share a label value (e.g. "default").
+	existingByName := make(map[string]*appsv1.DaemonSet, len(existingDaemonSets))
 	for i := range existingDaemonSets {
 		ds := &existingDaemonSets[i]
-		if nodeGroup, ok := ds.Labels[NodeGroupLabelKey]; ok {
-			existingByNodeGroup[nodeGroup] = ds
-		}
+		existingByName[ds.Name] = ds
 	}
 
-	// Track which node groups we've processed
-	processedNodeGroups := make(map[string]bool)
+	// Track which DaemonSets we've processed (by name)
+	processedNames := make(map[string]bool)
 
 	// Create or update DaemonSets for each node group
 	for _, group := range nodeGroups {
-		processedNodeGroups[group.LabelValue] = true
+		dsName := GenerateDaemonSetName(group)
+		processedNames[dsName] = true
 
 		// Calculate resources for this group
 		resources, err := a.nodeGrouper.CalculateResources(group)
@@ -231,7 +233,7 @@ func (a *Autoscaler) Reconcile(ctx context.Context) error {
 		}
 
 		// Check if DaemonSet exists
-		if existingDS, exists := existingByNodeGroup[group.LabelValue]; exists {
+		if existingDS, exists := existingByName[dsName]; exists {
 			// Update if needed
 			if err := a.updateDaemonSetIfNeeded(ctx, existingDS, desiredDS); err != nil {
 				logger.L().Error("failed to update DaemonSet",
@@ -249,8 +251,8 @@ func (a *Autoscaler) Reconcile(ctx context.Context) error {
 	}
 
 	// Delete orphaned DaemonSets (node groups that no longer exist)
-	for nodeGroup, ds := range existingByNodeGroup {
-		if !processedNodeGroups[nodeGroup] {
+	for name, ds := range existingByName {
+		if !processedNames[name] {
 			if err := a.deleteDaemonSet(ctx, ds); err != nil {
 				logger.L().Error("failed to delete orphaned DaemonSet",
 					helpers.String("name", ds.Name),
@@ -352,7 +354,6 @@ func (a *Autoscaler) updateDaemonSetIfNeeded(ctx context.Context, existing, desi
 	return nil
 }
 
-
 // deleteDaemonSet deletes a DaemonSet
 func (a *Autoscaler) deleteDaemonSet(ctx context.Context, ds *appsv1.DaemonSet) error {
 	logger.L().Info("deleting orphaned DaemonSet",
@@ -381,4 +382,3 @@ func (a *Autoscaler) recordEvent(obj runtime.Object, eventType, reason, message
 		a.eventRecorder.Event(obj, eventType, reason, message)
 	}
 }
-
diff --git a/nodeagentautoscaler/autoscaler_test.go b/nodeagentautoscaler/autoscaler_test.go
index 44e20ad..b115ad1 100644
--- a/nodeagentautoscaler/autoscaler_test.go
+++ b/nodeagentautoscaler/autoscaler_test.go
@@ -2,6 +2,8 @@ package nodeagentautoscaler
 
 import (
 	"context"
+	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -198,7 +200,7 @@ func TestNodeGrouper_GetNodeGroups(t *testing.T) {
 		},
 	}
 
-	client := fake.NewSimpleClientset(nodes...)
+	client := fake.NewClientset(nodes...)
 
 	cfg := config.NodeAgentAutoscalerConfig{
 		Enabled:        true,
@@ -269,7 +271,7 @@ func TestNodeGrouper_GetNodeGroups_SkipsNodesWithoutLabel(t *testing.T) {
 		},
 	}
 
-	client := fake.NewSimpleClientset(nodes...)
+	client := fake.NewClientset(nodes...)
 
 	cfg := config.NodeAgentAutoscalerConfig{
 		Enabled:        true,
@@ -340,7 +342,7 @@ func TestNodeGrouper_GetNodeGroups_DefaultNodeGroup(t *testing.T) {
 		},
 	}
 
-	client := fake.NewSimpleClientset(nodes...)
+	client := fake.NewClientset(nodes...)
 
 	cfg := config.NodeAgentAutoscalerConfig{
 		Enabled:          true,
@@ -361,8 +363,72 @@ func TestNodeGrouper_GetNodeGroups_DefaultNodeGroup(t *testing.T) {
 	}
 	require.Contains(t, byLabel, "default")
 	assert.Equal(t, 2, byLabel["default"].NodeCount)
+	assert.True(t, byLabel["default"].IsDefault, "fallback group must be flagged IsDefault")
 	require.Contains(t, byLabel, "m5.large")
 	assert.Equal(t, 1, byLabel["m5.large"].NodeCount)
+	assert.False(t, byLabel["m5.large"].IsDefault, "a labelled group must not be flagged IsDefault")
+}
+
+// TestNodeGrouper_GetNodeGroups_DefaultValueCollision ensures a node legitimately
+// labelled "<groupingLabel>=default" is NOT merged with the synthetic fallback group
+// of label-less nodes: they must stay distinct groups with distinct names so the
+// labelled node keeps a nodeSelector-targeted DaemonSet (not the DoesNotExist path).
+func TestNodeGrouper_GetNodeGroups_DefaultValueCollision(t *testing.T) {
+	ctx := context.Background()
+
+	newNode := func(name string, labels map[string]string) *corev1.Node {
+		return &corev1.Node{
+			ObjectMeta: metav1.ObjectMeta{Name: name, Labels: labels},
+			Status: corev1.NodeStatus{
+				Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
+				Allocatable: corev1.ResourceList{
+					corev1.ResourceCPU:    resource.MustParse("4"),
+					corev1.ResourceMemory: resource.MustParse("16Gi"),
+				},
+			},
+		}
+	}
+
+	nodes := []runtime.Object{
+		// A node legitimately labelled with the value "default".
+		newNode("labelled-default", map[string]string{"node.kubernetes.io/instance-type": "default"}),
+		// Two nodes missing the grouping label entirely.
+		newNode("no-label-1", map[string]string{}),
+		newNode("no-label-2", map[string]string{}),
+	}
+
+	client := fake.NewClientset(nodes...)
+	cfg := config.NodeAgentAutoscalerConfig{
+		Enabled:          true,
+		NodeGroupLabel:   "node.kubernetes.io/instance-type",
+		DefaultNodeGroup: "default",
+	}
+
+	groups, err := NewNodeGrouper(client, cfg, "kubescape").GetNodeGroups(ctx)
+	require.NoError(t, err)
+
+	// Two distinct groups, not one merged bucket.
+	require.Len(t, groups, 2)
+
+	var labelled, fallback *NodeGroup
+	for i := range groups {
+		if groups[i].IsDefault {
+			fallback = &groups[i]
+		} else {
+			labelled = &groups[i]
+		}
+	}
+
+	require.NotNil(t, labelled, "the legitimately labelled =default node must form its own group")
+	assert.Equal(t, 1, labelled.NodeCount)
+	assert.False(t, labelled.IsDefault)
+
+	require.NotNil(t, fallback, "label-less nodes must form the fallback group")
+	assert.Equal(t, 2, fallback.NodeCount)
+	assert.True(t, fallback.IsDefault)
+
+	// Names must be unique so they render to distinct DaemonSets.
+	assert.NotEqual(t, labelled.SanitizedName, fallback.SanitizedName)
 }
 
 func TestNodeGrouper_CalculateResources(t *testing.T) {
@@ -476,7 +542,7 @@ func TestAutoscaler_GetManagedDaemonSets(t *testing.T) {
 		},
 	}
 
-	client := fake.NewSimpleClientset(daemonSets...)
+	client := fake.NewClientset(daemonSets...)
 
 	autoscaler := &Autoscaler{
 		client:    client,
@@ -500,7 +566,7 @@ func TestGenerateDaemonSetName(t *testing.T) {
 }
 
 func TestNewAutoscaler(t *testing.T) {
-	client := fake.NewSimpleClientset()
+	client := fake.NewClientset()
 	cfg := config.NodeAgentAutoscalerConfig{
 		Enabled:              true,
 		GoMemLimitPercentage: 0.8,
@@ -513,3 +579,116 @@ func TestNewAutoscaler(t *testing.T) {
 	_, err := NewAutoscaler(client, cfg, "kubescape", "operator")
 	assert.Error(t, err)
 }
+
+// reconcileTestTemplate is a minimal DaemonSet template exercising both targeting
+// branches, used by the Reconcile test.
+const reconcileTestTemplate = `apiVersion: apps/v1
+kind: DaemonSet
+metadata:
+  name: "{{ .Name }}"
+  namespace: kubescape
+  labels:
+    kubescape.io/managed-by: operator-autoscaler
+    kubescape.io/node-group: "{{ .NodeGroupLabel }}"
+spec:
+  selector:
+    matchLabels:
+      app.kubernetes.io/name: node-agent
+  template:
+    metadata:
+      labels:
+        app.kubernetes.io/name: node-agent
+    spec:
+      containers:
+      - name: node-agent
+        image: "quay.io/kubescape/node-agent:test"
+        resources:
+          requests:
+            cpu: "{{ .Resources.Requests.CPU }}"
+            memory: "{{ .Resources.Requests.Memory }}"
+          limits:
+            cpu: "{{ .Resources.Limits.CPU }}"
+            memory: "{{ .Resources.Limits.Memory }}"
+{{- if .IsDefaultGroup }}
+      affinity:
+        nodeAffinity:
+          requiredDuringSchedulingIgnoredDuringExecution:
+            nodeSelectorTerms:
+            - matchExpressions:
+              - key: {{ .NodeGroupLabelKey }}
+                operator: DoesNotExist
+      nodeSelector:
+        kubernetes.io/os: linux
+{{- else }}
+      nodeSelector:
+        kubernetes.io/os: linux
+        {{ .NodeGroupLabelKey }}: "{{ .NodeGroupLabel }}"
+{{- end }}
+`
+
+// TestAutoscaler_Reconcile_CreatesPerGroupAndDeletesOrphans verifies that Reconcile
+// creates one DaemonSet per node group (keyed by the unique DaemonSet name) and
+// removes managed DaemonSets whose group no longer exists.
+func TestAutoscaler_Reconcile_CreatesPerGroupAndDeletesOrphans(t *testing.T) {
+	ctx := context.Background()
+
+	tmpDir := t.TempDir()
+	templatePath := filepath.Join(tmpDir, "daemonset-template.yaml")
+	require.NoError(t, os.WriteFile(templatePath, []byte(reconcileTestTemplate), 0644))
+
+	newNode := func(name string, labels map[string]string) *corev1.Node {
+		return &corev1.Node{
+			ObjectMeta: metav1.ObjectMeta{Name: name, Labels: labels},
+			Status: corev1.NodeStatus{
+				Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
+				Allocatable: corev1.ResourceList{
+					corev1.ResourceCPU:    resource.MustParse("4"),
+					corev1.ResourceMemory: resource.MustParse("16Gi"),
+				},
+			},
+		}
+	}
+
+	// A stale managed DaemonSet whose group no longer exists -> must be deleted.
+	orphan := &appsv1.DaemonSet{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "node-agent-gone",
+			Namespace: "kubescape",
+			Labels:    map[string]string{ManagedByLabel: ManagedByValue, NodeGroupLabelKey: "gone"},
+		},
+	}
+
+	client := fake.NewClientset(
+		newNode("labelled", map[string]string{"node.kubernetes.io/instance-type": "m5.large"}),
+		newNode("no-label", map[string]string{}),
+		orphan,
+	)
+
+	cfg := config.NodeAgentAutoscalerConfig{
+		Enabled:              true,
+		NodeGroupLabel:       "node.kubernetes.io/instance-type",
+		DefaultNodeGroup:     "default",
+		GoMemLimitPercentage: 0.8,
+		ResourcePercentages:  config.NodeAgentAutoscalerResourcePercentages{RequestCPU: 2, RequestMemory: 2, LimitCPU: 5, LimitMemory: 5},
+		MinResources:         config.NodeAgentAutoscalerResourceBounds{CPU: "100m", Memory: "180Mi"},
+		MaxResources:         config.NodeAgentAutoscalerResourceBounds{CPU: "2000m", Memory: "4Gi"},
+		ReconcileInterval:    5 * time.Minute,
+		TemplatePath:         templatePath,
+	}
+
+	a, err := NewAutoscaler(client, cfg, "kubescape", "")
+	require.NoError(t, err)
+
+	require.NoError(t, a.Reconcile(ctx))
+
+	list, err := client.AppsV1().DaemonSets("kubescape").List(ctx, metav1.ListOptions{})
+	require.NoError(t, err)
+
+	names := map[string]bool{}
+	for _, ds := range list.Items {
+		names[ds.Name] = true
+	}
+	assert.True(t, names["node-agent-m5-large"], "labelled group DaemonSet should exist")
+	assert.True(t, names["node-agent-default"], "default group DaemonSet should exist")
+	assert.False(t, names["node-agent-gone"], "orphaned DaemonSet should be deleted")
+}
diff --git a/nodeagentautoscaler/integration_test.go b/nodeagentautoscaler/integration_test.go
index a5b2b85..29999c3 100644
--- a/nodeagentautoscaler/integration_test.go
+++ b/nodeagentautoscaler/integration_test.go
@@ -29,7 +29,7 @@ func TestIntegration_HelmGeneratedTemplate(t *testing.T) {
 	}
 
 	// Create renderer
-	renderer, err := NewTemplateRenderer(templatePath, 0.8)
+	renderer, err := NewTemplateRenderer(templatePath, 0.8, "node.kubernetes.io/instance-type")
 	require.NoError(t, err)
 
 	// Test data simulating a node group
@@ -89,7 +89,7 @@ func TestIntegration_HelmGeneratedTemplate_DefaultGroup(t *testing.T) {
 		t.Skip("Integration test requires template file. Run Helm extraction first. See test comments for instructions.")
 	}
 
-	renderer, err := NewTemplateRenderer(templatePath, 0.8)
+	renderer, err := NewTemplateRenderer(templatePath, 0.8, "node.kubernetes.io/instance-type")
 	require.NoError(t, err)
 
 	group := NodeGroup{
@@ -107,6 +107,9 @@ func TestIntegration_HelmGeneratedTemplate_DefaultGroup(t *testing.T) {
 
 	assert.Equal(t, "node-agent-default", ds.Name)
 
+	// The OS selector must survive in the default branch (else the DaemonSet could
+	// target unsupported nodes).
+	assert.Equal(t, "linux", ds.Spec.Template.Spec.NodeSelector["kubernetes.io/os"])
 	// Must not pin to an instance-type value (the nodes lack the label).
 	assert.NotContains(t, ds.Spec.Template.Spec.NodeSelector, "node.kubernetes.io/instance-type")
 
diff --git a/nodeagentautoscaler/nodegrouper.go b/nodeagentautoscaler/nodegrouper.go
index 4abf9c9..9d1116d 100644
--- a/nodeagentautoscaler/nodegrouper.go
+++ b/nodeagentautoscaler/nodegrouper.go
@@ -16,6 +16,11 @@ import (
 	"k8s.io/client-go/kubernetes"
 )
 
+// defaultGroupKey is the internal map key for the fallback group of label-less
+// nodes. The leading NUL byte makes it impossible to clash with a real Kubernetes
+// label value, so the fallback population is never merged with a real group.
+const defaultGroupKey = "\x00default-node-group"
+
 // NodeGroup represents a group of nodes with the same label value
 type NodeGroup struct {
 	// LabelValue is the value of the grouping label (e.g., "m5.large")
@@ -80,6 +85,11 @@ func (ng *NodeGrouper) GetNodeGroups(ctx context.Context) ([]NodeGroup, error) {
 
 		labelValue, ok := node.Labels[ng.config.NodeGroupLabel]
 		isDefault := false
+		// groupKey buckets nodes. For labelled nodes it is the label value. For
+		// label-less nodes it is a sentinel that cannot be a valid label value, so the
+		// fallback population is never merged with a real group whose value happens to
+		// equal DefaultNodeGroup (e.g. a node legitimately labelled "...=default").
+		groupKey := labelValue
 		if !ok {
 			// Cloud managed Kubernetes (AKS, EKS, GKE) populate this field automatically.
 			// For on-prem or custom clusters the label may be absent, so fall back to the
@@ -96,16 +106,14 @@ func (ng *NodeGrouper) GetNodeGroups(ctx context.Context) ([]NodeGroup, error) {
 				helpers.String("requiredLabel", ng.config.NodeGroupLabel),
 				helpers.String("defaultNodeGroup", ng.config.DefaultNodeGroup))
 			labelValue = ng.config.DefaultNodeGroup
+			groupKey = defaultGroupKey
 			isDefault = true
 		}
 
-		if group, exists := groupMap[labelValue]; exists {
+		if group, exists := groupMap[groupKey]; exists {
 			group.NodeCount++
-			// A label value can be shared by labelled and label-less nodes; if any
-			// member lacks the label, the group must be targeted via the affinity path.
-			group.IsDefault = group.IsDefault || isDefault
 		} else {
-			groupMap[labelValue] = &NodeGroup{
+			groupMap[groupKey] = &NodeGroup{
 				LabelValue:        labelValue,
 				SanitizedName:     sanitizeName(labelValue),
 				AllocatableCPU:    *node.Status.Allocatable.Cpu(),
@@ -253,8 +261,13 @@ func resolveNameCollisions(groups []NodeGroup) []NodeGroup {
 				helpers.Int("collisionCount", len(indices)))
 
 			for _, idx := range indices {
-				hash := shortHash(groups[idx].LabelValue)
-				groups[idx].SanitizedName = sanitized + "-" + hash
+				// Salt the fallback group so it cannot hash to the same suffix as a
+				// real group sharing its LabelValue (e.g. a node labelled "...=default").
+				hashInput := groups[idx].LabelValue
+				if groups[idx].IsDefault {
+					hashInput = defaultGroupKey + hashInput
+				}
+				groups[idx].SanitizedName = sanitized + "-" + shortHash(hashInput)
 			}
 		}
 	}
diff --git a/nodeagentautoscaler/templaterenderer.go b/nodeagentautoscaler/templaterenderer.go
index fcbf347..f354236 100644
--- a/nodeagentautoscaler/templaterenderer.go
+++ b/nodeagentautoscaler/templaterenderer.go
@@ -23,6 +23,11 @@ type TemplateData struct {
 	Name string
 	// NodeGroupLabel is the value of the node group label
 	NodeGroupLabel string
+	// NodeGroupLabelKey is the configured grouping label key (e.g.
+	// "node.kubernetes.io/instance-type"). The template keys both the nodeSelector
+	// and the default group's affinity off this so they always match the operator's
+	// actual grouping label, even when it is overridden.
+	NodeGroupLabelKey string
 	// IsDefaultGroup is true for the fallback group of nodes that do not carry the
 	// grouping label. The template should select these nodes via a "DoesNotExist"
 	// node affinity rather than a nodeSelector matching NodeGroupLabel.
@@ -53,10 +58,11 @@ type TemplateRenderer struct {
 	watcher              *fsnotify.Watcher
 	stopCh               chan struct{}
 	goMemLimitPercentage float64
+	nodeGroupLabelKey    string // configured grouping label key, exposed to the template
 }
 
 // NewTemplateRenderer creates a new TemplateRenderer
-func NewTemplateRenderer(templatePath string, goMemLimitPercentage float64) (*TemplateRenderer, error) {
+func NewTemplateRenderer(templatePath string, goMemLimitPercentage float64, nodeGroupLabelKey string) (*TemplateRenderer, error) {
 	if goMemLimitPercentage <= 0 || goMemLimitPercentage > 1.0 {
 		return nil, fmt.Errorf("goMemLimitPercentage %v is out of valid range (0, 1.0]", goMemLimitPercentage)
 	}
@@ -64,6 +70,7 @@ func NewTemplateRenderer(templatePath string, goMemLimitPercentage float64) (*Te
 	tr := &TemplateRenderer{
 		templatePath:         templatePath,
 		goMemLimitPercentage: goMemLimitPercentage,
+		nodeGroupLabelKey:    nodeGroupLabelKey,
 	}
 
 	if err := tr.loadTemplate(); err != nil {
@@ -216,9 +223,10 @@ func (tr *TemplateRenderer) RenderDaemonSet(group NodeGroup, resources Calculate
 	goMemLimitMiB := int64(float64(limitBytes) * tr.goMemLimitPercentage / (1024 * 1024))
 
 	data := TemplateData{
-		Name:           fmt.Sprintf("node-agent-%s", group.SanitizedName),
-		NodeGroupLabel: group.LabelValue,
-		IsDefaultGroup: group.IsDefault,
+		Name:              fmt.Sprintf("node-agent-%s", group.SanitizedName),
+		NodeGroupLabel:    group.LabelValue,
+		NodeGroupLabelKey: tr.nodeGroupLabelKey,
+		IsDefaultGroup:    group.IsDefault,
 		Resources: TemplateResources{
 			Requests: TemplateResourcePair{
 				CPU:    resources.Requests.CPU.String(),
diff --git a/nodeagentautoscaler/templaterenderer_test.go b/nodeagentautoscaler/templaterenderer_test.go
index b40d0ce..109fdbe 100644
--- a/nodeagentautoscaler/templaterenderer_test.go
+++ b/nodeagentautoscaler/templaterenderer_test.go
@@ -105,14 +105,14 @@ spec:
           requiredDuringSchedulingIgnoredDuringExecution:
             nodeSelectorTerms:
             - matchExpressions:
-              - key: node.kubernetes.io/instance-type
+              - key: {{ .NodeGroupLabelKey }}
                 operator: DoesNotExist
       nodeSelector:
         kubernetes.io/os: linux
 {{- else }}
       nodeSelector:
         kubernetes.io/os: linux
-        node.kubernetes.io/instance-type: "{{ .NodeGroupLabel }}"
+        {{ .NodeGroupLabelKey }}: "{{ .NodeGroupLabel }}"
 {{- end }}
 `
 
@@ -123,7 +123,7 @@ spec:
 	require.NoError(t, err)
 
 	// Create renderer
-	renderer, err := NewTemplateRenderer(templatePath, 0.8)
+	renderer, err := NewTemplateRenderer(templatePath, 0.8, "node.kubernetes.io/instance-type")
 	require.NoError(t, err)
 
 	// Test data
@@ -200,14 +200,14 @@ spec:
           requiredDuringSchedulingIgnoredDuringExecution:
             nodeSelectorTerms:
             - matchExpressions:
-              - key: node.kubernetes.io/instance-type
+              - key: {{ .NodeGroupLabelKey }}
                 operator: DoesNotExist
       nodeSelector:
         kubernetes.io/os: linux
 {{- else }}
       nodeSelector:
         kubernetes.io/os: linux
-        node.kubernetes.io/instance-type: "{{ .NodeGroupLabel }}"
+        {{ .NodeGroupLabelKey }}: "{{ .NodeGroupLabel }}"
 {{- end }}
 `
 
@@ -215,7 +215,7 @@ spec:
 	templatePath := filepath.Join(tmpDir, "daemonset-template.yaml")
 	require.NoError(t, os.WriteFile(templatePath, []byte(templateContent), 0644))
 
-	renderer, err := NewTemplateRenderer(templatePath, 0.8)
+	renderer, err := NewTemplateRenderer(templatePath, 0.8, "node.kubernetes.io/instance-type")
 	require.NoError(t, err)
 
 	group := NodeGroup{
@@ -257,7 +257,7 @@ func TestTemplateRenderer_RenderDaemonSet_InvalidTemplate(t *testing.T) {
 	require.NoError(t, err)
 
 	// Should fail to create renderer
-	_, err = NewTemplateRenderer(templatePath, 0.8)
+	_, err = NewTemplateRenderer(templatePath, 0.8, "node.kubernetes.io/instance-type")
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "failed to parse template")
 }
@@ -279,14 +279,14 @@ func TestTemplateRenderer_NewTemplateRenderer_InvalidPercentage(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			_, err := NewTemplateRenderer(templatePath, tt.percentage)
+			_, err := NewTemplateRenderer(templatePath, tt.percentage, "node.kubernetes.io/instance-type")
 			assert.Error(t, err)
 			assert.Contains(t, err.Error(), "out of valid range")
 		})
 	}
 
 	// Boundary: exactly 1.0 should be valid
-	_, err = NewTemplateRenderer(templatePath, 1.0)
+	_, err = NewTemplateRenderer(templatePath, 1.0, "node.kubernetes.io/instance-type")
 	assert.NoError(t, err)
 }
 
@@ -350,7 +350,7 @@ spec:
 	require.NoError(t, err)
 
 	// Create renderer
-	renderer, err := NewTemplateRenderer(templatePath, 0.8)
+	renderer, err := NewTemplateRenderer(templatePath, 0.8, "node.kubernetes.io/instance-type")
 	require.NoError(t, err)
 
 	group := NodeGroup{
@@ -456,7 +456,7 @@ spec:
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			renderer, err := NewTemplateRenderer(templatePath, tt.percentage)
+			renderer, err := NewTemplateRenderer(templatePath, tt.percentage, "node.kubernetes.io/instance-type")
 			require.NoError(t, err)
 
 			group := NodeGroup{LabelValue: "m5.large", SanitizedName: "m5-large"}

From a96a01c07aeb6df57ee2745f9b547cf5cd598540 Mon Sep 17 00:00:00 2001
From: Matthias Bertschy <matthias.bertschy@gmail.com>
Date: Mon, 29 Jun 2026 14:05:19 +0200
Subject: [PATCH 3/4] ci: build chart dependencies before extracting daemonset
 template

The integration-build job rendered the chart from a fresh helm-charts
checkout, but the chart has local file:// subchart dependencies
(kubescape-alert-crd, kubescape-alert-crd-ns) that must be vendored first.
Run `helm dependency build` before `helm template` so the extraction
produces a non-empty template instead of failing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
---
 .github/workflows/pr-created.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.github/workflows/pr-created.yaml b/.github/workflows/pr-created.yaml
index f6a4fab..d93ed9d 100644
--- a/.github/workflows/pr-created.yaml
+++ b/.github/workflows/pr-created.yaml
@@ -44,6 +44,9 @@ jobs:
       - uses: azure/setup-helm@v4
       - name: Extract node-agent daemonset template
         run: |
+          # The chart has local (file://) subchart dependencies that must be
+          # vendored into charts/ before templating a fresh checkout.
+          helm dependency build helm-charts/charts/kubescape-operator
           helm template test helm-charts/charts/kubescape-operator \
             --kube-version 1.29.0 \
             --set nodeAgent.autoscaler.enabled=true --set clusterName=test \

From 99a27e95ce62fee9ac939197094bf0bcd829fa3b Mon Sep 17 00:00:00 2001
From: Matthias Bertschy <matthias.bertschy@gmail.com>
Date: Tue, 30 Jun 2026 07:52:42 +0200
Subject: [PATCH 4/4] fix(autoscaler): size default group off minimum
 allocatable

Address slashben's review on #389: the default (label-less) group can be
heterogeneous (mixed on-prem/bare-metal hardware), so sizing it off the
first-listed node is unsafe and order-dependent. Size it off the minimum
allocatable CPU/memory across its nodes instead, so the node-agent's requests
fit every node in the group regardless of node list order. Instance-type
groups are homogeneous and keep first-node sizing.

Also document the rare shared-selector edge case (a node legitimately labelled
<groupingLabel>=<defaultNodeGroup>): targeting is mutually exclusive and
ownerReferences prevent pod adoption, so it is safe; noted for operators.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
---
 docs/node-agent-autoscaler.md          | 23 +++++++++++++
 nodeagentautoscaler/autoscaler_test.go | 46 ++++++++++++++++++++++++++
 nodeagentautoscaler/nodegrouper.go     | 13 ++++++++
 3 files changed, 82 insertions(+)

diff --git a/docs/node-agent-autoscaler.md b/docs/node-agent-autoscaler.md
index 7c37733..bb978b8 100644
--- a/docs/node-agent-autoscaler.md
+++ b/docs/node-agent-autoscaler.md
@@ -107,11 +107,25 @@ Each DaemonSet selects the nodes it runs on based on the grouping label:
   so they always match the label the operator actually groups by — even when it is
   overridden from the default `node.kubernetes.io/instance-type`.
 
+The default group affinity carries **only** the `DoesNotExist` term; its OS
+requirement is already enforced by the `nodeSelector`. Non-default groups are not
+given an operator-managed affinity at all, so any user-provided `nodeAgent.affinity`
+(zone / GPU / topology constraints) is preserved for them.
+
 The label-less population is tracked under an internal sentinel key, so a node
 that legitimately carries `<groupingLabel>=<defaultNodeGroup>` forms its own
 normal (nodeSelector-targeted) group and is never merged into the affinity-based
 default group.
 
+> **Edge case — shared `kubescape.io/node-group` value.** In the unlikely event a
+> node is *labelled* `<groupingLabel>=<defaultNodeGroup>` while other nodes lack
+> the label, the real group and the synthetic default group both render their
+> `kubescape.io/node-group` selector value as `<defaultNodeGroup>`, so the two
+> DaemonSets share a pod selector. This is safe in practice — their node targeting
+> is mutually exclusive (label-present vs. absent) and `ownerReferences` prevent
+> pod adoption across them — but `kubectl get pods -l kubescape.io/node-group=…`
+> will list both groups' pods together.
+
 ### NodeGrouper (`nodegrouper.go`)
 
 Handles node discovery and resource calculation.
@@ -142,6 +156,15 @@ func calculatePercentage(q resource.Quantity, percent int) resource.Quantity {
 }
 ```
 
+**Representative allocatable per group:**
+
+Instance-type groups are homogeneous by definition, so a group's allocatable is
+taken from the first node seen. The **default group** is the exception: label-less
+nodes can be heterogeneous (mixed on-prem/bare-metal hardware), so it is sized off
+the **minimum** allocatable CPU/memory across its nodes. This guarantees the
+node-agent's requests fit every node in the group and makes the result independent
+of the (unstable) node list order.
+
 **Naming Collision Detection:**
 
 Different label values can sanitize to the same name (e.g., `m5.large` and `m5_large` both become `m5-large`). The autoscaler detects this and adds a short hash suffix:
diff --git a/nodeagentautoscaler/autoscaler_test.go b/nodeagentautoscaler/autoscaler_test.go
index b115ad1..92aa0d9 100644
--- a/nodeagentautoscaler/autoscaler_test.go
+++ b/nodeagentautoscaler/autoscaler_test.go
@@ -431,6 +431,52 @@ func TestNodeGrouper_GetNodeGroups_DefaultValueCollision(t *testing.T) {
 	assert.NotEqual(t, labelled.SanitizedName, fallback.SanitizedName)
 }
 
+// TestNodeGrouper_GetNodeGroups_DefaultGroupSizesOffMinimum verifies the
+// heterogeneous default group is sized from the minimum allocatable across its
+// nodes (a safe lower bound), not the arbitrary first-listed node.
+func TestNodeGrouper_GetNodeGroups_DefaultGroupSizesOffMinimum(t *testing.T) {
+	ctx := context.Background()
+
+	newNode := func(name, cpu, mem string) *corev1.Node {
+		return &corev1.Node{
+			ObjectMeta: metav1.ObjectMeta{Name: name, Labels: map[string]string{}},
+			Status: corev1.NodeStatus{
+				Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}},
+				Allocatable: corev1.ResourceList{
+					corev1.ResourceCPU:    resource.MustParse(cpu),
+					corev1.ResourceMemory: resource.MustParse(mem),
+				},
+			},
+		}
+	}
+
+	// Big node first, small node last: first-node sizing would pick 64 cores;
+	// minimum sizing must pick the 2-core / 4Gi box.
+	nodes := []runtime.Object{
+		newNode("big", "64", "256Gi"),
+		newNode("small", "2", "4Gi"),
+	}
+
+	client := fake.NewClientset(nodes...)
+	cfg := config.NodeAgentAutoscalerConfig{
+		Enabled:          true,
+		NodeGroupLabel:   "node.kubernetes.io/instance-type",
+		DefaultNodeGroup: "default",
+	}
+
+	groups, err := NewNodeGrouper(client, cfg, "kubescape").GetNodeGroups(ctx)
+	require.NoError(t, err)
+	require.Len(t, groups, 1)
+
+	g := groups[0]
+	require.True(t, g.IsDefault)
+	assert.Equal(t, 2, g.NodeCount)
+	minCPU := resource.MustParse("2")
+	minMem := resource.MustParse("4Gi")
+	assert.Equal(t, minCPU.MilliValue(), g.AllocatableCPU.MilliValue(), "CPU should be the group minimum")
+	assert.Equal(t, minMem.Value(), g.AllocatableMemory.Value(), "memory should be the group minimum")
+}
+
 func TestNodeGrouper_CalculateResources(t *testing.T) {
 	cfg := config.NodeAgentAutoscalerConfig{
 		ResourcePercentages: config.NodeAgentAutoscalerResourcePercentages{
diff --git a/nodeagentautoscaler/nodegrouper.go b/nodeagentautoscaler/nodegrouper.go
index 9d1116d..292d3eb 100644
--- a/nodeagentautoscaler/nodegrouper.go
+++ b/nodeagentautoscaler/nodegrouper.go
@@ -112,6 +112,19 @@ func (ng *NodeGrouper) GetNodeGroups(ctx context.Context) ([]NodeGroup, error) {
 
 		if group, exists := groupMap[groupKey]; exists {
 			group.NodeCount++
+			if group.IsDefault {
+				// Instance-type groups are homogeneous, so the first node is
+				// representative. The default group is the opposite: label-less nodes
+				// can be wildly heterogeneous (on-prem mixed hardware). Size it off the
+				// minimum allocatable across its nodes so requests fit every node and
+				// the result is independent of (unstable) node list order.
+				if nodeCPU := *node.Status.Allocatable.Cpu(); nodeCPU.Cmp(group.AllocatableCPU) < 0 {
+					group.AllocatableCPU = nodeCPU
+				}
+				if nodeMem := *node.Status.Allocatable.Memory(); nodeMem.Cmp(group.AllocatableMemory) < 0 {
+					group.AllocatableMemory = nodeMem
+				}
+			}
 		} else {
 			groupMap[groupKey] = &NodeGroup{
 				LabelValue:        labelValue,