mirror of
https://github.com/outbackdingo/proxmox-cloud-controller-manager.git
synced 2026-01-27 10:20:13 +00:00
refactor: move providerID to the package
Parsing and generating magic providerID moved to a separate package. Signed-off-by: Serge Logvinov <serge.logvinov@sinextra.dev>
This commit is contained in:
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
77
pkg/provider/provider.go
Normal file
77
pkg/provider/provider.go
Normal file
@@ -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
|
||||
}
|
||||
193
pkg/provider/provider_test.go
Normal file
193
pkg/provider/provider_test.go
Normal file
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user