diff --git a/cmd/proxmox-cloud-controller-manager/main.go b/cmd/proxmox-cloud-controller-manager/main.go index 64d30d6..2266eaa 100644 --- a/cmd/proxmox-cloud-controller-manager/main.go +++ b/cmd/proxmox-cloud-controller-manager/main.go @@ -26,7 +26,7 @@ import ( "github.com/spf13/pflag" - "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/proxmox" + "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/provider" "k8s.io/apimachinery/pkg/util/wait" cloudprovider "k8s.io/cloud-provider" @@ -52,7 +52,7 @@ func main() { command.Flags().VisitAll(func(flag *pflag.Flag) { if flag.Name == "cloud-provider" { - if err := flag.Value.Set(proxmox.ProviderName); err != nil { + if err := flag.Value.Set(provider.ProviderName); err != nil { klog.Fatalf("unable to set cloud-provider flag value: %s", err) } } diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go new file mode 100644 index 0000000..7603852 --- /dev/null +++ b/pkg/provider/provider.go @@ -0,0 +1,77 @@ +/* +Copyright 2023 The Kubernetes Authors. + +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 provider implements the providerID interface for Proxmox. +package provider + +import ( + "fmt" + "regexp" + "strconv" + "strings" + + pxapi "github.com/Telmate/proxmox-api-go/proxmox" +) + +const ( + // ProviderName is the name of the Proxmox provider. + ProviderName = "proxmox" +) + +var providerIDRegexp = regexp.MustCompile(`^` + ProviderName + `://([^/]*)/([^/]+)$`) + +// GetProviderID returns the magic providerID for kubernetes node. +func GetProviderID(region string, vmr *pxapi.VmRef) string { + return fmt.Sprintf("%s://%s/%d", ProviderName, region, vmr.VmId()) +} + +// GetVMID returns the VM ID from the providerID. +func GetVMID(providerID string) (int, error) { + if !strings.HasPrefix(providerID, ProviderName) { + return 0, fmt.Errorf("foreign providerID or empty \"%s\"", providerID) + } + + matches := providerIDRegexp.FindStringSubmatch(providerID) + if len(matches) != 3 { + return 0, fmt.Errorf("providerID \"%s\" didn't match expected format \"%s://region/InstanceID\"", providerID, ProviderName) + } + + vmID, err := strconv.Atoi(matches[2]) + if err != nil { + return 0, fmt.Errorf("InstanceID have to be a number, but got \"%s\"", matches[2]) + } + + return vmID, nil +} + +// ParseProviderID returns the VmRef and region from the providerID. +func ParseProviderID(providerID string) (*pxapi.VmRef, string, error) { + if !strings.HasPrefix(providerID, ProviderName) { + return nil, "", fmt.Errorf("foreign providerID or empty \"%s\"", providerID) + } + + matches := providerIDRegexp.FindStringSubmatch(providerID) + if len(matches) != 3 { + return nil, "", fmt.Errorf("providerID \"%s\" didn't match expected format \"%s://region/InstanceID\"", providerID, ProviderName) + } + + vmID, err := strconv.Atoi(matches[2]) + if err != nil { + return nil, "", fmt.Errorf("InstanceID have to be a number, but got \"%s\"", matches[2]) + } + + return pxapi.NewVmRef(vmID), matches[1], nil +} diff --git a/pkg/provider/provider_test.go b/pkg/provider/provider_test.go new file mode 100644 index 0000000..2bcae64 --- /dev/null +++ b/pkg/provider/provider_test.go @@ -0,0 +1,193 @@ +/* +Copyright 2023 The Kubernetes Authors. + +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 provider_test + +import ( + "fmt" + "testing" + + pxapi "github.com/Telmate/proxmox-api-go/proxmox" + "github.com/stretchr/testify/assert" + + provider "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/provider" +) + +func TestGetProviderID(t *testing.T) { + t.Parallel() + + tests := []struct { + msg string + region string + vmID int + expectedProviderID string + }{ + { + msg: "Valid providerID", + region: "region", + vmID: 123, + expectedProviderID: "proxmox://region/123", + }, + { + msg: "No region", + region: "", + vmID: 123, + expectedProviderID: "proxmox:///123", + }, + } + + for _, testCase := range tests { + testCase := testCase + + t.Run(fmt.Sprint(testCase.msg), func(t *testing.T) { + t.Parallel() + + providerID := provider.GetProviderID(testCase.region, pxapi.NewVmRef(testCase.vmID)) + + assert.Equal(t, testCase.expectedProviderID, providerID) + }) + } +} + +func TestGetVmID(t *testing.T) { + t.Parallel() + + tests := []struct { + msg string + providerID string + expectedError error + expectedvmID int + }{ + { + msg: "Valid VMID", + providerID: "proxmox://region/123", + expectedError: nil, + expectedvmID: 123, + }, + { + msg: "Valid VMID with empty region", + providerID: "proxmox:///123", + expectedError: nil, + expectedvmID: 123, + }, + { + msg: "Invalid providerID format", + providerID: "proxmox://123", + expectedError: fmt.Errorf("providerID \"proxmox://123\" didn't match expected format \"proxmox://region/InstanceID\""), + }, + { + msg: "Non proxmox providerID", + providerID: "cloud:///123", + expectedError: fmt.Errorf("foreign providerID or empty \"cloud:///123\""), + expectedvmID: 123, + }, + { + msg: "Non proxmox providerID", + providerID: "cloud://123", + expectedError: fmt.Errorf("foreign providerID or empty \"cloud://123\""), + expectedvmID: 123, + }, + { + msg: "InValid VMID", + providerID: "proxmox://region/abc", + expectedError: fmt.Errorf("InstanceID have to be a number, but got \"abc\""), + expectedvmID: 0, + }, + } + + for _, testCase := range tests { + testCase := testCase + + t.Run(fmt.Sprint(testCase.msg), func(t *testing.T) { + t.Parallel() + + VMID, err := provider.GetVMID(testCase.providerID) + + if testCase.expectedError != nil { + assert.NotNil(t, err) + assert.Equal(t, err.Error(), testCase.expectedError.Error()) + } else { + assert.Equal(t, testCase.expectedvmID, VMID) + } + }) + } +} + +func TestParseProviderID(t *testing.T) { + t.Parallel() + + tests := []struct { + msg string + providerID string + expectedError error + expectedvmID int + expectedRegion string + }{ + { + msg: "Valid VMID", + providerID: "proxmox://region/123", + expectedError: nil, + expectedvmID: 123, + expectedRegion: "region", + }, + { + msg: "Valid VMID with empty region", + providerID: "proxmox:///123", + expectedError: nil, + expectedvmID: 123, + expectedRegion: "", + }, + { + msg: "Invalid providerID format", + providerID: "proxmox://123", + expectedError: fmt.Errorf("providerID \"proxmox://123\" didn't match expected format \"proxmox://region/InstanceID\""), + }, + { + msg: "Non proxmox providerID", + providerID: "cloud:///123", + expectedError: fmt.Errorf("foreign providerID or empty \"cloud:///123\""), + }, + { + msg: "Non proxmox providerID", + providerID: "cloud://123", + expectedError: fmt.Errorf("foreign providerID or empty \"cloud://123\""), + }, + { + msg: "InValid VMID", + providerID: "proxmox://region/abc", + expectedError: fmt.Errorf("InstanceID have to be a number, but got \"abc\""), + }, + } + + for _, testCase := range tests { + testCase := testCase + + t.Run(fmt.Sprint(testCase.msg), func(t *testing.T) { + t.Parallel() + + vmr, region, err := provider.ParseProviderID(testCase.providerID) + + if testCase.expectedError != nil { + assert.NotNil(t, err) + assert.Equal(t, err.Error(), testCase.expectedError.Error()) + } else { + assert.NotNil(t, vmr) + assert.Equal(t, testCase.expectedvmID, vmr.VmId()) + assert.Equal(t, testCase.expectedRegion, region) + } + }) + } +} diff --git a/pkg/proxmox/cloud.go b/pkg/proxmox/cloud.go index 2fc9174..00c9b4a 100644 --- a/pkg/proxmox/cloud.go +++ b/pkg/proxmox/cloud.go @@ -22,6 +22,7 @@ import ( "io" "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/cluster" + provider "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/provider" clientkubernetes "k8s.io/client-go/kubernetes" cloudprovider "k8s.io/cloud-provider" @@ -29,10 +30,8 @@ import ( ) const ( - // ProviderName is the name of the Proxmox provider. - ProviderName = "proxmox" // ServiceAccountName is the service account name used in kube-system namespace. - ServiceAccountName = "proxmox-cloud-controller-manager" + ServiceAccountName = provider.ProviderName + "-cloud-controller-manager" ) type cloud struct { @@ -45,7 +44,7 @@ type cloud struct { } func init() { - cloudprovider.RegisterCloudProvider(ProviderName, func(config io.Reader) (cloudprovider.Interface, error) { + cloudprovider.RegisterCloudProvider(provider.ProviderName, func(config io.Reader) (cloudprovider.Interface, error) { cfg, err := cluster.ReadCloudConfig(config) if err != nil { klog.Errorf("failed to read config: %v", err) @@ -137,7 +136,7 @@ func (c *cloud) Routes() (cloudprovider.Routes, bool) { // ProviderName returns the cloud provider ID. func (c *cloud) ProviderName() string { - return ProviderName + return provider.ProviderName } // HasClusterID is not implemented. diff --git a/pkg/proxmox/cloud_test.go b/pkg/proxmox/cloud_test.go index 7c47e64..f5a41bf 100644 --- a/pkg/proxmox/cloud_test.go +++ b/pkg/proxmox/cloud_test.go @@ -23,6 +23,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/cluster" + provider "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/provider" ) func TestNewCloudError(t *testing.T) { @@ -73,7 +74,7 @@ clusters: assert.Equal(t, res, false) pName := cloud.ProviderName() - assert.Equal(t, pName, ProviderName) + assert.Equal(t, pName, provider.ProviderName) clID := cloud.HasClusterID() assert.Equal(t, clID, true) diff --git a/pkg/proxmox/instances.go b/pkg/proxmox/instances.go index 9868e3f..5385c29 100644 --- a/pkg/proxmox/instances.go +++ b/pkg/proxmox/instances.go @@ -19,13 +19,12 @@ package proxmox import ( "context" "fmt" - "regexp" - "strconv" "strings" pxapi "github.com/Telmate/proxmox-api-go/proxmox" "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/cluster" + provider "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/provider" v1 "k8s.io/api/core/v1" cloudprovider "k8s.io/cloud-provider" @@ -48,7 +47,7 @@ func newInstances(client *cluster.Cluster) *instances { func (i *instances) InstanceExists(_ context.Context, node *v1.Node) (bool, error) { klog.V(4).Info("instances.InstanceExists() called node: ", node.Name) - if !strings.HasPrefix(node.Spec.ProviderID, ProviderName) { + if !strings.HasPrefix(node.Spec.ProviderID, provider.ProviderName) { klog.V(4).Infof("instances.InstanceExists() node %s has foreign providerID: %s, skipped", node.Name, node.Spec.ProviderID) return true, nil @@ -73,13 +72,13 @@ func (i *instances) InstanceExists(_ context.Context, node *v1.Node) (bool, erro func (i *instances) InstanceShutdown(_ context.Context, node *v1.Node) (bool, error) { klog.V(4).Info("instances.InstanceShutdown() called, node: ", node.Name) - if !strings.HasPrefix(node.Spec.ProviderID, ProviderName) { + if !strings.HasPrefix(node.Spec.ProviderID, provider.ProviderName) { klog.V(4).Infof("instances.InstanceShutdown() node %s has foreign providerID: %s, skipped", node.Name, node.Spec.ProviderID) return false, nil } - vmr, region, err := i.parseProviderID(node.Spec.ProviderID) + vmr, region, err := provider.ParseProviderID(node.Spec.ProviderID) if err != nil { klog.Errorf("instances.InstanceShutdown() failed to parse providerID %s: %v", node.Spec.ProviderID, err) @@ -126,7 +125,7 @@ func (i *instances) InstanceMetadata(_ context.Context, node *v1.Node) (*cloudpr if err != nil { return nil, fmt.Errorf("instances.InstanceMetadata() - failed to find instance by name %s: %v, skipped", node.Name, err) } - } else if !strings.HasPrefix(node.Spec.ProviderID, ProviderName) { + } else if !strings.HasPrefix(node.Spec.ProviderID, provider.ProviderName) { klog.V(4).Infof("instances.InstanceMetadata() node %s has foreign providerID: %s, skipped", node.Name, node.Spec.ProviderID) return &cloudprovider.InstanceMetadata{}, nil @@ -148,7 +147,7 @@ func (i *instances) InstanceMetadata(_ context.Context, node *v1.Node) (*cloudpr } return &cloudprovider.InstanceMetadata{ - ProviderID: i.getProviderID(region, vmRef), + ProviderID: provider.GetProviderID(region, vmRef), NodeAddresses: addresses, InstanceType: instanceType, Zone: vmRef.Node(), @@ -162,7 +161,7 @@ func (i *instances) InstanceMetadata(_ context.Context, node *v1.Node) (*cloudpr } func (i *instances) getInstance(node *v1.Node) (*pxapi.VmRef, string, error) { - vm, region, err := i.parseProviderID(node.Spec.ProviderID) + vm, region, err := provider.ParseProviderID(node.Spec.ProviderID) if err != nil { return nil, "", fmt.Errorf("instances.getInstance() error: %v", err) } @@ -205,27 +204,3 @@ func (i *instances) getInstanceType(vmRef *pxapi.VmRef, region string) (string, vmInfo["maxcpu"].(float64), vmInfo["maxmem"].(float64)/1024/1024/1024), nil } - -var providerIDRegexp = regexp.MustCompile(`^` + ProviderName + `://([^/]*)/([^/]+)$`) - -func (i *instances) getProviderID(region string, vmr *pxapi.VmRef) string { - return fmt.Sprintf("%s://%s/%d", ProviderName, region, vmr.VmId()) -} - -func (i *instances) parseProviderID(providerID string) (*pxapi.VmRef, string, error) { - if !strings.HasPrefix(providerID, ProviderName) { - return nil, "", fmt.Errorf("foreign providerID or empty \"%s\"", providerID) - } - - matches := providerIDRegexp.FindStringSubmatch(providerID) - if len(matches) != 3 { - return nil, "", fmt.Errorf("providerID \"%s\" didn't match expected format \"%s://region/InstanceID\"", providerID, ProviderName) - } - - vmID, err := strconv.Atoi(matches[2]) - if err != nil { - return nil, "", fmt.Errorf("providerID \"%s\" didn't match expected format \"%s://region/InstanceID\"", providerID, ProviderName) - } - - return pxapi.NewVmRef(vmID), matches[1], nil -} diff --git a/pkg/proxmox/instances_test.go b/pkg/proxmox/instances_test.go index 13cc4b3..b099169 100644 --- a/pkg/proxmox/instances_test.go +++ b/pkg/proxmox/instances_test.go @@ -29,6 +29,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/cluster" + provider "github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/provider" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -464,8 +465,6 @@ func (ts *ccmTestSuite) TestInstanceMetadata() { func TestGetProviderID(t *testing.T) { t.Parallel() - i := newInstances(nil) - tests := []struct { msg string region string @@ -492,7 +491,7 @@ func TestGetProviderID(t *testing.T) { t.Run(fmt.Sprint(testCase.msg), func(t *testing.T) { t.Parallel() - expected := i.getProviderID(testCase.region, testCase.vmr) + expected := provider.GetProviderID(testCase.region, testCase.vmr) assert.Equal(t, expected, testCase.expected) }) } @@ -501,8 +500,6 @@ func TestGetProviderID(t *testing.T) { func TestParseProviderID(t *testing.T) { t.Parallel() - i := newInstances(nil) - tests := []struct { msg string magic string @@ -540,7 +537,7 @@ func TestParseProviderID(t *testing.T) { { msg: "Cluster and wrong InstanceID", magic: "proxmox://cluster/name", - expectedError: fmt.Errorf("providerID \"proxmox://cluster/name\" didn't match expected format \"proxmox://region/InstanceID\""), + expectedError: fmt.Errorf("InstanceID have to be a number, but got \"name\""), }, } @@ -550,7 +547,7 @@ func TestParseProviderID(t *testing.T) { t.Run(fmt.Sprint(testCase.msg), func(t *testing.T) { t.Parallel() - vmr, cluster, err := i.parseProviderID(testCase.magic) + vmr, cluster, err := provider.ParseProviderID(testCase.magic) if testCase.expectedError != nil { assert.Equal(t, testCase.expectedError, err)