Skip to content

Commit d6353f7

Browse files
abdullah599abdullah599
committed
🌱 Ignore empty nodes with providerIDs when ensuring load balancer targets
1 parent 3e13cac commit d6353f7

File tree

5 files changed

+328
-86
lines changed

5 files changed

+328
-86
lines changed

‎hcloud/instances.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
"github.com/hetznercloud/hcloud-go/v2/hcloud"
2424
"github.com/syself/hetzner-cloud-controller-manager/internal/metrics"
25+
"github.com/syself/hetzner-cloud-controller-manager/internal/providerid"
2526
robotclient "github.com/syself/hetzner-cloud-controller-manager/internal/robot/client"
2627
"github.com/syself/hrobot-go/models"
2728
corev1 "k8s.io/api/core/v1"
@@ -58,7 +59,7 @@ func (i *instances) lookupServer(
5859
) (hcloudServer *hcloud.Server, bmServer *models.Server, isHCloudServer bool, err error) {
5960
if node.Spec.ProviderID != "" {
6061
var serverID int64
61-
serverID, isHCloudServer, err = providerIDToServerID(node.Spec.ProviderID)
62+
serverID, isHCloudServer, err = providerid.ToServerID(node.Spec.ProviderID)
6263
if err != nil {
6364
return nil, nil, false, fmt.Errorf("failed to convert provider id to server id: %w", err)
6465
}
@@ -144,7 +145,7 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *corev1.Node) (*c
144145
node.Name, errServerNotFound)
145146
}
146147
return &cloudprovider.InstanceMetadata{
147-
ProviderID: serverIDToProviderIDHCloud(hcloudServer.ID),
148+
ProviderID: providerid.FromCloudServerID(hcloudServer.ID),
148149
InstanceType: hcloudServer.ServerType.Name,
149150
NodeAddresses: hcloudNodeAddresses(i.addressFamily, i.networkID, hcloudServer),
150151
Zone: hcloudServer.Datacenter.Name,
@@ -156,7 +157,7 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *corev1.Node) (*c
156157
node.Name, errServerNotFound)
157158
}
158159
return &cloudprovider.InstanceMetadata{
159-
ProviderID: serverIDToProviderIDRobot(bmServer.ServerNumber),
160+
ProviderID: providerid.LegacyFromRobotServerNumber(bmServer.ServerNumber),
160161
InstanceType: getInstanceTypeOfRobotServer(bmServer),
161162
NodeAddresses: robotNodeAddresses(i.addressFamily, bmServer),
162163
Zone: getZoneOfRobotServer(bmServer),

‎hcloud/util.go‎

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121
"fmt"
2222
"regexp"
23-
"strconv"
2423
"strings"
2524

2625
"github.com/hetznercloud/hcloud-go/v2/hcloud"
@@ -29,7 +28,6 @@ import (
2928
robotclient "github.com/syself/hetzner-cloud-controller-manager/internal/robot/client"
3029
"github.com/syself/hrobot-go/models"
3130
corev1 "k8s.io/api/core/v1"
32-
"k8s.io/klog/v2"
3331
)
3432

3533
func getHCloudServerByName(ctx context.Context, c *hcloud.Client, name string) (*hcloud.Server, error) {
@@ -118,50 +116,10 @@ func getRobotServerByID(c robotclient.Client, id int, node *corev1.Node) (s *mod
118116
return server, nil
119117
}
120118

121-
func providerIDToServerID(providerID string) (id int64, isHCloudServer bool, err error) {
122-
const op = "hcloud/providerIDToServerID"
123-
metrics.OperationCalled.WithLabelValues(op).Inc()
124-
125-
providerPrefixHCloud := providerName + "://"
126-
providerPrefixRobot := providerName + "://" + hostNamePrefixRobot
127-
128-
if !strings.HasPrefix(providerID, providerPrefixHCloud) && !strings.HasPrefix(providerID, providerPrefixRobot) {
129-
klog.Infof("%s: make sure your cluster configured for an external cloud provider", op)
130-
return 0, false, fmt.Errorf("%s: missing prefix %s or %s. %s", providerPrefixHCloud, providerPrefixRobot, op, providerID)
131-
}
132-
133-
isHCloudServer = true
134-
idString := providerID
135-
if strings.HasPrefix(providerID, providerPrefixRobot) {
136-
isHCloudServer = false
137-
idString = strings.ReplaceAll(idString, providerPrefixRobot, "")
138-
} else {
139-
idString = strings.ReplaceAll(providerID, providerPrefixHCloud, "")
140-
}
141-
142-
if idString == "" {
143-
return 0, false, fmt.Errorf("%s: missing serverID: %s", op, providerID)
144-
}
145-
146-
id, err = strconv.ParseInt(idString, 10, 64)
147-
if err != nil {
148-
return 0, false, fmt.Errorf("%s: invalid serverID: %s", op, providerID)
149-
}
150-
return id, isHCloudServer, nil
151-
}
152-
153119
func isHCloudServerByName(name string) bool {
154120
return !strings.HasPrefix(name, hostNamePrefixRobot)
155121
}
156122

157-
func serverIDToProviderIDRobot(serverID int) string {
158-
return fmt.Sprintf("%s://%s%d", providerName, hostNamePrefixRobot, serverID)
159-
}
160-
161-
func serverIDToProviderIDHCloud(serverID int64) string {
162-
return fmt.Sprintf("%s://%d", providerName, serverID)
163-
}
164-
165123
func getInstanceTypeOfRobotServer(bmServer *models.Server) string {
166124
if bmServer == nil {
167125
panic("getInstanceTypeOfRobotServer called with nil server")

‎internal/hcops/load_balancer.go‎

Lines changed: 13 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,13 @@ import (
55
"errors"
66
"fmt"
77
"net"
8-
"strconv"
9-
"strings"
108
"sync"
119
"time"
1210

1311
"github.com/hetznercloud/hcloud-go/v2/hcloud"
1412
"github.com/syself/hetzner-cloud-controller-manager/internal/annotation"
1513
"github.com/syself/hetzner-cloud-controller-manager/internal/metrics"
14+
"github.com/syself/hetzner-cloud-controller-manager/internal/providerid"
1615
"github.com/syself/hetzner-cloud-controller-manager/internal/robot/client"
1716
"github.com/syself/hrobot-go/models"
1817
corev1 "k8s.io/api/core/v1"
@@ -627,8 +626,19 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
627626

628627
// Extract HC server IDs of all K8S nodes assigned to the K8S cluster.
629628
for _, node := range nodes {
630-
id, isHCloudServer, err := providerIDToServerID(node.Spec.ProviderID)
629+
id, isHCloudServer, err := providerid.ToServerID(node.Spec.ProviderID)
631630
if err != nil {
631+
if errors.As(err, new(*providerid.UnkownPrefixError)) {
632+
// ProviderID has unknown prefix, cluster might have non-hccm nodes that can not be added to the
633+
// Load Balancer. Emitting an event and ignoring that Node in this reconciliation loop.
634+
l.Recorder.Event(
635+
node,
636+
corev1.EventTypeWarning,
637+
"UnknownProviderIDPrefix",
638+
fmt.Sprintf("Node could not be added to Load Balancer for service %s because the provider ID does not match any known format", svc.Name),
639+
)
640+
continue
641+
}
632642
return changed, fmt.Errorf("%s: %w", op, err)
633643
}
634644
if isHCloudServer {
@@ -1373,44 +1383,6 @@ func (b *hclbServiceOptsBuilder) buildUpdateServiceOpts() (hcloud.LoadBalancerUp
13731383
return opts, nil
13741384
}
13751385

1376-
// TODO this is a copy of the function in hcloud/utils.go => refactor.
1377-
const (
1378-
providerName = "hcloud"
1379-
hostNamePrefixRobot = "bm-"
1380-
)
1381-
1382-
func providerIDToServerID(providerID string) (id int64, isHCloudServer bool, err error) {
1383-
const op = "hcloud/providerIDToServerID"
1384-
metrics.OperationCalled.WithLabelValues(op).Inc()
1385-
1386-
providerPrefixHCloud := providerName + "://"
1387-
providerPrefixRobot := providerName + "://" + hostNamePrefixRobot
1388-
1389-
if !strings.HasPrefix(providerID, providerPrefixHCloud) && !strings.HasPrefix(providerID, providerPrefixRobot) {
1390-
klog.Infof("%s: make sure your cluster configured for an external cloud provider", op)
1391-
return 0, false, fmt.Errorf("%s: missing prefix %s or %s. %s", providerPrefixHCloud, providerPrefixRobot, op, providerID)
1392-
}
1393-
1394-
isHCloudServer = true
1395-
idString := providerID
1396-
if strings.HasPrefix(providerID, providerPrefixRobot) {
1397-
isHCloudServer = false
1398-
idString = strings.ReplaceAll(idString, providerPrefixRobot, "")
1399-
} else {
1400-
idString = strings.ReplaceAll(providerID, providerPrefixHCloud, "")
1401-
}
1402-
1403-
if idString == "" {
1404-
return 0, false, fmt.Errorf("%s: missing serverID: %s", op, providerID)
1405-
}
1406-
1407-
id, err = strconv.ParseInt(idString, 10, 64)
1408-
if err != nil {
1409-
return 0, false, fmt.Errorf("%s: invalid serverID: %s", op, providerID)
1410-
}
1411-
return id, isHCloudServer, nil
1412-
}
1413-
14141386
func lbAttached(lb *hcloud.LoadBalancer, nwID int64) bool {
14151387
for _, nw := range lb.PrivateNet {
14161388
if nw.Network.ID == nwID {

‎internal/providerid/providerid.go‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package providerid
2+
3+
import (
4+
"fmt"
5+
"strconv"
6+
"strings"
7+
)
8+
9+
const (
10+
// prefixCloud is the prefix for Cloud Server provider IDs.
11+
//
12+
// It MUST not be changed, otherwise existing nodes will not be recognized anymore.
13+
prefixCloud = "hcloud://"
14+
15+
// prefixRobot is the prefix for Robot Server provider IDs.
16+
//
17+
// It MUST not be changed, otherwise existing nodes will not be recognized anymore.
18+
prefixRobot = "hrobot://"
19+
20+
// prefixRobot is the prefix used by the Syself Fork for Robot Server provider IDs.
21+
// This Prefix is no longer used for new nodes, instead [prefixRobot] should be used.
22+
//
23+
// It MUST not be changed, otherwise existing nodes will not be recognized anymore.
24+
prefixRobotLegacy = "hcloud://bm-"
25+
)
26+
27+
type UnkownPrefixError struct {
28+
ProviderID string
29+
}
30+
31+
func (e *UnkownPrefixError) Error() string {
32+
return fmt.Sprintf(
33+
"Provider ID does not have one of the the expected prefixes (%s, %s, %s): %s",
34+
prefixCloud,
35+
prefixRobot,
36+
prefixRobotLegacy,
37+
e.ProviderID,
38+
)
39+
}
40+
41+
// ToServerID parses the Cloud or Robot Server ID from a ProviderID.
42+
//
43+
// This method supports all formats for the ProviderID that were ever used.
44+
// If a format is ever dropped from this method the Nodes that still use that
45+
// format will get abandoned and can no longer be processed by HCCM.
46+
func ToServerID(providerID string) (id int64, isCloudServer bool, err error) {
47+
idString := ""
48+
switch {
49+
case strings.HasPrefix(providerID, prefixRobot):
50+
idString = strings.ReplaceAll(providerID, prefixRobot, "")
51+
52+
case strings.HasPrefix(providerID, prefixRobotLegacy):
53+
// This case needs to be before [prefixCloud], as [prefixCloud] is a superset of [prefixRobotLegacy]
54+
idString = strings.ReplaceAll(providerID, prefixRobotLegacy, "")
55+
56+
case strings.HasPrefix(providerID, prefixCloud):
57+
isCloudServer = true
58+
idString = strings.ReplaceAll(providerID, prefixCloud, "")
59+
60+
default:
61+
return 0, false, &UnkownPrefixError{providerID}
62+
}
63+
64+
if idString == "" {
65+
return 0, false, fmt.Errorf("providerID is missing a serverID: %s", providerID)
66+
}
67+
68+
id, err = strconv.ParseInt(idString, 10, 64)
69+
if err != nil {
70+
return 0, false, fmt.Errorf("unable to parse server id: %s", providerID)
71+
}
72+
return id, isCloudServer, nil
73+
}
74+
75+
// FromCloudServerID generates the canonical ProviderID for a Cloud Server.
76+
func FromCloudServerID(serverID int64) string {
77+
return fmt.Sprintf("%s%d", prefixCloud, serverID)
78+
}
79+
80+
// LegacyFromRobotServerNumber generates the canonical ProviderID for a Robot Server.
81+
func LegacyFromRobotServerNumber(serverNumber int) string {
82+
return fmt.Sprintf("%s%d", prefixRobotLegacy, serverNumber)
83+
}

0 commit comments

Comments
 (0)