Support multi node proxmox (use websocket instead of ssh)

README.md

@@ -24,15 +24,10 @@ clusterctl init --infrastructure=proxmox:v0.2.3 --config https://raw.githubuserc
 2. Create your first workload cluster
 ```sh
 # export env variables
-export CONTROLPLANE_HOST=X.X.X.X                          # control-plane vip
+export CONTROLPLANE_HOST=X.X.X.X                   # control-plane vip
 export PROXMOX_URL=https://X.X.X.X:8006/api2/json
-# export PROXMOX_PASSWORD=password                        # (optional)
-# export PROXMOX_USER=user@pam                            # (optional)
-export PROXMOX_TOKENID='root@pam!api-token-id'            # (optional)
-export PROXMOX_SECRET=aaaaaaaa-bbbb-cccc-dddd-ee12345678  # (optional)
-export NODE_URL=node.ssh.url:22
-export NODE_USER=node-ssh-user
-export NODE_PASSWORD=node-ssh-password
+export PROXMOX_PASSWORD=password
+export PROXMOX_USER=user@pam
 
 # generate manifests (available flags: --target-namespace, --kubernetes-version, --control-plane-machine-count, --worker-machine-count)
 clusterctl generate cluster cappx-test --control-plane-machine-count=3 --infrastructure=proxmox:v0.2.3 --config https://raw.githubusercontent.com/sp-yduck/cluster-api-provider-proxmox/main/clusterctl.yaml > cappx-test.yaml
@@ -62,9 +57,11 @@ kubectl delete cluster cappx-test
 
 ## Fetures
 
-- No need to prepare vm templates. You can specify any vm image in `ProxmoxMachine.Spec.Image`.
+- No need to prepare vm templates. You can specify any vm image in `ProxmoxMachine.Spec.Image`. CAPPX bootstrap your vm from scratch.
 
-- Supports custom cloud-config (user data). CAPPX uses ssh for bootstrapping nodes so it can applies custom cloud-config that can not be achieved by only Proxmox API.
+- Supports qcow2 image format. CAPPX uses VNC websocket for downloading/installing node images so it can support raw image format not ISO (Proxmox API can only support ISO)
+
+- Supports custom cloud-config (user data). CAPPX uses VNC websockert for bootstrapping nodes so it can applies custom cloud-config that can not be achieved by only Proxmox API.
 
 ### Node Images
 
@@ -99,7 +96,7 @@ This project aims to follow the Cluster API [Provider contract](https://cluster-
 
 ### ProxmoxCluster
 
-Because Proxmox-VE does not provide LBaaS solution, CAPPX does not follow the [typical infra-cluster logic](https://cluster-api.sigs.k8s.io/developer/providers/cluster-infrastructure.html#behavior). ProxmoxCluster controller reconciles only Proxmox storages used for instances. You need to prepare control plane load balancer by yourself if you creates HA control plane workload cluster.
+Because Proxmox-VE does not provide LBaaS solution, CAPPX does not follow the [typical infra-cluster logic](https://cluster-api.sigs.k8s.io/developer/providers/cluster-infrastructure.html#behavior). ProxmoxCluster controller reconciles only Proxmox storages used for instances. You need to prepare control plane load balancer by yourself if you creates HA control plane workload cluster. In the [cluster-template.yaml](./templates/cluster-template.yaml), you can find HA control plane example with [kube-vip](https://github.com/kube-vip/kube-vip).
 
 ### ProxmoxMachine
 

api/v1beta1/proxmoxcluster_types.go

@@ -38,9 +38,6 @@ type ProxmoxClusterSpec struct {
 	// ServerRef is used for configuring Proxmox client
 	ServerRef ServerRef `json:"serverRef"`
 
-	// NodesRef contains reference of nodes used for ProxmoxCluster
-	NodeRefs []NodeRef `json:"nodeRefs,omitempty"`
-
 	// storage is for proxmox storage used by vm instances
 	// +optional
 	Storage Storage `json:"storage"`

cloud/cloudinit/user.go

@@ -25,9 +25,9 @@ func GenerateUserYaml(config infrav1.User) (string, error) {
 	return fmt.Sprintf("#cloud-config\n%s", string(b)), nil
 }
 
-func MergeUsers(a, b infrav1.User) (*infrav1.User, error) {
-	if err := mergo.Merge(&a, b, mergo.WithAppendSlice); err != nil {
+func MergeUsers(a, b *infrav1.User) (*infrav1.User, error) {
+	if err := mergo.Merge(a, b, mergo.WithAppendSlice); err != nil {
 		return nil, err
 	}
-	return &a, nil
+	return a, nil
 }

cloud/cloudinit/user_test.go

@@ -67,7 +67,7 @@ func TestMergeUsers(t *testing.T) {
 		User:   "override-user",
 		RunCmd: []string{"command A", "command B", "command C"},
 	}
-	c, err := cloudinit.MergeUsers(a, b)
+	c, err := cloudinit.MergeUsers(&a, &b)
 	if err != nil {
 		t.Errorf("failed to merge cloud init user data: %v", err)
 	}

cloud/interfaces.go

@@ -7,7 +7,6 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 
 	infrav1 "github.com/sp-yduck/cluster-api-provider-proxmox/api/v1beta1"
-	"github.com/sp-yduck/cluster-api-provider-proxmox/cloud/scope"
 )
 
 type Reconciler interface {
@@ -17,7 +16,6 @@ type Reconciler interface {
 
 type Client interface {
 	CloudClient() *proxmox.Service
-	RemoteClient() *scope.SSHClient
 }
 
 type Cluster interface {

cloud/scope/clients.go

@@ -29,7 +29,6 @@ import (
 
 type ProxmoxServices struct {
 	Compute *proxmox.Service
-	Remote  *SSHClient
 }
 
 func newComputeService(ctx context.Context, serverRef infrav1.ServerRef, crClient client.Client) (*proxmox.Service, error) {
@@ -53,30 +52,3 @@ func newComputeService(ctx context.Context, serverRef infrav1.ServerRef, crClien
 
 	return proxmox.NewService(serverRef.Endpoint, authConfig, true)
 }
-
-func newRemoteClient(ctx context.Context, secretRef *infrav1.ObjectReference, crClient client.Client) (*SSHClient, error) {
-	if secretRef == nil {
-		return nil, errors.New("failed to get proxmox client form nil secretRef")
-	}
-
-	var secret corev1.Secret
-	key := client.ObjectKey{Namespace: secretRef.Namespace, Name: secretRef.Name}
-	if err := crClient.Get(ctx, key, &secret); err != nil {
-		return nil, err
-	}
-
-	nodeurl, ok := secret.Data["NODE_URL"]
-	if !ok {
-		return nil, errors.Errorf("failed to fetch NODE_URL from Secret : %v", key)
-	}
-	nodeuser, ok := secret.Data["NODE_USER"]
-	if !ok {
-		return nil, errors.Errorf("failed to fetch PROXMOX_USER from Secret : %v", key)
-	}
-	nodepassword, ok := secret.Data["NODE_PASSWORD"]
-	if !ok {
-		return nil, errors.Errorf("failed to fetch PROXMOX_PASSWORD from Secret : %v", key)
-	}
-
-	return NewSSHClient(string(nodeurl), string(nodeuser), string(nodepassword))
-}

cloud/scope/cluster.go

@@ -52,15 +52,6 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc
 		params.ProxmoxServices.Compute = computeSvc
 	}
 
-	if params.ProxmoxServices.Remote == nil {
-		// current CAPPX is compatible with only single node proxmox cluster
-		remote, err := newRemoteClient(ctx, params.ProxmoxCluster.Spec.NodeRefs[0].SecretRef, params.Client)
-		if err != nil {
-			return nil, errors.Errorf("failed to create remote client: %v", err)
-		}
-		params.ProxmoxServices.Remote = remote
-	}
-
 	helper, err := patch.NewHelper(params.ProxmoxCluster, params.Client)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to init patch helper")
@@ -79,11 +70,6 @@ func populateNamespace(proxmoxCluster *infrav1.ProxmoxCluster) {
 	if proxmoxCluster.Spec.ServerRef.SecretRef.Namespace == "" {
 		proxmoxCluster.Spec.ServerRef.SecretRef.Namespace = proxmoxCluster.Namespace
 	}
-	for i, nodeRef := range proxmoxCluster.Spec.NodeRefs {
-		if nodeRef.SecretRef.Namespace == "" {
-			proxmoxCluster.Spec.NodeRefs[i].SecretRef.Namespace = proxmoxCluster.Namespace
-		}
-	}
 }
 
 type ClusterScope struct {
@@ -114,10 +100,6 @@ func (s *ClusterScope) CloudClient() *proxmox.Service {
 	return s.ProxmoxServices.Compute
 }
 
-func (s *ClusterScope) RemoteClient() *SSHClient {
-	return s.ProxmoxServices.Remote
-}
-
 func (s *ClusterScope) Close() error {
 	return s.PatchObject()
 }

cloud/scope/machine.go

@@ -82,10 +82,6 @@ func (m *MachineScope) CloudClient() *proxmox.Service {
 	return m.ClusterGetter.CloudClient()
 }
 
-func (m *MachineScope) RemoteClient() *SSHClient {
-	return m.ClusterGetter.Remote
-}
-
 func (m *MachineScope) GetStorage() infrav1.Storage {
 	return m.ClusterGetter.ProxmoxCluster.Spec.Storage
 }

cloud/services/compute/instance/cloudinit.go

@@ -5,7 +5,7 @@
 	"fmt"
 
 	"github.com/pkg/errors"
-	"k8s.io/klog/v2"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	infrav1 "github.com/sp-yduck/cluster-api-provider-proxmox/api/v1beta1"
 	"github.com/sp-yduck/cluster-api-provider-proxmox/cloud/cloudinit"
@@ -16,21 +16,25 @@
 )
 
 // reconcileCloudInit
-func (s *Service) reconcileCloudInit(bootstrap string) error {
+func (s *Service) reconcileCloudInit(ctx context.Context) error {
+	log := log.FromContext(ctx)
+	log.Info("Reconciling cloud init")
+
 	// user
-	if err := s.reconcileCloudInitUser(bootstrap); err != nil {
+	if err := s.reconcileCloudInitUser(ctx); err != nil {
 		return err
 	}
+
 	return nil
 }
 
 // delete CloudConfig
 func (s *Service) deleteCloudConfig(ctx context.Context) error {
 	storageName := s.scope.GetStorage().Name
 	path := userSnippetPath(s.scope.Name())
 	volumeID := fmt.Sprintf("%s:%s", storageName, path)
 
 	node, err := s.client.Node(ctx, s.scope.NodeName())
 	if err != nil {
 		return err
 	}
@@ -42,42 +46,62 @@
 	return storage.DeleteVolume(ctx, volumeID)
 }
 
-func (s *Service) reconcileCloudInitUser(bootstrap string) error {
-	vmName := s.scope.Name()
-	storagePath := s.scope.GetStorage().Path
-	config := s.scope.GetCloudInit().User
+func (s *Service) reconcileCloudInitUser(ctx context.Context) error {
+	log := log.FromContext(ctx)
 
+	// cloud init from bootstrap provider
+	bootstrap, err := s.scope.GetBootstrapData()
+	if err != nil {
+		log.Error(err, "Error getting bootstrap data for machine")
+		return errors.Wrap(err, "failed to retrieve bootstrap data")
+	}
 	bootstrapConfig, err := cloudinit.ParseUser(bootstrap)
 	if err != nil {
 		return err
 	}
-	base := baseUserData(vmName)
-	if config != nil {
-		base, err = cloudinit.MergeUsers(*config, *base)
-		if err != nil {
-			return err
-		}
-	}
-	cloudConfig, err := cloudinit.MergeUsers(*base, *bootstrapConfig)
+
+	vmName := s.scope.Name()
+	cloudConfig, err := mergeUserDatas(bootstrapConfig, baseUserData(vmName), s.scope.GetCloudInit().User)
 	if err != nil {
 		return err
 	}
+
 	configYaml, err := cloudinit.GenerateUserYaml(*cloudConfig)
 	if err != nil {
 		return err
 	}
 
-	klog.Info(configYaml)
-
 	// to do: should be set via API
-	out, err := s.remote.RunWithStdin(fmt.Sprintf("tee %s/%s", storagePath, userSnippetPath(vmName)), configYaml)
+	vnc, err := s.vncClient(s.scope.NodeName())
 	if err != nil {
-		return errors.Errorf("ssh command error : %s : %v", out, err)
+		return err
+	}
+	defer vnc.Close()
+	filePath := fmt.Sprintf("%s/%s", s.scope.GetStorage().Path, userSnippetPath(vmName))
+	if err := vnc.WriteFile(context.TODO(), configYaml, filePath); err != nil {
+		return errors.Errorf("failed to write file error : %v", err)
 	}
 
 	return nil
 }
 
+// a and b must not be nil
+// only c can be nil
+func mergeUserDatas(a, b, c *infrav1.User) (*infrav1.User, error) {
+	var err error
+	if c != nil {
+		b, err = cloudinit.MergeUsers(b, c)
+		if err != nil {
+			return nil, err
+		}
+	}
+	a, err = cloudinit.MergeUsers(a, b)
+	if err != nil {
+		return nil, err
+	}
+	return a, err
+}
+
 func userSnippetPath(vmName string) string {
 	return fmt.Sprintf(userSnippetPathFormat, vmName)
 }