[WIP] Self hosted: Add lgalloc integration setup #31796

bobbyiliev · 2025-03-07T18:14:51Z

Motivation

As per: https://github.com/MaterializeInc/cloud/issues/10880

Things that still need to be handled here:

OpenEBS helm chart installation via the Helm Terraform module: Add OpenEBS install option terraform-helm-materialize#28
Extend CI to build the bootstrap image and push it to the Materialize Docker Hub org. Open to suggestions regarding the image name!
Update the AWS Terraform test CI to actually test this by default
Extend the AWS Terraform modules to add support for installing the daemonst and support all this, initial implementation: [WIP] Self hosted: Add lgalloc integration setup terraform-aws-materialize#41
Docs

alex-hunt-materialize · 2025-03-11T16:01:10Z

misc/nvme-bootstrap/README.md

+
+1. Automatically detects NVMe instance store devices on your nodes
+2. Creates an LVM volume group from these devices
+3. Configures OpenEBS to provision persistent volumes from this storage


Suggested change

3. Configures OpenEBS to provision persistent volumes from this storage

3. Configures OpenEBS LVM Local-PV to provision persistent volumes from this storage

OpenEBS is a whole family of providers, we should be specific about which we're using.

alex-hunt-materialize · 2025-03-11T16:35:45Z

misc/nvme-bootstrap/README.md

+
+  # Disable Materialize Operator installation as it will require the storage class
+  # TODO: The module will support this configuration in the future
+  install_materialize_operator = false


We probably want to support this before releasing lgalloc support.

Started working on that already here: MaterializeInc/terraform-aws-materialize#41

alex-hunt-materialize · 2025-03-11T16:36:52Z

misc/nvme-bootstrap/README.md

+docker build -t your-registry/nvme-bootstrap:latest .
+
+# Push to your registry
+docker push your-registry/nvme-bootstrap:latest


If they are pushing it themselves, where do we configure which bootstrap image to use in the terraform?

Ah yes this is more of a note for myself while testing, next on my list is to add this to CI so we push the image to Docker hub. Open to suggestions on the Docker image name though!

alex-hunt-materialize · 2025-03-11T16:47:24Z

misc/nvme-bootstrap/README.md

+
+# Delete OpenEBS if no longer needed
+helm uninstall openebs -n openebs
+kubectl delete namespace openebs


What is this cleanup section cleaning up? If they ran the terraform, this would likely break their stack.

alex-hunt-materialize · 2025-03-11T16:58:00Z

misc/nvme-bootstrap/container/configure-disks.sh

+# Function to detect cloud provider
+detect_cloud_provider() {
+    # Check for AWS
+    if curl -s -m 5 http://169.254.169.254/latest/meta-data/ >/dev/null; then


The metadata service may not be available in all clusters. It is generally good security practice to disallow this.

Can we just pass in the cloud provider as an arg instead?

I agree, lets just pass this in, if some doesn't know what what cloud they're deploying into then that's on them.

I've changed this so now users can pass --cloud-provider same as we have it in orchestratord

alex-hunt-materialize · 2025-03-11T17:05:09Z

misc/nvme-bootstrap/container/configure-disks.sh

+# the Business Source License, use of this software will be governed
+# by the Apache License, Version 2.0.
+
+set -xeo pipefail


Suggested change

set -xeo pipefail

set -xeuo pipefail

alex-hunt-materialize · 2025-03-11T17:05:39Z

misc/nvme-bootstrap/container/manage-taints.sh

+        ;;
+    *)
+        echo "Usage: $0 [add|remove]"
+        exit 0  # Exit with success to avoid crash loop


As above, we probably want it to crash.

alex-hunt-materialize · 2025-03-11T17:06:26Z

misc/nvme-bootstrap/kubernetes/daemonset.yaml

+      serviceAccountName: nvme-setup-sa
+      initContainers:
+      - name: apply-taint
+        image: bobbyiliev/nvme-bootstrap:latest


This image is fine for testing, but probably should be moved to the materialize org with some automation for building/pushing it.

alex-hunt-materialize · 2025-03-11T17:07:14Z

misc/nvme-bootstrap/kubernetes/daemonset.yaml

+      initContainers:
+      - name: apply-taint
+        image: bobbyiliev/nvme-bootstrap:latest
+        command: ["/usr/local/bin/manage-taints.sh", "add"]


This should be set using a startup taint, not by the daemonset.

alex-hunt-materialize · 2025-03-11T17:08:21Z

misc/nvme-bootstrap/kubernetes/daemonset.yaml

+        effect: NoSchedule
+      - key: node-role.kubernetes.io/control-plane
+        operator: Exists
+        effect: NoSchedule


Why these other two tolerations?

jubrad

I'm not sure if this is the norm, but we could potentially make the nvme setup container an init container on the daemonset and have the main container be a pause image.

jubrad · 2025-03-11T13:17:01Z

misc/nvme-bootstrap/container/configure-disks.sh

+            # Handle both standard Linux and Bottlerocket paths
+            if [ -d "/.bottlerocket" ]; then
+                # Bottlerocket specific path
+                BOTTLEROCKET_ROOT="/.bottlerocket/rootfs"
+                mapfile -t SSD_NVME_DEVICE_LIST < <(lsblk --json --output-all | jq -r '.blockdevices[] | select(.model // empty | contains("Amazon EC2 NVMe Instance Storage")) | .path')
+                for device in "${SSD_NVME_DEVICE_LIST[@]}"; do
+                    nvme_devices+=("$BOTTLEROCKET_ROOT$device")
+                done
+            else
+                # Standard EC2 instances
+                mapfile -t nvme_devices < <(lsblk --json --output-all | jq -r '.blockdevices[] | select(.model // empty | contains("Amazon EC2 NVMe Instance Storage")) | .path')
+            fi
+            ;;
+        # Add more cloud providers here


If you're doing a case on cloud, I would turn this into a handle_aws function for readability... or an internal case/if for check_bottlerocket, check_linux.

jubrad · 2025-03-11T13:24:33Z

misc/nvme-bootstrap/container/configure-disks.sh

+# the Business Source License, use of this software will be governed
+# by the Apache License, Version 2.0.
+
+set -xeo pipefail


you may want to include u in this -u as well

-u When the shell tries to expand an unset parameter other than the '@' and '*' special parameters, it shall write a message to standard error and the expansion shall fail with the consequences specified in Section 2.8.1, Consequences of Shell Errors.

jubrad · 2025-03-11T13:29:37Z

misc/nvme-bootstrap/container/configure-disks.sh

+    return 0
+}
+
+# Main execution


nit: I think I prefer a main function function main() {...} to scope this logic.. it makes the script sourceable.

if [ "$0" == "${BASH_SOURCE[0]}" ]; then # shellcheck disable=SC2068 main $@ fi

jubrad · 2025-03-11T17:10:41Z

misc/nvme-bootstrap/container/manage-taints.sh

+    kubectl --server="https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT}" \
+            --token="$(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" \
+            --certificate-authority="/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" \
+            taint nodes "$NODE_NAME" "$TAINT_KEY=$TAINT_VALUE:$TAINT_EFFECT" --overwrite || {
+                echo "Warning: Failed to add taint, but continuing anyway"
+            }
+}


Already discussed, but we shouldn't add the taint here, that should be part of the node_group, or whatever defines nodes in the particular k8s cluster.

bobbyiliev · 2025-03-11T20:03:07Z

misc/nvme-bootstrap/README.md

@@ -0,0 +1,249 @@
+# OpenEBS NVMe Bootstrap for Materialize


I might delete this readme file all together, most of this is already available in the helm chart readme anyway, and the AWS specific implementation will be done via Terraform.

bobbyiliev · 2025-03-12T15:53:23Z

misc/nvme-bootstrap/kubernetes/daemonset.yaml

@@ -0,0 +1,72 @@
+# Copyright Materialize, Inc. and contributors. All rights reserved.


Do we also need those yaml files as well?

We have this as an example in Terraform already: https://github.com/MaterializeInc/terraform-aws-materialize/pull/41/files#diff-17bfdd719dbe7e71d2a1611e4312e71e140ca1c05f2ba6cddf62b7ec4d1c38c2R113

bobbyiliev requested a review from jubrad March 7, 2025 18:14

bobbyiliev mentioned this pull request Mar 7, 2025

[WIP] Self hosted: Add lgalloc integration setup MaterializeInc/terraform-aws-materialize#41

Closed

bobbyiliev requested review from kay-kim and a team March 7, 2025 18:17

[WIP] Self hosted: Add lgalloc integration setup

1aafe74

bobbyiliev force-pushed the self-hosted-lgalloc-setup branch from df2f61e to 1aafe74 Compare March 7, 2025 18:49

jubrad requested a review from alex-hunt-materialize March 11, 2025 13:14

alex-hunt-materialize reviewed Mar 11, 2025

View reviewed changes

jubrad reviewed Mar 11, 2025

View reviewed changes

Handle initial node taints via Terraform

60bd2f2

bobbyiliev force-pushed the self-hosted-lgalloc-setup branch from 181df29 to 60bd2f2 Compare March 11, 2025 19:09

bobbyiliev mentioned this pull request Mar 11, 2025

Add OpenEBS install option MaterializeInc/terraform-helm-materialize#28

Merged

bobbyiliev commented Mar 11, 2025

View reviewed changes

bobbyiliev commented Mar 12, 2025

View reviewed changes

bobbyiliev requested review from jubrad and alex-hunt-materialize March 12, 2025 18:07

bobbyiliev marked this pull request as draft March 19, 2025 13:23

bobbyiliev closed this Mar 27, 2025

	3. Configures OpenEBS to provision persistent volumes from this storage
	3. Configures OpenEBS LVM Local-PV to provision persistent volumes from this storage

		@@ -0,0 +1,72 @@
		# Copyright Materialize, Inc. and contributors. All rights reserved.

[WIP] Self hosted: Add lgalloc integration setup #31796

[WIP] Self hosted: Add lgalloc integration setup #31796

Uh oh!

Conversation

bobbyiliev commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jubrad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bobbyiliev commented Mar 7, 2025 •

edited

Loading