refactor: proxmox cloud config

Move Proxmox cluster config to 'cluster' folder.

.dockerignore

@@ -3,11 +3,11 @@
 .git/
 **/.gitignore
 #
+bin/
 charts/
 docs/
 hack/
 Dockerfile
-/proxmox-cloud-controller-manager*
 #
 # other
 *.md

.gitignore

@@ -1,4 +1,5 @@
 #
+/bin/
 /charts/proxmox-cloud-controller-manager/values-dev.yaml
 /proxmox-cloud-controller-manager*
 /kubeconfig

Dockerfile

@@ -19,6 +19,6 @@ FROM --platform=${TARGETARCH} gcr.io/distroless/static-debian11:nonroot AS relea
 LABEL org.opencontainers.image.source https://github.com/sergelogvinov/proxmox-cloud-controller-manager
 
 ARG TARGETARCH
-COPY --from=builder /src/proxmox-cloud-controller-manager-${TARGETARCH} /proxmox-cloud-controller-manager
+COPY --from=builder /src/bin/proxmox-cloud-controller-manager-${TARGETARCH} /proxmox-cloud-controller-manager
 
 ENTRYPOINT ["/proxmox-cloud-controller-manager"]

Makefile

@@ -48,14 +48,18 @@ help: ## This help menu.
 build-all-archs:
 	@for arch in $(ARCHS); do $(MAKE) ARCH=$${arch} build ; done
 
+.PHONY: clean
+clean: ## Clean
+	rm -rf bin
+
 .PHONY: build
 build: ## Build
 	CGO_ENABLED=0 GOOS=$(OS) GOARCH=$(ARCH) go build $(GO_LDFLAGS) \
-		-o proxmox-cloud-controller-manager-$(ARCH) ./cmd/proxmox-cloud-controller-manager
+		-o bin/proxmox-cloud-controller-manager-$(ARCH) ./cmd/proxmox-cloud-controller-manager
 
 .PHONY: run
 run: build
-	./proxmox-cloud-controller-manager-$(ARCH) --v=5 --kubeconfig=kubeconfig --cloud-config=proxmox-config.yaml --controllers=cloud-node,cloud-node-lifecycle \
+	./bin/proxmox-cloud-controller-manager-$(ARCH) --v=5 --kubeconfig=kubeconfig --cloud-config=proxmox-config.yaml --controllers=cloud-node,cloud-node-lifecycle \
 		--use-service-account-credentials --leader-elect=false --bind-address=127.0.0.1
 
 .PHONY: lint

pkg/cluster/client.go

@@ -0,0 +1,85 @@
+// Package cluster implements the multi-cloud provider interface for Proxmox.
+package cluster
+
+import (
+	"crypto/tls"
+	"fmt"
+	"os"
+
+	pxapi "github.com/Telmate/proxmox-api-go/proxmox"
+)
+
+// Client is a Proxmox client.
+type Client struct {
+	config  *ClustersConfig
+	proxmox map[string]*pxapi.Client
+}
+
+// NewClient creates a new Proxmox client.
+func NewClient(config *ClustersConfig) (*Client, error) {
+	clusters := len(config.Clusters)
+	if clusters > 0 {
+		proxmox := make(map[string]*pxapi.Client, clusters)
+
+		for _, cfg := range config.Clusters {
+			tlsconf := &tls.Config{InsecureSkipVerify: true}
+			if !cfg.Insecure {
+				tlsconf = nil
+			}
+
+			client, err := pxapi.NewClient(cfg.URL, nil, os.Getenv("PM_HTTP_HEADERS"), tlsconf, "", 600)
+			if err != nil {
+				return nil, err
+			}
+
+			client.SetAPIToken(cfg.TokenID, cfg.TokenSecret)
+
+			if _, err := client.GetVersion(); err != nil {
+				return nil, fmt.Errorf("failed to initialized proxmox client in cluster %s: %v", cfg.Region, err)
+			}
+
+			proxmox[cfg.Region] = client
+		}
+
+		return &Client{
+			config:  config,
+			proxmox: proxmox,
+		}, nil
+	}
+
+	return nil, nil
+}
+
+// CheckClusters checks if the Proxmox connection is working.
+func (c *Client) CheckClusters() error {
+	for region, client := range c.proxmox {
+		if _, err := client.GetVersion(); err != nil {
+			return fmt.Errorf("failed to initialized proxmox client in region %s, error: %v", region, err)
+		}
+	}
+
+	return nil
+}
+
+// GetProxmoxCluster returns a Proxmox cluster client in a given region.
+func (c *Client) GetProxmoxCluster(region string) (*pxapi.Client, error) {
+	if c.proxmox[region] != nil {
+		return c.proxmox[region], nil
+	}
+
+	return nil, fmt.Errorf("proxmox cluster %s not found", region)
+}
+
+// FindVMByName find a VM by name in all Proxmox clusters.
+func (c *Client) FindVMByName(name string) (*pxapi.VmRef, string, error) {
+	for region, px := range c.proxmox {
+		vmr, err := px.GetVmRefByName(name)
+		if err != nil {
+			continue
+		}
+
+		return vmr, region, nil
+	}
+
+	return nil, "", fmt.Errorf("VM %s not found", name)
+}

pkg/cluster/cloud_config.go

@@ -0,0 +1,53 @@
+package cluster
+
+import (
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+
+	yaml "gopkg.in/yaml.v3"
+)
+
+// ClustersConfig is proxmox multi-cluster cloud config.
+type ClustersConfig struct {
+	Clusters []struct {
+		URL         string `yaml:"url"`
+		Insecure    bool   `yaml:"insecure,omitempty"`
+		TokenID     string `yaml:"token_id,omitempty"`
+		TokenSecret string `yaml:"token_secret,omitempty"`
+		Region      string `yaml:"region,omitempty"`
+	} `yaml:"clusters,omitempty"`
+}
+
+// ReadCloudConfig reads cloud config from a reader.
+func ReadCloudConfig(config io.Reader) (ClustersConfig, error) {
+	cfg := ClustersConfig{}
+
+	if config != nil {
+		if err := yaml.NewDecoder(config).Decode(&cfg); err != nil {
+			return ClustersConfig{}, err
+		}
+	}
+
+	return cfg, nil
+}
+
+// ReadCloudConfigFromFile reads cloud config from a file.
+func ReadCloudConfigFromFile(file string) (ClustersConfig, error) {
+	f, err := os.Open(filepath.Clean(file))
+	if err != nil {
+		return ClustersConfig{}, fmt.Errorf("error reading %s: %v", file, err)
+	}
+	defer f.Close() // nolint: errcheck
+
+	cfg := ClustersConfig{}
+
+	if f != nil {
+		if err := yaml.NewDecoder(f).Decode(&cfg); err != nil {
+			return ClustersConfig{}, err
+		}
+	}
+
+	return cfg, nil
+}

pkg/proxmox/client.go

@@ -1,68 +0,0 @@
-package proxmox
-
-import (
-	"crypto/tls"
-	"os"
-
-	pxapi "github.com/Telmate/proxmox-api-go/proxmox"
-
-	clientkubernetes "k8s.io/client-go/kubernetes"
-	"k8s.io/klog/v2"
-)
-
-type client struct {
-	config  *cloudConfig
-	proxmox []pxCluster
-	kclient clientkubernetes.Interface
-}
-
-type pxCluster struct {
-	client *pxapi.Client
-	region string
-}
-
-func newClient(config *cloudConfig) (*client, error) {
-	clusters := len(config.Clusters)
-	if clusters > 0 {
-		proxmox := make([]pxCluster, clusters)
-
-		for idx, cfg := range config.Clusters {
-			tlsconf := &tls.Config{InsecureSkipVerify: true}
-			if !cfg.Insecure {
-				tlsconf = nil
-			}
-
-			client, err := pxapi.NewClient(cfg.URL, nil, os.Getenv("PM_HTTP_HEADERS"), tlsconf, "", 600)
-			if err != nil {
-				return nil, err
-			}
-
-			client.SetAPIToken(cfg.TokenID, cfg.TokenSecret)
-
-			if _, err := client.GetVersion(); err != nil {
-				klog.Errorf("failed to initialized proxmox client in cluster %s: %v", cfg.Region, err)
-
-				return nil, err
-			}
-
-			proxmox[idx] = pxCluster{client: client, region: cfg.Region}
-		}
-
-		return &client{
-			config:  config,
-			proxmox: proxmox,
-		}, nil
-	}
-
-	return nil, nil
-}
-
-func (c *client) GetProxmoxCluster(region string) (*pxCluster, error) {
-	for _, px := range c.proxmox {
-		if px.region == region {
-			return &px, nil
-		}
-	}
-
-	return nil, nil
-}

pkg/proxmox/cloud.go

@@ -5,6 +5,9 @@ import (
 	"context"
 	"io"
 
+	"github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/cluster"
+
+	clientkubernetes "k8s.io/client-go/kubernetes"
 	cloudprovider "k8s.io/cloud-provider"
 	"k8s.io/klog/v2"
 )
@@ -17,7 +20,8 @@ const (
 )
 
 type cloud struct {
-	client      *client
+	client      *cluster.Client
+	kclient     clientkubernetes.Interface
 	instancesV2 cloudprovider.InstancesV2
 
 	ctx  context.Context //nolint:containedctx
@@ -26,7 +30,7 @@ type cloud struct {
 
 func init() {
 	cloudprovider.RegisterCloudProvider(ProviderName, func(config io.Reader) (cloudprovider.Interface, error) {
-		cfg, err := readCloudConfig(config)
+		cfg, err := cluster.ReadCloudConfig(config)
 		if err != nil {
 			klog.Errorf("failed to read config: %v", err)
 
@@ -37,8 +41,8 @@ func init() {
 	})
 }
 
-func newCloud(config *cloudConfig) (cloudprovider.Interface, error) {
+func newCloud(config *cluster.ClustersConfig) (cloudprovider.Interface, error) {
-	client, err := newClient(config)
+	client, err := cluster.NewClient(config)
 	if err != nil {
 		return nil, err
 	}
@@ -55,7 +59,7 @@ func newCloud(config *cloudConfig) (cloudprovider.Interface, error) {
 // to perform housekeeping or run custom controllers specific to the cloud provider.
 // Any tasks started here should be cleaned up when the stop channel closes.
 func (c *cloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) {
-	c.client.kclient = clientBuilder.ClientOrDie(ServiceAccountName)
+	c.kclient = clientBuilder.ClientOrDie(ServiceAccountName)
 
 	klog.Infof("clientset initialized")
 
@@ -63,12 +67,9 @@ func (c *cloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder,
 	c.ctx = ctx
 	c.stop = cancel
 
-	for _, px := range c.client.proxmox {
+	err := c.client.CheckClusters()
-		if _, err := px.client.GetVersion(); err != nil {
+	if err != nil {
-			klog.Errorf("failed to initialized proxmox client on region %s: %v", px.region, err)
+		klog.Errorf("failed to initialized proxmox client: %v", err)
-
-			return
-		}
 	}
 
 	// Broadcast the upstream stop signal to all provider-level goroutines

pkg/proxmox/cloud_config.go

@@ -1,31 +0,0 @@
-package proxmox
-
-import (
-	"io"
-
-	yaml "gopkg.in/yaml.v3"
-)
-
-type cloudConfig struct {
-	Clusters []struct {
-		URL         string `yaml:"url"`
-		Insecure    bool   `yaml:"insecure,omitempty"`
-		TokenID     string `yaml:"token_id,omitempty"`
-		TokenSecret string `yaml:"token_secret,omitempty"`
-		Region      string `yaml:"region,omitempty"`
-	} `yaml:"clusters,omitempty"`
-}
-
-func readCloudConfig(config io.Reader) (cloudConfig, error) {
-	cfg := cloudConfig{}
-
-	if config != nil {
-		if err := yaml.NewDecoder(config).Decode(&cfg); err != nil {
-			return cloudConfig{}, err
-		}
-	}
-
-	// klog.V(5).Infof("cloudConfig: %+v", cfg)
-
-	return cfg, nil
-}

pkg/proxmox/instances.go

@@ -9,6 +9,8 @@ import (
 
 	pxapi "github.com/Telmate/proxmox-api-go/proxmox"
 
+	"github.com/sergelogvinov/proxmox-cloud-controller-manager/pkg/cluster"
+
 	v1 "k8s.io/api/core/v1"
 	cloudprovider "k8s.io/cloud-provider"
 	cloudproviderapi "k8s.io/cloud-provider/api"
@@ -16,10 +18,10 @@ import (
 )
 
 type instances struct {
-	c *client
+	c *cluster.Client
 }
 
-func newInstances(client *client) *instances {
+func newInstances(client *cluster.Client) *instances {
 	return &instances{
 		c: client,
 	}
@@ -71,7 +73,7 @@ func (i *instances) InstanceShutdown(_ context.Context, node *v1.Node) (bool, er
 		return false, err
 	}
 
-	vmState, err := px.client.GetVmState(vmRef)
+	vmState, err := px.GetVmState(vmRef)
 	if err != nil {
 		return false, err
 	}
@@ -93,22 +95,16 @@ func (i *instances) InstanceMetadata(_ context.Context, node *v1.Node) (*cloudpr
 		var (
 			vmRef  *pxapi.VmRef
 			region string
+			err    error
 		)
 
 		providerID := node.Spec.ProviderID
 		if providerID == "" {
 			klog.V(4).Infof("instances.InstanceMetadata() - trying to find providerID for node %s", node.Name)
 
-			for _, px := range i.c.proxmox {
+			vmRef, region, err = i.c.FindVMByName(node.Name)
-				vm, err := px.client.GetVmRefByName(node.Name)
+			if err != nil {
-				if err != nil {
+				return nil, fmt.Errorf("instances.InstanceMetadata() - failed to find instance by name %s: %v, skipped", node.Name, err)
-					continue
-				}
-
-				vmRef = vm
-				region = px.region
-
-				break
 			}
 		} else if !strings.HasPrefix(node.Spec.ProviderID, ProviderName) {
 			klog.V(4).Infof("instances.InstanceMetadata() node %s has foreign providerID: %s, skipped", node.Name, node.Spec.ProviderID)
@@ -117,8 +113,6 @@ func (i *instances) InstanceMetadata(_ context.Context, node *v1.Node) (*cloudpr
 		}
 
 		if vmRef == nil {
-			var err error
-
 			vmRef, region, err = i.getInstance(node)
 			if err != nil {
 				return nil, err
@@ -128,15 +122,13 @@ func (i *instances) InstanceMetadata(_ context.Context, node *v1.Node) (*cloudpr
 		addresses := []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: providedIP}}
 		addresses = append(addresses, v1.NodeAddress{Type: v1.NodeHostName, Address: node.Name})
 
-		providerID = fmt.Sprintf("%s://%s/%d", ProviderName, region, vmRef.VmId())
-
 		instanceType, err := i.getInstanceType(vmRef, region)
 		if err != nil {
 			instanceType = vmRef.GetVmType()
 		}
 
 		return &cloudprovider.InstanceMetadata{
-			ProviderID:    providerID,
+			ProviderID:    i.getProviderID(region, vmRef),
 			NodeAddresses: addresses,
 			InstanceType:  instanceType,
 			Zone:          vmRef.Node(),
@@ -147,6 +139,10 @@ func (i *instances) InstanceMetadata(_ context.Context, node *v1.Node) (*cloudpr
 	return &cloudprovider.InstanceMetadata{}, nil
 }
 
+func (i *instances) getProviderID(region string, vmr *pxapi.VmRef) string {
+	return fmt.Sprintf("%s://%s/%d", ProviderName, region, vmr.VmId())
+}
+
 func (i *instances) getInstance(node *v1.Node) (*pxapi.VmRef, string, error) {
 	if !strings.HasPrefix(node.Spec.ProviderID, ProviderName) {
 		klog.V(4).Infof("instances.getInstance() node %s has foreign providerID: %s, skipped", node.Name, node.Spec.ProviderID)
@@ -154,19 +150,17 @@ func (i *instances) getInstance(node *v1.Node) (*pxapi.VmRef, string, error) {
 		return nil, "", fmt.Errorf("node %s has foreign providerID: %s", node.Name, node.Spec.ProviderID)
 	}
 
-	vmid, region, err := i.parseProviderID(node.Spec.ProviderID)
+	vm, region, err := i.parseProviderID(node.Spec.ProviderID)
 	if err != nil {
-		return nil, "", err
+		return nil, "", fmt.Errorf("instances.getInstance() error: %v", err)
 	}
 
-	vmRef := pxapi.NewVmRef(vmid)
-
 	px, err := i.c.GetProxmoxCluster(region)
 	if err != nil {
-		return nil, "", err
+		return nil, "", fmt.Errorf("instances.getInstance() error: %v", err)
 	}
 
-	vmInfo, err := px.client.GetVmInfo(vmRef)
+	vmInfo, err := px.GetVmInfo(vm)
 	if err != nil {
 		if strings.Contains(err.Error(), "not found") {
 			return nil, "", cloudprovider.InstanceNotFound
@@ -177,7 +171,7 @@ func (i *instances) getInstance(node *v1.Node) (*pxapi.VmRef, string, error) {
 
 	klog.V(5).Infof("instances.getInstance() vmInfo %+v", vmInfo)
 
-	return vmRef, region, nil
+	return vm, region, nil
 }
 
 func (i *instances) getInstanceType(vmRef *pxapi.VmRef, region string) (string, error) {
@@ -186,7 +180,7 @@ func (i *instances) getInstanceType(vmRef *pxapi.VmRef, region string) (string,
 		return "", err
 	}
 
-	vmInfo, err := px.client.GetVmInfo(vmRef)
+	vmInfo, err := px.GetVmInfo(vmRef)
 	if err != nil {
 		return "", err
 	}
@@ -198,16 +192,16 @@ func (i *instances) getInstanceType(vmRef *pxapi.VmRef, region string) (string,
 
 var providerIDRegexp = regexp.MustCompile(`^` + ProviderName + `://([^/]*)/([^/]+)$`)
 
-func (i *instances) parseProviderID(providerID string) (int, string, error) {
+func (i *instances) parseProviderID(providerID string) (*pxapi.VmRef, string, error) {
 	matches := providerIDRegexp.FindStringSubmatch(providerID)
 	if len(matches) != 3 {
-		return 0, "", fmt.Errorf("ProviderID \"%s\" didn't match expected format \"%s://region/InstanceID\"", providerID, ProviderName)
+		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 0, "", err
+		return nil, "", err
 	}
 
-	return vmID, matches[1], nil
+	return pxapi.NewVmRef(vmID), matches[1], nil
 }