docs: differing Robot server name and Node name not supported (#1170)

In the [docs we
state](https://github.com/hetznercloud/hcloud-cloud-controller-manager/blob/main/docs/explanation/robot-support.md#identifying-the-correct-serve):

> If you absolutely need to use different names in Robot & Hostname, you
can also configure the Provider ID yourself. This can be done on the
kubelet through the flag
[--provider-id](https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/).
You need to follow the format hrobot://$SERVER_NUMBER when setting this.
If this format is not followed exactly we can not process this node.

This is actually not supported and changing this would be a breaking change.

Author: lukasmetzner

docs/explanation/robot-support.md

@@ -50,8 +50,6 @@ When a new Node joins the cluster, we first need to figure out which Robot (or C
 
 _This means that by default, your **Hostname** needs to be the **name of the server in Robot**_. If this does not match, we can not properly match the two entities. Once we have made this connection, we save the Robot Server Number to the field `spec.providerId` on the Node, and use this identifier for any further processing.
 
-If you absolutely need to use different names in Robot & Hostname, you can also configure the Provider ID yourself. This can be done on the `kubelet` through the flag [`--provider-id`](https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/). You need to follow the format `hrobot://$SERVER_NUMBER` when setting this. If this format is not followed exactly we can not process this node.
-
 ## Credentials
 
 Robot API credentials (`ROBOT_USER` / `ROBOT_PASSWORD`) are **optional**. They control which features are available:

hcloud/instances_util.go

@@ -111,7 +111,9 @@ func getRobotServerByID(i *instances, id int, node *corev1.Node) (*hrobotmodels.
 		return nil, nil
 	}
 
-	// check whether name matches - otherwise this server does not belong to the respective node anymore
+	// CAPH reuses Robot servers for multiple clusters and therefore the Robot ID does not change, but only
+	// the name in the Robot API is updated. As the node no longer exists in the cluster with the old name,
+	// we need to return nil here.
 	if server.Name != node.Name {
 		i.recorder.Eventf(
 			node,
@@ -124,7 +126,6 @@ func getRobotServerByID(i *instances, id int, node *corev1.Node) (*hrobotmodels.
 		return nil, nil
 	}
 
-	// return nil, nil if server could not be found
 	return server, nil
 }
 

hcloud/instances_util_test.go

@@ -0,0 +1,65 @@
+package hcloud
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	hrobotmodels "github.com/syself/hrobot-go/models"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/tools/record"
+
+	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks"
+)
+
+func TestGetRobotServerByID(t *testing.T) {
+	tests := []struct {
+		name          string
+		nodeName      string
+		expectedEvent string
+	}{
+		{
+			name:     "no diff robot and node name",
+			nodeName: "foobar",
+		},
+		{
+			name:          "diff robot and node name",
+			nodeName:      "barfoo",
+			expectedEvent: `Warning PossibleNodeDeletion Might be deleted by node-lifecycle-manager due to name mismatch; Node name "barfoo" differs from Robot name "foobar"`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			recorder := record.NewFakeRecorder(1)
+
+			robotClientMock := &mocks.RobotClient{}
+			robotClientMock.Test(t)
+			robotClientMock.On("ServerGet").Return(&hrobotmodels.Server{ServerNumber: 1, Name: "foobar"}, nil)
+
+			inst := &instances{
+				recorder:    recorder,
+				robotClient: robotClientMock,
+			}
+
+			node := &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: tt.nodeName,
+				},
+			}
+
+			server, err := getRobotServerByID(inst, 1, node)
+			require.NoError(t, err)
+
+			if tt.expectedEvent != "" {
+				require.Nil(t, server)
+				event := <-recorder.Events
+				assert.Equal(t, tt.expectedEvent, event)
+			} else {
+				assert.Equal(t, "foobar", server.Name)
+				assert.Empty(t, recorder.Events)
+			}
+		})
+	}
+}

internal/mocks/casts.go

@@ -39,6 +39,14 @@ func getLoadBalancerPtrS(args mock.Arguments, i int) []*hcloud.LoadBalancer {
 	return v.([]*hcloud.LoadBalancer)
 }
 
+func getRobotServer(args mock.Arguments, i int) *hrobotmodels.Server {
+	v := args.Get(i)
+	if v == nil {
+		return nil
+	}
+	return v.(*hrobotmodels.Server)
+}
+
 func getRobotServers(args mock.Arguments, i int) []hrobotmodels.Server {
 	v := args.Get(i)
 	if v == nil {

internal/mocks/robot.go

@@ -9,6 +9,11 @@ type RobotClient struct {
 	mock.Mock
 }
 
+func (m *RobotClient) ServerGet(id int) (*hrobotmodels.Server, error) {
+	args := m.Called()
+	return getRobotServer(args, 0), args.Error(1)
+}
+
 func (m *RobotClient) ServerGetList() ([]hrobotmodels.Server, error) {
 	args := m.Called()
 	return getRobotServers(args, 0), args.Error(1)
@@ -62,9 +67,6 @@ func (m *RobotClient) ResetGet(id int) (*hrobotmodels.Reset, error) {
 func (m *RobotClient) ResetSet(id int, input *hrobotmodels.ResetSetInput) (*hrobotmodels.ResetPost, error) {
 	panic("this method should not be called")
 }
-func (m *RobotClient) ServerGet(id int) (*hrobotmodels.Server, error) {
-	panic("this method should not be called")
-}
 func (m *RobotClient) ServerReverse(id int) (*hrobotmodels.Cancellation, error) {
 	panic("this method should not be called")
 }