Merge pull request #8297 from pat-s/feat/imagesForArch-nodepool

Hetzner(feat): add option to set nodepool-specific image IDs

Author: k8s-ci-robot

cluster-autoscaler/cloudprovider/hetzner/README.md

@@ -10,16 +10,16 @@ The cluster autoscaler for Hetzner Cloud scales worker nodes.
 
 `HCLOUD_IMAGE` Defaults to `ubuntu-20.04`, @see https://docs.hetzner.cloud/#images. You can also use an image ID here (e.g. `15512617`), or a label selector associated with a custom snapshot (e.g. `customized_ubuntu=true`). The most recent snapshot will be used in the latter case.
 
-`HCLOUD_CLUSTER_CONFIG` This is the new format replacing 
- * `HCLOUD_CLOUD_INIT` 
- * `HCLOUD_IMAGE` 
- 
+`HCLOUD_CLUSTER_CONFIG` This is the new format replacing
+ * `HCLOUD_CLOUD_INIT`
+ * `HCLOUD_IMAGE`
+
  Base64 encoded JSON according to the following structure
 
 ```json
 {
     "imagesForArch": { // These should be the same format as HCLOUD_IMAGE
-        "arm64": "", 
+        "arm64": "",
         "amd64": ""
     },
     "nodeConfigs": {
@@ -28,7 +28,7 @@ The cluster autoscaler for Hetzner Cloud scales worker nodes.
             "labels": {
                 "node.kubernetes.io/role": "autoscaler-node"
             },
-            "taints": 
+            "taints":
             [
                 {
                     "key": "node.kubernetes.io/role",
@@ -47,6 +47,13 @@ Can be useful when you have many different node pools and run into issues of the
 
 **NOTE**: In contrast to `HCLOUD_CLUSTER_CONFIG`, this file is not base64 encoded.
 
+The global `imagesForArch` configuration can be overridden on a per-nodepool basis by adding an `imagesForArch` field to individual nodepool configurations.
+
+The image selection logic works as follows:
+
+1. If a nodepool has its own `imagesForArch` configuration, it will be used for that specific nodepool
+1. If a nodepool doesn't have `imagesForArch` configured, the global `imagesForArch` configuration will be used as a fallback
+1. If neither is configured, the legacy `HCLOUD_IMAGE` environment variable will be used
 
 `HCLOUD_NETWORK` Default empty , The id or name of the network that is used in the cluster , @see https://docs.hetzner.cloud/#networks
 
@@ -105,5 +112,5 @@ git add hcloud-go/
 
 ## Debugging
 
-To enable debug logging, set the log level of the autoscaler to at least level 5 via cli flag: `--v=5`  
-The logs will include all requests and responses made towards the Hetzner API including headers and body.
+To enable debug logging, set the log level of the autoscaler to at least level 5 via cli flag: `--v=5`
+The logs will include all requests and responses made towards the Hetzner API including headers and body.

cluster-autoscaler/cloudprovider/hetzner/hetzner_manager.go

@@ -77,6 +77,7 @@ type NodeConfig struct {
 	PlacementGroup string
 	Taints         []apiv1.Taint
 	Labels         map[string]string
+	ImagesForArch  *ImageList
 }
 
 // LegacyConfig holds the configuration in the legacy format

cluster-autoscaler/cloudprovider/hetzner/hetzner_node_group.go

@@ -528,12 +528,20 @@ func findImage(n *hetznerNodeGroup, serverType *hcloud.ServerType) (*hcloud.Imag
 	// Select correct image based on server type architecture
 	imageName := n.manager.clusterConfig.LegacyConfig.ImageName
 	if n.manager.clusterConfig.IsUsingNewFormat {
+		// Check for nodepool-specific images first, then fall back to global images
+		var imagesForArch *ImageList
+		if nodeConfig, exists := n.manager.clusterConfig.NodeConfigs[n.id]; exists && nodeConfig.ImagesForArch != nil {
+			imagesForArch = nodeConfig.ImagesForArch
+		} else {
+			imagesForArch = &n.manager.clusterConfig.ImagesForArch
+		}
+
 		if serverType.Architecture == hcloud.ArchitectureARM {
-			imageName = n.manager.clusterConfig.ImagesForArch.Arm64
+			imageName = imagesForArch.Arm64
 		}
 
 		if serverType.Architecture == hcloud.ArchitectureX86 {
-			imageName = n.manager.clusterConfig.ImagesForArch.Amd64
+			imageName = imagesForArch.Amd64
 		}
 	}
 

cluster-autoscaler/cloudprovider/hetzner/hetzner_node_group_test.go

@@ -0,0 +1,174 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package hetzner
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFindImageWithPerNodepoolConfig(t *testing.T) {
+	// Test case 1: Nodepool with specific imagesForArch should use those images
+	t.Run("nodepool with specific imagesForArch", func(t *testing.T) {
+		manager := &hetznerManager{
+			clusterConfig: &ClusterConfig{
+				IsUsingNewFormat: true,
+				ImagesForArch: ImageList{
+					Arm64: "global-arm64-image",
+					Amd64: "global-amd64-image",
+				},
+				NodeConfigs: map[string]*NodeConfig{
+					"pool1": {
+						ImagesForArch: &ImageList{
+							Arm64: "pool1-arm64-image",
+							Amd64: "pool1-amd64-image",
+						},
+					},
+				},
+			},
+		}
+
+		nodeGroup := &hetznerNodeGroup{
+			id:      "pool1",
+			manager: manager,
+		}
+
+		// This would normally call the actual API, but we're just testing the logic
+		// The actual image selection logic is in findImage function
+		// For this test, we'll verify the configuration is set up correctly
+		nodeConfig, exists := manager.clusterConfig.NodeConfigs[nodeGroup.id]
+		require.True(t, exists)
+		require.NotNil(t, nodeConfig.ImagesForArch)
+		assert.Equal(t, "pool1-arm64-image", nodeConfig.ImagesForArch.Arm64)
+		assert.Equal(t, "pool1-amd64-image", nodeConfig.ImagesForArch.Amd64)
+	})
+
+	// Test case 2: Nodepool without specific imagesForArch should fall back to global
+	t.Run("nodepool without specific imagesForArch", func(t *testing.T) {
+		manager := &hetznerManager{
+			clusterConfig: &ClusterConfig{
+				IsUsingNewFormat: true,
+				ImagesForArch: ImageList{
+					Arm64: "global-arm64-image",
+					Amd64: "global-amd64-image",
+				},
+				NodeConfigs: map[string]*NodeConfig{
+					"pool2": {
+						// No ImagesForArch specified
+					},
+				},
+			},
+		}
+
+		nodeGroup := &hetznerNodeGroup{
+			id:      "pool2",
+			manager: manager,
+		}
+
+		nodeConfig, exists := manager.clusterConfig.NodeConfigs[nodeGroup.id]
+		require.True(t, exists)
+		assert.Nil(t, nodeConfig.ImagesForArch)
+		assert.Equal(t, "global-arm64-image", manager.clusterConfig.ImagesForArch.Arm64)
+		assert.Equal(t, "global-amd64-image", manager.clusterConfig.ImagesForArch.Amd64)
+	})
+
+	// Test case 3: Nodepool with nil ImagesForArch should fall back to global
+	t.Run("nodepool with nil imagesForArch", func(t *testing.T) {
+		manager := &hetznerManager{
+			clusterConfig: &ClusterConfig{
+				IsUsingNewFormat: true,
+				ImagesForArch: ImageList{
+					Arm64: "global-arm64-image",
+					Amd64: "global-amd64-image",
+				},
+				NodeConfigs: map[string]*NodeConfig{
+					"pool3": {
+						ImagesForArch: nil, // Explicitly nil
+					},
+				},
+			},
+		}
+
+		nodeGroup := &hetznerNodeGroup{
+			id:      "pool3",
+			manager: manager,
+		}
+
+		nodeConfig, exists := manager.clusterConfig.NodeConfigs[nodeGroup.id]
+		require.True(t, exists)
+		assert.Nil(t, nodeConfig.ImagesForArch)
+		assert.Equal(t, "global-arm64-image", manager.clusterConfig.ImagesForArch.Arm64)
+		assert.Equal(t, "global-amd64-image", manager.clusterConfig.ImagesForArch.Amd64)
+	})
+}
+
+func TestImageSelectionLogic(t *testing.T) {
+	// Test the image selection logic that would be used in findImage function
+	t.Run("image selection logic", func(t *testing.T) {
+		manager := &hetznerManager{
+			clusterConfig: &ClusterConfig{
+				IsUsingNewFormat: true,
+				ImagesForArch: ImageList{
+					Arm64: "global-arm64-image",
+					Amd64: "global-amd64-image",
+				},
+				NodeConfigs: map[string]*NodeConfig{
+					"pool1": {
+						ImagesForArch: &ImageList{
+							Arm64: "pool1-arm64-image",
+							Amd64: "pool1-amd64-image",
+						},
+					},
+					"pool2": {
+						// No ImagesForArch specified
+					},
+				},
+			},
+		}
+
+		// Test pool1 (has specific imagesForArch)
+		nodeConfig, exists := manager.clusterConfig.NodeConfigs["pool1"]
+		require.True(t, exists)
+		require.NotNil(t, nodeConfig.ImagesForArch)
+
+		var imagesForArch *ImageList
+		if nodeConfig.ImagesForArch != nil {
+			imagesForArch = nodeConfig.ImagesForArch
+		} else {
+			imagesForArch = &manager.clusterConfig.ImagesForArch
+		}
+
+		assert.Equal(t, "pool1-arm64-image", imagesForArch.Arm64)
+		assert.Equal(t, "pool1-amd64-image", imagesForArch.Amd64)
+
+		// Test pool2 (no specific imagesForArch, should use global)
+		nodeConfig, exists = manager.clusterConfig.NodeConfigs["pool2"]
+		require.True(t, exists)
+		assert.Nil(t, nodeConfig.ImagesForArch)
+
+		if nodeConfig.ImagesForArch != nil {
+			imagesForArch = nodeConfig.ImagesForArch
+		} else {
+			imagesForArch = &manager.clusterConfig.ImagesForArch
+		}
+
+		assert.Equal(t, "global-arm64-image", imagesForArch.Arm64)
+		assert.Equal(t, "global-amd64-image", imagesForArch.Amd64)
+	})
+}