Onpremises provisioning for all cluster resources was implemented #624

testisnullus · 2023-11-15T15:33:48Z

No description provided.

DoodgeMatvey · 2023-11-16T15:50:35Z

apis/clusters/v1beta1/kafka_webhook.go

+		osDiskSizeMatched, err := regexp.Match(models.StorageRegExp, []byte(k.Spec.OnPremisesSpec.OSDiskSize))
+		if !osDiskSizeMatched || err != nil {
+			return fmt.Errorf("disk size field for node OS must fit pattern: %s",
+				models.StorageRegExp)
+		}
+
+		dataDiskSizeMatched, err := regexp.Match(models.StorageRegExp, []byte(k.Spec.OnPremisesSpec.DataDiskSize))
+		if !dataDiskSizeMatched || err != nil {
+			return fmt.Errorf("disk size field for storring cluster data must fit pattern: %s",
+				models.StorageRegExp)
+		}
+
+		nodeMemoryMatched, err := regexp.Match(models.MemoryRegExp, []byte(k.Spec.OnPremisesSpec.DataDiskSize))
+		if !nodeMemoryMatched || err != nil {
+			return fmt.Errorf("node memory field must fit pattern: %s",
+				models.MemoryRegExp)
+		}
+
+		if k.Spec.PrivateNetworkCluster {
+			if k.Spec.OnPremisesSpec.SSHGatewayCPU == 0 || k.Spec.OnPremisesSpec.SSHGatewayMemory == "" {
+				return fmt.Errorf("fields SSHGatewayCPU and SSHGatewayMemory must not be empty")
+			}
+			sshGatewayMemoryMatched, err := regexp.Match(models.MemoryRegExp, []byte(k.Spec.OnPremisesSpec.DataDiskSize))
+			if !sshGatewayMemoryMatched || err != nil {
+				return fmt.Errorf("ssh gateway memory field must fit pattern: %s",
+					models.MemoryRegExp)
+			}
+		}


Can you move it to separate function to not duplicate such code a lot of times?

Yep, will fix

DoodgeMatvey · 2023-11-16T15:50:54Z

apis/clusters/v1beta1/kafka_types.go

+func (k *Kafka) NewExposePorts() []k8scorev1.ServicePort {
+	var ports []k8scorev1.ServicePort
+	ports = []k8scorev1.ServicePort{{
+		Name: models.SSH,
+		Port: models.Port22,
+		TargetPort: intstr.IntOrString{
+			Type:   intstr.Int,
+			IntVal: models.Port22,
+		},
+	},
+	}
+
+	if !k.Spec.PrivateNetworkCluster {
+		additionalPorts := []k8scorev1.ServicePort{
+			{
+				Name: models.KafkaClient,
+				Port: models.Port9092,
+				TargetPort: intstr.IntOrString{
+					Type:   intstr.Int,
+					IntVal: models.Port9092,
+				},
+			},
+			{
+				Name: models.KafkaControlPlane,
+				Port: models.Port9093,
+				TargetPort: intstr.IntOrString{
+					Type:   intstr.Int,
+					IntVal: models.Port9093,
+				},
+			},
+		}
+		if k.Spec.ClientToClusterEncryption {
+			sslPort := k8scorev1.ServicePort{
+				Name: models.KafkaBroker,
+				Port: models.Port9094,
+				TargetPort: intstr.IntOrString{
+					Type:   intstr.Int,
+					IntVal: models.Port9094,
+				},
+			}
+			additionalPorts = append(additionalPorts, sslPort)
+		}
+		ports = append(ports, additionalPorts...)
+	}
+
+	return ports
+}
+
+func (k *Kafka) NewHeadlessPorts() []k8scorev1.ServicePort {
+	ports := []k8scorev1.ServicePort{
+		{
+			Name: models.KafkaClient,
+			Port: models.Port9092,
+			TargetPort: intstr.IntOrString{
+				Type:   intstr.Int,
+				IntVal: models.Port9092,
+			},
+		},
+		{
+			Name: models.KafkaControlPlane,
+			Port: models.Port9093,
+			TargetPort: intstr.IntOrString{
+				Type:   intstr.Int,
+				IntVal: models.Port9093,
+			},
+		},
+	}
+	if k.Spec.ClientToClusterEncryption {
+		kafkaBrokerPort := k8scorev1.ServicePort{
+			Name: models.KafkaBroker,
+			Port: models.Port9094,
+			TargetPort: intstr.IntOrString{
+				Type:   intstr.Int,
+				IntVal: models.Port9094,
+			},
+		}
+		ports = append(ports, kafkaBrokerPort)
+	}
+
+	return ports
+}


Can you move it to separate function and just path values you need to not duplicate such code a lot of times?

ribaraka · 2023-11-20T12:20:46Z

apis/clusters/v1beta1/structs.go

+	NodeCPU            int64      `json:"nodeCPU"`
+	NodeMemory         string     `json:"nodeMemory"`
+	OSImageURL         string     `json:"osImageURL"`
+	CloudInitScriptRef *Reference `json:"cloudInitScriptNamespacedName"`


how about changing the JSON tag to json:"cloudInitScriptRef"?

RostislavPorohnya · 2023-11-21T13:58:32Z

apis/clusters/v1beta1/cadence_types.go

+		sslPort := k8scorev1.ServicePort{
+			Name: models.CadenceGRPC,
+			Port: models.Port7833,
+			TargetPort: intstr.IntOrString{
+				Type:   intstr.Int,
+				IntVal: models.Port7833,
+			},
+		}
+		headlessPorts = append(headlessPorts, sslPort)


Unneded variable. I suggest:
headlessPorts = append(headlessPorts, k8scorev1.ServicePort{...})

The same with getExpostPorts method

It's more readable I think

RostislavPorohnya · 2023-11-21T14:03:31Z

apis/clusters/v1beta1/cadence_webhook.go

+	if len(c.Spec.DataCentres) > 1 && c.Spec.OnPremisesSpec != nil {
+		return fmt.Errorf("on-premises cluster can be provisioned with only one data centre")


Lets check len != 1 && onprem

We have this kind of check above:

if len(c.Spec.DataCentres) == 0 { return fmt.Errorf("data centres field is empty") }

RostislavPorohnya · 2023-11-21T14:11:26Z

controllers/clusters/cadence_controller.go

+			return reconcile.Result{}, err
+		}
+
+		bootstrap := newOnPremiseBootstrap(


newOnPremiseBootstrap -> newOnPremisesBootstrap

RostislavPorohnya · 2023-11-21T14:17:51Z

controllers/clusters/on_premises.go

+	}
+}
+
+func handleCreateOnPremisesClusterResources(ctx context.Context, b *onPremiseBootstrap) error {


onPremiseBootstrap -> onPremisesBootstrap

RostislavPorohnya · 2023-11-21T14:18:19Z

controllers/clusters/on_premises.go

+}
+
+func handleCreateOnPremisesClusterResources(ctx context.Context, b *onPremiseBootstrap) error {
+	if len(b.ClusterStatus.DataCentres) < 1 {


lets check len == 0

RostislavPorohnya · 2023-11-21T14:23:28Z

controllers/clusters/on_premises.go

+	vmis := &virtcorev1.VirtualMachineInstanceList{}
+	err = K8sClient.List(ctx, vmis, &client.ListOptions{
+		LabelSelector: labels.SelectorFromSet(map[string]string{
+			models.ClusterIDLabel: clusterID,
+		}),
+		Namespace: ns,
+	})
+	if err != nil {
+		return err
+	}
+
+	for _, vmi := range vmis.Items {
+		err = K8sClient.Delete(ctx, &vmi)
+		if err != nil {
+			return err
+		}
+
+		patch := client.MergeFrom(vmi.DeepCopy())
+		controllerutil.RemoveFinalizer(&vmi, models.DeletionFinalizer)
+		err = K8sClient.Patch(ctx, &vmi, patch)
+		if err != nil {
+			return err
+		}
+	}


VMIs are deleted when their VM resource is deleted isn't?
If so we don't need to explicitly delete VMIs

Let's be totally sure that all resources are deleted

ribaraka · 2023-11-28T14:36:13Z

doc/clusters/cassandra.md

+### CloudInitScript
+
+cloud-init.sh:
+```shell
+#!/bin/bash
+
+export NEW_PASS="qwerty12345"
+export SSH_PUB_KEY=""
+export BOOTSTRAP_SSH_KEY="ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAEAQDgeaO3zkY5v1dww3fFONPzUnEgIqJ4kUK0Usu8iFdp+TWIulw9dDeQHa+PdWXP97l5Vv1mG9ipqShEIu7/2bp13KxSblWX4iV1MYZbtimhY3UDOsPn1G3E1Ipis6y+/tosDAm8LoWaGEMcLuE5UjP6gs6K57rCEjkVGjg7vjhIypAMC0J2N2+CxK9o/Y1+LZAec+FL5cmSJoajoo9y/VYJjz/a52jJ93wRafD2uu6ObAl5gkN/+gqY4IJFSMx20sPsIRXdbiBNDqiap56ibHHPKTeZhRbdXvZfjYkHtutXnSM2xn7BjnV8CguISxS3rXlzlzRVYwSUjnKUf5SKBbeyZbCokx271vCeUd5EXfHphvW6FIOna2AI5lpCSYkw5Kho3HaPi2NjXJ9i2dCr1zpcZpCiettDuaEjxR0Cl4Jd6PrAiAEZ0Ns0u2ysVhudshVzQrq6qdd7W9/MLjbDIHdTToNjFLZA6cbE0MQf18LXwJAl+G/QrXgcVaiopUmld+68JL89Xym55LzkMhI0NiDtWufawd/NiZ6jm13Z3+atvnOimdyuqBYeFWgbtcxjs0yN9v7h7PfPh6TbiQYFx37DCfQiIucqx1GWmMijmd7HMY6Nv3UvnoTUTSn4yz1NxhEpC61N+iAInZDpeJEtULSzlEMWlbzL4t5cF+Rm1dFdq3WpZt1mi8F4DgrsgZEuLGAw22RNW3++EWYFUNnJXaYyctPrMpWQktr4DB5nQGIHF92WR8uncxTNUXfWuT29O9e+bFYh1etmq8rsCoLcxN0zFHWvcECK55aE+47lfNAR+HEjuwYW10mGU/pFmO0F9FFmcQRSw4D4tnVUgl3XjKe3bBiTa4lUrzrKkLZ6n9/buW2e7c3jbjmXdPh2R+2Msr/vfuWs9glxQf+CYEbBW6Ye4pekIyI77SaB/bVhaHtXutKxm+QWdNle8aeqiA8Ji1Ml+s75vIg+n5v6viCnl5aV33xHRFpGQJzj2ktsXl9P9d5kgal9eXJYTywC2SnVbZVLb6FGN4kPZTVwX1f+u7v7JCm4YWlbQZtwwiXKjs99AVtQnBWqQvUH5sFUkVXlHA1Y9W6wlup0r+F6URL+7Yw+d0dHByfevrJg3pvmpLb3sEpjIAZodW3dIUReE7Ku3s/q/O9foFnfRBnCcZ2QsnxI5pqNrbrundD1ApOnNXEvICvPXHBBQ44cW0hzAO+WxY5VxyG8y/kXnb48G9efkIQFkNaITJrU9SiOk6bFP4QANdS/pmaSLjJIsHixa+7vmYjRy1SVoQ/39vDUnyCbqKtO56QMH32hQLRO3Vk7NVG6o4dYjFkiaMSaqVlHKMkJQHVzlK2PW9/fjVXfkAHmmhoD debian"
+echo "debian:$NEW_PASS" | chpasswd
+echo "root:$NEW_PASS" | sudo chpasswd root
+sudo echo "$SSH_PUB_KEY" > /home/debian/.ssh/authorized_keys
+sudo echo "$BOOTSTRAP_SSH_KEY" >> /home/debian/.ssh/authorized_keys
+sudo chown -R debian: /home/debian/.ssh
+sudo cp /usr/share/doc/apt/examples/sources.list /etc/apt/sources.list
+data_device=$(lsblk -dfn -o NAME,SERIAL | awk '$2 == "DATADISK" {print $1}')
+sudo mkfs -t ext4 /dev/"${data_device}"
+```
+
+Create the base64 encoded string with the script using `cat cloud-init.sh | base64 -w0` command and create a secret using this yaml manifest:
+
+```yaml
+apiVersion: v1
+kind: Secret
+metadata:
+  name: cloud-init-secret
+data:
+  userdata: <base64 encoded string>
+```
+
+Use `kubectl apply -f cloud-init-secret.yaml` command to create the secret.
+


I suggest moving it below the on-premise Cluster create flow, specifically under the section:

Or if you want to create an on-premises cluster:

RostislavPorohnya · 2023-11-28T15:28:44Z

doc/clusters/cassandra.md

+### ReferenceObject
+
+| Field     | Type   | Description                                       |
+|-----------|--------|---------------------------------------------------|
+| name      | string | Name of the cloud-init secret.                    |
+| namespace | string | Namespace in which the cloud-init secret located. |
+


If we don't plan to add anything to reference, I think we should use namespacedName instead

testisnullus requested review from RostislavPorohnya and ribaraka November 15, 2023 15:33

testisnullus self-assigned this Nov 15, 2023

testisnullus added the enhancement New feature or request label Nov 15, 2023

testisnullus force-pushed the onpremises-provisioning branch 3 times, most recently from e446339 to 73e6f7a Compare November 16, 2023 15:46

DoodgeMatvey requested changes Nov 16, 2023

View reviewed changes

testisnullus force-pushed the onpremises-provisioning branch from 73e6f7a to 38e8003 Compare November 17, 2023 08:47

testisnullus requested a review from DoodgeMatvey November 17, 2023 08:48

testisnullus force-pushed the onpremises-provisioning branch 3 times, most recently from a5270a5 to cbe1995 Compare November 17, 2023 13:30

DoodgeMatvey approved these changes Nov 17, 2023

View reviewed changes

testisnullus force-pushed the onpremises-provisioning branch 3 times, most recently from 201c44e to 5ae02d4 Compare November 19, 2023 12:13

ribaraka requested changes Nov 20, 2023

View reviewed changes

testisnullus force-pushed the onpremises-provisioning branch from 5ae02d4 to c2fc134 Compare November 20, 2023 14:12

testisnullus requested a review from ribaraka November 20, 2023 14:12

ribaraka approved these changes Nov 20, 2023

View reviewed changes

ribaraka changed the title ~~[WIP] Onpremises provisioning for all cluster resources was implemented~~ Onpremises provisioning for all cluster resources was implemented Nov 20, 2023

RostislavPorohnya reviewed Nov 21, 2023

View reviewed changes

testisnullus force-pushed the onpremises-provisioning branch 2 times, most recently from 9c28f6b to a29cde3 Compare November 21, 2023 14:41

testisnullus requested review from RostislavPorohnya, ribaraka and DoodgeMatvey November 21, 2023 16:27

testisnullus force-pushed the onpremises-provisioning branch 2 times, most recently from 8b59b35 to 350d2f0 Compare November 28, 2023 10:30

testisnullus force-pushed the onpremises-provisioning branch from 350d2f0 to b82a311 Compare November 28, 2023 13:44

ribaraka reviewed Nov 28, 2023

View reviewed changes

testisnullus force-pushed the onpremises-provisioning branch from b82a311 to 9366fa1 Compare November 28, 2023 14:42

ribaraka approved these changes Nov 28, 2023

View reviewed changes

RostislavPorohnya reviewed Nov 28, 2023

View reviewed changes

testisnullus force-pushed the onpremises-provisioning branch from 9366fa1 to 9292b20 Compare November 28, 2023 15:32

testisnullus force-pushed the onpremises-provisioning branch from 9292b20 to 6d3121b Compare December 11, 2023 12:37

vendor

b504941

testisnullus force-pushed the onpremises-provisioning branch from 6d3121b to 0f36b70 Compare December 11, 2023 12:42

On premises flow was added for all cluster resources

d7fcad4

testisnullus force-pushed the onpremises-provisioning branch from 0f36b70 to d7fcad4 Compare December 11, 2023 15:24

taaraora approved these changes Dec 11, 2023

View reviewed changes

taaraora merged commit 94adf58 into main Dec 11, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Onpremises provisioning for all cluster resources was implemented #624

Onpremises provisioning for all cluster resources was implemented #624

testisnullus commented Nov 15, 2023

DoodgeMatvey Nov 16, 2023

testisnullus Nov 17, 2023

DoodgeMatvey Nov 16, 2023

testisnullus Nov 17, 2023 •

edited

Loading

ribaraka Nov 20, 2023

testisnullus Nov 20, 2023

RostislavPorohnya Nov 21, 2023

RostislavPorohnya Nov 21, 2023

testisnullus Nov 21, 2023

RostislavPorohnya Nov 21, 2023

testisnullus Nov 21, 2023

RostislavPorohnya Nov 21, 2023

RostislavPorohnya Nov 21, 2023

RostislavPorohnya Nov 21, 2023

RostislavPorohnya Nov 21, 2023

testisnullus Nov 21, 2023

ribaraka Nov 28, 2023

RostislavPorohnya Nov 28, 2023

		if len(c.Spec.DataCentres) > 1 && c.Spec.OnPremisesSpec != nil {
		return fmt.Errorf("on-premises cluster can be provisioned with only one data centre")

Onpremises provisioning for all cluster resources was implemented #624

Onpremises provisioning for all cluster resources was implemented #624

Conversation

testisnullus commented Nov 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

testisnullus Nov 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

testisnullus Nov 17, 2023 •

edited

Loading