Skip to content

Commit

Permalink
refactor: move providerID to the package
Browse files Browse the repository at this point in the history
Parsing and generating magic providerID moved to a separate package.

Signed-off-by: Serge Logvinov <serge.logvinov@sinextra.dev>
  • Loading branch information
sergelogvinov committed Feb 15, 2024
1 parent 6f0c667 commit 7b73b5f
Show file tree
Hide file tree
Showing 7 changed files with 289 additions and 47 deletions.
4 changes: 2 additions & 2 deletions cmd/proxmox-cloud-controller-manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}
}
Expand Down
77 changes: 77 additions & 0 deletions pkg/provider/provider.go
Original file line number Diff line number Diff line change
@@ -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 changes: 193 additions & 0 deletions pkg/provider/provider_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
9 changes: 4 additions & 5 deletions pkg/proxmox/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,16 @@ 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"
"k8s.io/klog/v2"
)

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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion pkg/proxmox/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 7b73b5f

Please sign in to comment.