Skip to content

Commit

Permalink
refactor: duplicate utility to get server ID from ProviderID (#532)
Browse files Browse the repository at this point in the history
This utility function was duplicated with nearly the exact same
functionality. This commit cleans it up by extracting to a new package
(to avoid cyclic imports).

These two methods are about to get more complicated with #523, better to
clean it up now than to make changes to both locations in the future.

---------

Co-authored-by: Jonas L. <[email protected]>
  • Loading branch information
apricote and jooola authored Oct 6, 2023
1 parent 6e320a6 commit 6f4b4a6
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 80 deletions.
5 changes: 3 additions & 2 deletions hcloud/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
cloudprovider "k8s.io/cloud-provider"

"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics"
"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/providerid"
"github.com/hetznercloud/hcloud-go/v2/hcloud"
)

Expand Down Expand Up @@ -51,7 +52,7 @@ func newInstances(client *hcloud.Client, addressFamily addressFamily, networkID
func (i *instances) lookupServer(ctx context.Context, node *corev1.Node) (*hcloud.Server, error) {
var server *hcloud.Server
if node.Spec.ProviderID != "" {
serverID, err := providerIDToServerID(node.Spec.ProviderID)
serverID, err := providerid.ToServerID(node.Spec.ProviderID)
if err != nil {
return nil, fmt.Errorf("failed to convert provider id to server id: %w", err)
}
Expand Down Expand Up @@ -110,7 +111,7 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *corev1.Node) (*c
}

return &cloudprovider.InstanceMetadata{
ProviderID: serverIDToProviderID(server.ID),
ProviderID: providerid.FromServerID(server.ID),
InstanceType: server.ServerType.Name,
NodeAddresses: nodeAddresses(i.addressFamily, i.networkID, server),
Zone: server.Datacenter.Name,
Expand Down
53 changes: 0 additions & 53 deletions hcloud/util.go

This file was deleted.

27 changes: 2 additions & 25 deletions internal/hcops/load_balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"context"
"errors"
"fmt"
"strconv"
"strings"
"sync"
"time"

Expand All @@ -14,6 +12,7 @@ import (

"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation"
"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics"
"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/providerid"
"github.com/hetznercloud/hcloud-go/v2/hcloud"
)

Expand Down Expand Up @@ -594,7 +593,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(

// Extract HC server IDs of all K8S nodes assigned to the K8S cluster.
for _, node := range nodes {
id, err := providerIDToServerID(node.Spec.ProviderID)
id, err := providerid.ToServerID(node.Spec.ProviderID)
if err != nil {
return changed, fmt.Errorf("%s: %w", op, err)
}
Expand Down Expand Up @@ -1204,28 +1203,6 @@ func (b *hclbServiceOptsBuilder) buildUpdateServiceOpts() (hcloud.LoadBalancerUp
return opts, nil
}

// TODO this is a copy of the function in hcloud/utils.go => refactor.
func providerIDToServerID(providerID string) (int64, error) {
const op = "hcops/providerIDToServerID"
metrics.OperationCalled.WithLabelValues(op).Inc()

providerPrefix := "hcloud://"
if !strings.HasPrefix(providerID, providerPrefix) {
return 0, fmt.Errorf("%s: missing prefix hcloud://: %s", op, providerID)
}

idString := strings.ReplaceAll(providerID, providerPrefix, "")
if idString == "" {
return 0, fmt.Errorf("%s: missing serverID: %s", op, providerID)
}

id, err := strconv.ParseInt(idString, 10, 64)
if err != nil {
return 0, fmt.Errorf("%s: invalid serverID: %s", op, providerID)
}
return id, nil
}

func lbAttached(lb *hcloud.LoadBalancer, nwID int64) bool {
for _, nw := range lb.PrivateNet {
if nw.Network.ID == nwID {
Expand Down
36 changes: 36 additions & 0 deletions internal/providerid/providerid.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package providerid

import (
"fmt"
"strconv"
"strings"
)

const (
// providerPrefix is the prefix for all provider IDs. It MUST not be changed,
// otherwise existing nodes will not be recognized anymore.
providerPrefix = "hcloud://"
)

// ToServerID converts a ProviderID to a server ID.
func ToServerID(providerID string) (int64, error) {
if !strings.HasPrefix(providerID, providerPrefix) {
return 0, fmt.Errorf("providerID does not have the expected prefix %s: %s", providerPrefix, providerID)
}

idString := strings.ReplaceAll(providerID, providerPrefix, "")
if idString == "" {
return 0, fmt.Errorf("providerID is missing a serverID: %s", providerID)
}

id, err := strconv.ParseInt(idString, 10, 64)
if err != nil {
return 0, fmt.Errorf("unable to parse server id: %s", providerID)
}
return id, nil
}

// FromServerID converts a server ID to a ProviderID.
func FromServerID(serverID int64) string {
return fmt.Sprintf("%s%d", providerPrefix, serverID)
}

0 comments on commit 6f4b4a6

Please sign in to comment.