Arvados

In the create-ebs-volume patch, the relationship between $device and the new $nvme_device is mysterious.

• If there's no available $device, we error out, but if there's no available $nvme device we continue with nvme_device=""
• We call "attach-volume --device $device" but then we wait for $nvme_device to appear

My impression is that the amazon-provided script is already very fragile and I don't think we expect to fix that here, just hoping to avoid making it worse with our patch. It looks like the recommended way to find the device id for a known volume ID is lsblk -o +SERIAL. https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/nvme-ebs-volumes.html

It looks like usr-local-bin-ensure-encrypted-partitions-aws-ebs-autoscale.sh does not actually do encrypted partitions... if that's true, it seems like bad form to still call it "ensure encrypted partitions", although I see how that makes it convenient to install. Is this something worth mentioning in the docs, too?

IamInstanceProfile should be IAMInstanceProfile

Autoscaler bit in docs should mention you need to use an IAMInstanceProfile.

Tom Clegg wrote:

My impression is that the amazon-provided script is already very fragile and I don't think we expect to fix that here, just hoping to avoid making it worse with our patch. It looks like the recommended way to find the device id for a known volume ID is lsblk -o +SERIAL. https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/nvme-ebs-volumes.html

Ah, yeah, thanks, that's a nicer way to do it, I've updated the patch.

In the create-ebs-volume patch, the relationship between $device and the new $nvme_device is mysterious.

• If there's no available $device, we error out, but if there's no available $nvme device we continue with nvme_device=""
• We call "attach-volume --device $device" but then we wait for $nvme_device to appear

These are moot now that I've refactored the patch.

It looks like usr-local-bin-ensure-encrypted-partitions-aws-ebs-autoscale.sh does not actually do encrypted partitions... if that's true, it seems like bad form to still call it "ensure encrypted partitions", although I see how that makes it convenient to install. Is this something worth mentioning in the docs, too?

Well, the volume that is attached is actually encrypted, but at the AWS level, because we use the default config for ebs-autoscale:

{
    "mountpoint": "/tmp",
    "filesystem": "lvm.ext4",
    "lvm": {
      "volume_group": "autoscale_vg",
      "logical_volume": "autoscale_lv" 
    },
    "volume": {
        "type": "gp3",
        "iops": 3000,
        "encrypted": 1
    },
    "detection_interval": 2,
    "limits": {
        "max_ebs_volume_size": 1500,
        "max_logical_volume_size": 8000,
        "max_ebs_volume_count": 16
    },
    "logging": {
        "log_file": "/var/log/ebs-autoscale.log",
        "log_interval": 300
    }
}

I've added that configuration to the documentation to make that clear.

IamInstanceProfile should be IAMInstanceProfile

Fixed, thanks. It is unfortunate that the AWS sdk calls it IamInstanceProfile.

Autoscaler bit in docs should mention you need to use an IAMInstanceProfile.

Yes! I've added that to the documentation, thanks.

Ready for another look at f7c83a41f84033e4ea9e570dd85e0152f0d81aab on branch 18772-ebs-autoscale-2

developer-run-tests: #2928

Just a couple of nitpicks:

It looks like we could do this and the "set +e" / "set -e" thing wouldn't be needed:

-        LSBLK=`lsblk -o +SERIAL |grep $valid_volume_id`
-        if [[ $? -eq 0 ]]; then
+        if LSBLK=`lsblk -o NAME,SERIAL |grep $valid_volume_id`; then

xargs seems kind of unnecessary here (although to be fair, polishing bash code doesn't always feel worthwhile):

-            nvme_device=`echo $LSBLK|cut -f1 -d' '|xargs -I {} echo "/dev/{}"`
+            nvme_device=/dev/`echo $LSBLK|cut -f1 -d' '`

LGTM, thanks!

Tom Clegg wrote:

Just a couple of nitpicks:

It looks like we could do this and the "set +e" / "set -e" thing wouldn't be needed:
xargs seems kind of unnecessary here (although to be fair, polishing bash code doesn't always feel worthwhile):

Thanks, those are great improvements, merged with those included.