🌱 Support hrobot:// ProviderID of upstream ccm.

README.md

@@ -9,7 +9,14 @@ The Hetzner Cloud controller manager seamlessly integrates your Kubernetes clust
 
 ## About the Fork
 
-In the long run, we (Syself) would like to switch to the [upstream ccm](https://github.com/syself/hetzner-cloud-controller-manager/) again.
+We plan to support the [upstream hcloud
+ccm](https://github.com/hetznercloud/hcloud-cloud-controller-manager/) in
+[CAPH](https://github.com/syself/cluster-api-provider-hetzner/). After that, this fork is no longer
+needed.
+
+## About the Fork (old)
+
+In the long run, we (Syself) would like to switch to the [upstream ccm](https://github.com/hetznercloud/hcloud-cloud-controller-manager/) again.
 
 A lot of changes were made in the upstream fork, and we don't plan to merge them into our fork.
 
@@ -62,9 +69,12 @@ See [CAPH docs](https://syself.com/docs/caph/topics/baremetal/creating-workload-
 
 ## Usage
 
-We recommend to mount the secret `hetzner` as volume and make it avaiable for the container as `/etc/hetzner-secret`.
-Then the credentials are automatically reloaded, when the secret changes.
-You see an example in the [ccm helm chart](https://github.com/syself/charts/tree/main/charts/ccm-hetzner)
+We recommend to mount the secret `hetzner` as volume and make it avaiable for the container as
+`/etc/hetzner-secret`. Then the credentials are automatically reloaded, when the secret changes.
+When you use the hot-reloading, be sure that the keys in the secret use these names: "hcloud" (for
+the HCLOUD_TOKEN), "robot-user" and "robot-password". For compatibility with the upstream hcloud-ccm
+we support the key "token" instead of "hcloud". You see an example in the [ccm helm
+chart](https://github.com/syself/charts/tree/main/charts/ccm-hetzner)
 
 ## Env Variables
 

hcloud/cloud.go

@@ -65,6 +65,7 @@ const (
 	hcloudLoadBalancersUsePrivateIP          = "HCLOUD_LOAD_BALANCERS_USE_PRIVATE_IP"
 	hcloudLoadBalancersDisableIPv6           = "HCLOUD_LOAD_BALANCERS_DISABLE_IPV6"
 	hcloudMetricsEnabledENVVar               = "HCLOUD_METRICS_ENABLED"
+	UseHrobotProviderIDForBaremetalEnvVar    = "HCLOUD_USE_HROBOT_PROVIDER_ID_FOR_BAREMETAL"
 	hcloudMetricsAddress                     = ":8233"
 	providerName                             = "hcloud"
 	hostNamePrefixRobot                      = "bm-"
@@ -247,6 +248,10 @@ func newCloud(_ io.Reader) (cloudprovider.Interface, error) {
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", op, err)
 	}
+	useHrobotProviderIDForBaremetal, err := getEnvBool(UseHrobotProviderIDForBaremetalEnvVar)
+	if err != nil {
+		return nil, fmt.Errorf("%s: %w", op, err)
+	}
 
 	credentialsDir := credentials.GetDirectory(rootDir)
 	_, err = os.Stat(credentialsDir)
@@ -261,7 +266,7 @@ func newCloud(_ io.Reader) (cloudprovider.Interface, error) {
 	return &cloud{
 		hcloudClient: hcloudClient,
 		robotClient:  robotClient,
-		instances:    newInstances(hcloudClient, robotClient, instancesAddressFamily, networkID),
+		instances:    newInstances(hcloudClient, robotClient, instancesAddressFamily, networkID, useHrobotProviderIDForBaremetal),
 		loadBalancer: loadBalancers,
 		routes:       nil,
 		networkID:    networkID,

hcloud/cloud_test.go

@@ -436,7 +436,7 @@ func Test_updateHcloudCredentials(t *testing.T) {
 			},
 		})
 	})
-	i := newInstances(hcloudClient, nil, AddressFamilyIPv4, 0)
+	i := newInstances(hcloudClient, nil, AddressFamilyIPv4, 0, false)
 
 	node := &corev1.Node{
 		Spec: corev1.NodeSpec{ProviderID: "hcloud://1"},

hcloud/instances.go

@@ -39,16 +39,29 @@ const (
 )
 
 type instances struct {
-	client        *hcloud.Client
-	robotClient   robotclient.Client
-	addressFamily addressFamily
-	networkID     int64
+	client              *hcloud.Client
+	robotClient         robotclient.Client
+	addressFamily       addressFamily
+	networkID           int64
+	useHrobotProviderID bool
 }
 
 var errServerNotFound = fmt.Errorf("server not found")
 
-func newInstances(client *hcloud.Client, robotClient robotclient.Client, addressFamily addressFamily, networkID int64) *instances {
-	return &instances{client, robotClient, addressFamily, networkID}
+func newInstances(
+	client *hcloud.Client,
+	robotClient robotclient.Client,
+	addressFamily addressFamily,
+	networkID int64,
+	useHrobotProviderID bool,
+) *instances {
+	return &instances{
+		client:              client,
+		robotClient:         robotClient,
+		addressFamily:       addressFamily,
+		networkID:           networkID,
+		useHrobotProviderID: useHrobotProviderID,
+	}
 }
 
 // lookupServer attempts to locate the corresponding hcloud.Server or models.Server (robot server) for a given v1.Node.
@@ -160,8 +173,16 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *corev1.Node) (me
 		return nil, fmt.Errorf("failed to get instance metadata: no matching bare metal server found for node '%s': %w",
 			node.Name, errServerNotFound)
 	}
+	providerID, err := providerid.GetBaremetalProviderId(node, bmServer.ServerNumber, i.useHrobotProviderID)
+	if err != nil {
+		return nil, err
+	}
 	return &cloudprovider.InstanceMetadata{
-		ProviderID:    providerid.LegacyFromRobotServerNumber(bmServer.ServerNumber),
+		// By default this returns the legacy format "hcloud://bm-NNNN". When the
+		// --use-hrobot-provider-id-for-baremetal flag is set, it returns "hrobot://NNNN".
+		// We will use the upstream hcloud-ccm in the future. Related PR for caph:
+		// https://github.com/syself/cluster-api-provider-hetzner/pull/1703
+		ProviderID:    providerID,
 		InstanceType:  getInstanceTypeOfRobotServer(bmServer),
 		NodeAddresses: robotNodeAddresses(i.addressFamily, bmServer),
 		Zone:          getZoneOfRobotServer(bmServer),

hcloud/instances_test.go

@@ -88,7 +88,7 @@ func TestInstances_InstanceExists(t *testing.T) {
 		})
 	})
 
-	instances := newInstances(env.Client, env.RobotClient, AddressFamilyIPv4, 0)
+	instances := newInstances(env.Client, env.RobotClient, AddressFamilyIPv4, 0, false)
 
 	tests := []struct {
 		name     string
@@ -206,7 +206,7 @@ func TestInstances_InstanceShutdown(t *testing.T) {
 		})
 	})
 
-	instances := newInstances(env.Client, env.RobotClient, AddressFamilyIPv4, 0)
+	instances := newInstances(env.Client, env.RobotClient, AddressFamilyIPv4, 0, false)
 
 	tests := []struct {
 		name     string
@@ -272,7 +272,7 @@ func TestInstances_InstanceMetadata(t *testing.T) {
 		})
 	})
 
-	instances := newInstances(env.Client, env.RobotClient, AddressFamilyIPv4, 0)
+	instances := newInstances(env.Client, env.RobotClient, AddressFamilyIPv4, 0, false)
 
 	metadata, err := instances.InstanceMetadata(context.TODO(), &corev1.Node{
 		Spec: corev1.NodeSpec{ProviderID: "hcloud://1"},
@@ -313,7 +313,7 @@ func TestInstances_InstanceMetadataRobotServer(t *testing.T) {
 		})
 	})
 
-	instances := newInstances(env.Client, env.RobotClient, AddressFamilyIPv4, 0)
+	instances := newInstances(env.Client, env.RobotClient, AddressFamilyIPv4, 0, false)
 
 	metadata, err := instances.InstanceMetadata(context.TODO(), &corev1.Node{
 		ObjectMeta: metav1.ObjectMeta{
@@ -341,6 +341,51 @@ func TestInstances_InstanceMetadataRobotServer(t *testing.T) {
 	}
 }
 
+func TestInstances_InstanceMetadataRobotServerUsesHrobotProviderIDFlag(t *testing.T) {
+	env := newTestEnv()
+	defer env.Teardown()
+	env.Mux.HandleFunc("/robot/server", func(w http.ResponseWriter, _ *http.Request) {
+		json.NewEncoder(w).Encode([]models.ServerResponse{
+			{
+				Server: models.Server{
+					ServerIP:      "123.123.123.123",
+					ServerIPv6Net: "2a01:f48:111:4221::",
+					ServerNumber:  321,
+					Product:       "bm-product 1",
+					Name:          "bm-server1",
+					Dc:            "NBG1-DC1",
+				},
+			},
+		})
+	})
+
+	instances := newInstances(env.Client, env.RobotClient, AddressFamilyIPv4, 0, true)
+
+	metadata, err := instances.InstanceMetadata(context.TODO(), &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "bm-server1",
+		},
+	})
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	expectedMetadata := &cloudprovider.InstanceMetadata{
+		ProviderID:   "hrobot://321",
+		InstanceType: "bm-product-1",
+		NodeAddresses: []corev1.NodeAddress{
+			{Type: corev1.NodeHostName, Address: "bm-server1"},
+			{Type: corev1.NodeExternalIP, Address: "123.123.123.123"},
+		},
+		Zone:   "nbg1",
+		Region: "eu-central",
+	}
+
+	if !reflect.DeepEqual(metadata, expectedMetadata) {
+		t.Fatalf("Expected metadata %+v but got %+v", *expectedMetadata, *metadata)
+	}
+}
+
 func TestNodeAddresses(t *testing.T) {
 	tests := []struct {
 		name           string

internal/credentials/hotreload.go

@@ -1,6 +1,7 @@
 package credentials
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -91,7 +92,7 @@ func handleEvent(credentialsDir, baseName string, hcloudClient *hcloud.Client, r
 		// This case is executed, when the process is running on a local machine.
 		return loadRobotCredentials(credentialsDir, robotClient)
 
-	case "hcloud":
+	case "hcloud", "token":
 		// This case is executed, when the process is running on a local machine.
 		err := loadHcloudCredentials(credentialsDir, hcloudClient)
 		if err != nil {
@@ -237,12 +238,19 @@ func GetInitialHcloudCredentialsFromDirectory(credentialsDir string) (string, er
 }
 
 func readHcloudCredentials(credentialsDir string) (string, error) {
-	hcloudTokenFile := filepath.Join(credentialsDir, "hcloud")
-	data, err := os.ReadFile(hcloudTokenFile)
-	if err != nil {
-		return "", fmt.Errorf("reading hcloud token from %q failed: %w", hcloudTokenFile, err)
+	var allErrors []error
+	for _, key := range []string{"hcloud", "token"} {
+		// upstream hcloud ccm expects by defaul the key "token" in the secret. To ease migration
+		// (back and forward), we support that, too.
+		hcloudTokenFile := filepath.Join(credentialsDir, key)
+		data, err := os.ReadFile(hcloudTokenFile)
+		if err != nil {
+			allErrors = append(allErrors, fmt.Errorf("reading hcloud token from %q failed: %w", hcloudTokenFile, err))
+			continue
+		}
+		return strings.TrimSpace(string(data)), nil
 	}
-	return strings.TrimSpace(string(data)), nil
+	return "", errors.Join(allErrors...)
 }
 
 // GetDirectory returns the directory where the credentials are stored.

internal/providerid/providerid.go

@@ -4,6 +4,8 @@
 	"fmt"
 	"strconv"
 	"strings"
+
+	corev1 "k8s.io/api/core/v1"
 )
 
 const (
@@ -17,6 +19,8 @@
 	//
 	// It MUST not be changed, otherwise existing nodes will not be recognized anymore.
 	prefixRobotLegacy = "hcloud://bm-"
+
+	prefixRobotNew = "hrobot://"
 )
 
 type UnkownPrefixError struct {
@@ -25,9 +29,10 @@
 
 func (e *UnkownPrefixError) Error() string {
 	return fmt.Sprintf(
-		"Provider ID does not have one of the the expected prefixes (%s, %s): %s",
+		"Provider ID does not have one of the the expected prefixes (%s, %s, %s): %s",
 		prefixCloud,
 		prefixRobotLegacy,
+		prefixRobotNew,
 		e.ProviderID,
 	)
 }
@@ -40,6 +45,13 @@
 func ToServerID(providerID string) (id int64, isCloudServer bool, err error) {
 	idString := ""
 	switch {
+	case strings.HasPrefix(providerID, prefixRobotNew):
+		// If a cluster switched from old-syself-ccm to upstream-hcloud-ccm, and then back again to
+		// old-syself-ccm, then there might be nodes with the new format. Let's support this
+		// edge-case, but in the long run the upstream-hcloud-ccm should be used. Related:
+		// https://github.com/syself/cluster-api-provider-hetzner/pull/1703
+		idString = strings.ReplaceAll(providerID, prefixRobotNew, "")
+
 	case strings.HasPrefix(providerID, prefixRobotLegacy):
 		// This case needs to be before [prefixCloud], as [prefixCloud] is a superset of [prefixRobotLegacy]
 		idString = strings.ReplaceAll(providerID, prefixRobotLegacy, "")
@@ -68,7 +80,12 @@
 	return fmt.Sprintf("%s%d", prefixCloud, serverID)
 }
 
-// LegacyFromRobotServerNumber generates the canonical ProviderID for a Robot Server.
-func LegacyFromRobotServerNumber(serverNumber int) string {
-	return fmt.Sprintf("%s%d", prefixRobotLegacy, serverNumber)
+func GetBaremetalProviderId(node *corev1.Node, serverNumber int, useHrobotProviderID bool) (string, error) {
+	if node.Spec.ProviderID != "" {
+		return node.Spec.ProviderID, nil
+	}
+	if useHrobotProviderID {
+		return fmt.Sprintf("%s%d", prefixRobotNew, serverNumber), nil
+	}
+	return fmt.Sprintf("%s%d", prefixRobotLegacy, serverNumber), nil
 }