Merge pull request #170 from wanyufe/k8s-nodename

Use capi machine name as hostname and k8s node name

Author: rejoshed

controllers/cloudstackmachine_controller.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"math/rand"
 	"reflect"
+	"regexp"
 
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
@@ -40,6 +41,11 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
+var (
+	hostnameMatcher      = regexp.MustCompile(`\{\{\s*ds\.meta_data\.hostname\s*\}\}`)
+	failuredomainMatcher = regexp.MustCompile(`\{\{\s*ds\.meta_data\.failuredomain\s*\}\}`)
+)
+
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=cloudstackmachines,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=cloudstackmachines/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=cloudstackmachines/finalizers,verbs=update
@@ -169,7 +175,8 @@ func (r *CloudStackMachineReconciliationRunner) GetOrCreateVMInstance() (retRes
 		return ctrl.Result{}, errors.New("bootstrap secret data not yet set")
 	}
 
-	err := r.CSUser.GetOrCreateVMInstance(r.ReconciliationSubject, r.CAPIMachine, r.CSCluster, r.FailureDomain, r.AffinityGroup, string(data))
+	userData := processCustomMetadata(data, r)
+	err := r.CSUser.GetOrCreateVMInstance(r.ReconciliationSubject, r.CAPIMachine, r.CSCluster, r.FailureDomain, r.AffinityGroup, userData)
 
 	if err == nil && !controllerutil.ContainsFinalizer(r.ReconciliationSubject, infrav1.MachineFinalizer) { // Fetched or Created?
 		r.Log.Info("CloudStack instance Created", "instanceStatus", r.ReconciliationSubject.Status)
@@ -179,6 +186,14 @@ func (r *CloudStackMachineReconciliationRunner) GetOrCreateVMInstance() (retRes
 	return ctrl.Result{}, err
 }
 
+func processCustomMetadata(data []byte, r *CloudStackMachineReconciliationRunner) string {
+	// since cloudstack metadata does not allow custom data added into meta_data, following line is a workaround to specify a hostname name
+	// {{ ds.meta_data.hostname }} is expected to be used as a node name when kubelet register a node
+	userData := hostnameMatcher.ReplaceAllString(string(data), r.CAPIMachine.Name)
+	userData = failuredomainMatcher.ReplaceAllString(userData, r.FailureDomain.Spec.Name)
+	return userData
+}
+
 // ConfirmVMStatus checks the Instance's status for running state and requeues otherwise.
 func (r *CloudStackMachineReconciliationRunner) RequeueIfInstanceNotRunning() (retRes ctrl.Result, reterr error) {
 	if r.ReconciliationSubject.Status.InstanceState == "Running" {

controllers/cloudstackmachine_controller_test.go

@@ -72,6 +72,32 @@ var _ = Describe("CloudStackMachineReconciler", func() {
 				return false
 			}, timeout).WithPolling(pollInterval).Should(BeTrue())
 		})
+
+		It("Should replace ds.meta_data.hostname with capi machine name.", func() {
+			// Mock a call to GetOrCreateVMInstance and set the machine to running.
+			mockCloudClient.EXPECT().GetOrCreateVMInstance(
+				gomock.Any(), gomock.Any(), gomock.Any(),
+				gomock.Any(), gomock.Any(), gomock.Any()).Do(
+				func(arg1, _, _, _, _, userdata interface{}) {
+					Ω(userdata == dummies.CAPIMachine.Name).Should(BeTrue())
+					arg1.(*infrav1.CloudStackMachine).Status.InstanceState = "Running"
+				}).AnyTimes()
+
+			// Have to do this here or the reconcile call to GetOrCreateVMInstance may happen too early.
+			setupMachineCRDs()
+
+			// Eventually the machine should set ready to true.
+			Eventually(func() bool {
+				tempMachine := &infrav1.CloudStackMachine{}
+				key := client.ObjectKey{Namespace: dummies.ClusterNameSpace, Name: dummies.CSMachine1.Name}
+				if err := k8sClient.Get(ctx, key, tempMachine); err == nil {
+					if tempMachine.Status.Ready == true {
+						return true
+					}
+				}
+				return false
+			}, timeout).WithPolling(pollInterval).Should(BeTrue())
+		})
 	})
 
 	Context("With a fake ctrlRuntimeClient and no test Env at all.", func() {

pkg/cloud/instance.go

@@ -237,7 +237,7 @@ func (c *client) GetOrCreateVMInstance(
 	p := c.cs.VirtualMachine.NewDeployVirtualMachineParams(offeringID, templateID, fd.Spec.Zone.ID)
 	p.SetNetworkids([]string{fd.Spec.Zone.Network.ID})
 	setIfNotEmpty(csMachine.Name, p.SetName)
-	setIfNotEmpty(csMachine.Name, p.SetDisplayname)
+	setIfNotEmpty(capiMachine.Name, p.SetDisplayname)
 	setIfNotEmpty(diskOfferingID, p.SetDiskofferingid)
 	setIntIfPositive(csMachine.Spec.DiskOffering.CustomSize, p.SetSize)
 

pkg/cloud/instance_test.go

@@ -263,7 +263,12 @@ var _ = Describe("Instance", func() {
 					Return(&cloudstack.DeployVirtualMachineParams{})
 
 				deploymentResp := &cloudstack.DeployVirtualMachineResponse{Id: *dummies.CSMachine1.Spec.InstanceID}
-				vms.EXPECT().DeployVirtualMachine(gomock.Any()).Return(deploymentResp, nil)
+
+				vms.EXPECT().DeployVirtualMachine(gomock.Any()).Do(
+					func(p interface{}) {
+						displayName, _ := p.(*cloudstack.DeployVirtualMachineParams).GetDisplayname()
+						Ω(displayName == dummies.CAPIMachine.Name).Should(BeTrue())
+					}).Return(deploymentResp, nil)
 
 				Ω(client.GetOrCreateVMInstance(
 					dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")).

test/dummies/v1beta2/vars.go

@@ -398,11 +398,12 @@ func SetDummyIsoNetToNameOnly() {
 
 func SetDummyBootstrapSecretVar() {
 	BootstrapSecretName := "such-secret-much-wow"
+	BootstrapSecretValue := "{{ ds.meta_data.hostname }}"
 	BootstrapSecret = &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: ClusterNameSpace,
 			Name:      BootstrapSecretName},
-		Data: map[string][]byte{"value": make([]byte, 0)}}
+		Data: map[string][]byte{"value": []byte(BootstrapSecretValue)}}
 }
 
 // Sets cluster spec to specified network.