From 6f4b4a64bf90e7b55fa732ce524e74df696d9e74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20T=C3=B6lle?= Date: Fri, 6 Oct 2023 15:43:28 +0200 Subject: [PATCH] refactor: duplicate utility to get server ID from ProviderID (#532) 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. --- hcloud/instances.go | 5 +-- hcloud/util.go | 53 ------------------------------- internal/hcops/load_balancer.go | 27 ++-------------- internal/providerid/providerid.go | 36 +++++++++++++++++++++ 4 files changed, 41 insertions(+), 80 deletions(-) delete mode 100644 hcloud/util.go create mode 100644 internal/providerid/providerid.go diff --git a/hcloud/instances.go b/hcloud/instances.go index caa00d322..bee697aec 100644 --- a/hcloud/instances.go +++ b/hcloud/instances.go @@ -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" ) @@ -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) } @@ -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, diff --git a/hcloud/util.go b/hcloud/util.go deleted file mode 100644 index dca2ea88d..000000000 --- a/hcloud/util.go +++ /dev/null @@ -1,53 +0,0 @@ -/* -Copyright 2018 Hetzner Cloud GmbH. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package hcloud - -import ( - "fmt" - "strconv" - "strings" - - "k8s.io/klog/v2" - - "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics" -) - -func providerIDToServerID(providerID string) (int64, error) { - const op = "hcloud/providerIDToServerID" - metrics.OperationCalled.WithLabelValues(op).Inc() - - providerPrefix := providerName + "://" - if !strings.HasPrefix(providerID, providerPrefix) { - klog.Infof("%s: make sure your cluster configured for an external cloud provider", op) - 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 serverIDToProviderID(serverID int64) string { - return fmt.Sprintf("%s://%d", providerName, serverID) -} diff --git a/internal/hcops/load_balancer.go b/internal/hcops/load_balancer.go index ee420f641..b8f9001d3 100644 --- a/internal/hcops/load_balancer.go +++ b/internal/hcops/load_balancer.go @@ -4,8 +4,6 @@ import ( "context" "errors" "fmt" - "strconv" - "strings" "sync" "time" @@ -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" ) @@ -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) } @@ -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 { diff --git a/internal/providerid/providerid.go b/internal/providerid/providerid.go new file mode 100644 index 000000000..ada5d69d7 --- /dev/null +++ b/internal/providerid/providerid.go @@ -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) +}