Add lgalloc support #36
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| #!/bin/bash | ||
| set -xeuo pipefail | ||
|
|
||
| echo "Starting GCP NVMe SSD setup" | ||
|
|
||
| # Install required tools | ||
| if command -v apt-get >/dev/null 2>&1; then | ||
| apt-get update | ||
| apt-get install -y lvm2 | ||
| elif command -v yum >/dev/null 2>&1; then | ||
| yum install -y lvm2 | ||
| else | ||
| echo "No package manager found. Please install required tools manually." | ||
| exit 1 | ||
| fi | ||
|
Comment on lines
+6
to
+15
There was a problem hiding this comment. We should definitely version lock this, or if there aren't dependencies, install from a binary. There was a problem hiding this comment. Just pinned this to a specific version. Regarding the dependencies: There was a problem hiding this comment. This is extremely unlikely to work long-term. If you want to pin it, you need to bake it into an image. The upstream repos will likely not contain that specific version for long. There was a problem hiding this comment. Sure, but I thought that we did not want to maintain our own image because of security concerns? There was a problem hiding this comment. Well, we can't pin the version this way, and the security concerns don't go away just because you're using someone else's image. We can either:
There was a problem hiding this comment. Just removed the version pin. When I was originally working on that custom container, I got pushback because of concerns that a lot of vulnerability scanner noise could come from bootstrap containers. But if we are all fine with this, I am happy to work on that bootstrap Docker image. |
||
|
|
||
| # Find NVMe devices | ||
| SSD_DEVICE_LIST=() | ||
|
|
||
| devices=$(find /dev/disk/by-id/ -name "google-local-ssd-*" 2>/dev/null || true) | ||
| if [ -n "$devices" ]; then | ||
| while read -r device; do | ||
| SSD_DEVICE_LIST+=("$device") | ||
| done <<<"$devices" | ||
| else | ||
| echo "ERROR: No Local SSD devices found at standard path /dev/disk/by-id/google-local-ssd-*" | ||
| echo "Please verify that local SSDs were properly attached to this instance" | ||
| echo "See: https://cloud.google.com/compute/docs/disks/local-ssd" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Check if any of the devices are already in use by LVM | ||
| for device in "${SSD_DEVICE_LIST[@]}"; do | ||
| if pvdisplay "$device" &>/dev/null; then | ||
| echo "$device is already part of LVM, skipping setup" | ||
| exit 0 | ||
| fi | ||
| done | ||
|
|
||
| # Create physical volumes | ||
| for device in "${SSD_DEVICE_LIST[@]}"; do | ||
| echo "Creating physical volume on $device" | ||
| pvcreate -f "$device" | ||
| done | ||
|
|
||
| # Create volume group | ||
| echo "Creating volume group instance-store-vg" | ||
| vgcreate instance-store-vg "${SSD_DEVICE_LIST[@]}" | ||
bobbyiliev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| # Display results | ||
| pvs | ||
| vgs | ||
|
|
||
| echo "Disk setup completed" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we just base this one entirely on
var.enable_disk_supportand not have this extra var?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the idea was that users only set
enable_disk_support, and the rest use defaults. I addeddisk_support_configjust for extra flexibility if we ever need to override things. Happy to simplify if we want to keep it more opinionated though, up to you!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part of the PR is more or less a copy and paste from the AWS implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok.... let's just keep this