From 228bd4ef237821ec65a26249bbde16c60c088f78 Mon Sep 17 00:00:00 2001 From: Maksim Paskal Date: Wed, 17 Apr 2024 08:14:06 +0100 Subject: [PATCH] Fix Autoscaling for worker nodes with invalid ProviderID This change fixes a bug that arises when the user's cluster includes worker nodes not from Hetzner Cloud, such as a Hetzner Dedicated server or any server resource other than Hetzner. It also corrects the behavior when a server has been physically deleted from Hetzner Cloud. Signed-off-by: Maksim Paskal --- .../cloudprovider/hetzner/hetzner_manager.go | 10 ++++++++++ .../cloudprovider/hetzner/hetzner_servers_cache.go | 4 ++-- .../hetzner/hetzner_servers_cache_test.go | 5 +++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/hetzner/hetzner_manager.go b/cluster-autoscaler/cloudprovider/hetzner/hetzner_manager.go index f75c69ee3df5..9cf165ed7f80 100644 --- a/cluster-autoscaler/cloudprovider/hetzner/hetzner_manager.go +++ b/cluster-autoscaler/cloudprovider/hetzner/hetzner_manager.go @@ -256,9 +256,19 @@ func (m *hetznerManager) addNodeToDrainingPool(node *apiv1.Node) (*hetznerNodeGr return m.nodeGroups[drainingNodePoolId], nil } +func (m *hetznerManager) validProviderID(providerID string) bool { + return strings.HasPrefix(providerID, providerIDPrefix) +} + func (m *hetznerManager) serverForNode(node *apiv1.Node) (*hcloud.Server, error) { var nodeIdOrName string if node.Spec.ProviderID != "" { + if !m.validProviderID(node.Spec.ProviderID) { + // This cluster-autoscaler provider only handles Hetzner Cloud servers. + // Any other provider ID prefix is invalid, and we return no server. Returning an error here breaks hybrid + // clusters with nodes from Hetzner Cloud & Robot (or other providers). + return nil, nil + } nodeIdOrName = strings.TrimPrefix(node.Spec.ProviderID, providerIDPrefix) } else { nodeIdOrName = node.Name diff --git a/cluster-autoscaler/cloudprovider/hetzner/hetzner_servers_cache.go b/cluster-autoscaler/cloudprovider/hetzner/hetzner_servers_cache.go index 04a2e964d6ff..36090d41d467 100644 --- a/cluster-autoscaler/cloudprovider/hetzner/hetzner_servers_cache.go +++ b/cluster-autoscaler/cloudprovider/hetzner/hetzner_servers_cache.go @@ -18,7 +18,6 @@ package hetzner import ( "context" - "errors" "strconv" "sync" "time" @@ -136,7 +135,8 @@ func (m *serversCache) getServer(nodeIdOrName string) (*hcloud.Server, error) { } } - return nil, errors.New("server not found") + // return nil if server not found + return nil, nil } func (m *serversCache) getServersByNodeGroupName(nodeGroup string) ([]*hcloud.Server, error) { diff --git a/cluster-autoscaler/cloudprovider/hetzner/hetzner_servers_cache_test.go b/cluster-autoscaler/cloudprovider/hetzner/hetzner_servers_cache_test.go index 629365dd45c1..c0cc7ec5a540 100644 --- a/cluster-autoscaler/cloudprovider/hetzner/hetzner_servers_cache_test.go +++ b/cluster-autoscaler/cloudprovider/hetzner/hetzner_servers_cache_test.go @@ -70,6 +70,7 @@ func TestServersCache(t *testing.T) { require.NoError(t, err) assert.Equal(t, "test2", foundservers.Name) - _, err = c.getServer("test3") - require.Error(t, err) + server, err := c.getServer("test3") + require.Nil(t, server) + require.NoError(t, err) }