This message was deleted.

Nice! You reminded me I have to add the UI extension to our plugin. You should add a LICENSE file by the way!

fair point. thanks @rapid-eye-61308!

Yessir!

First off, thanks for the contribution! We appreciate you taking the initiative and enabling provisioning for a wider audience. I have a couple of suggestions that might make maintenance of this driver easier: •

clusterIDFromMachineName

, There are a few edge cases with this function, while you're correct that the name will be of the format

<cluster>-<pool>-<hash>-<hash>

, it is possible to have a cluster with

-

in the name, or in the machine pool name, and if the cluster name is > 64 characters this name will be truncated with a hash appended at exactly 63 characters, which may make the original cluster name unrecoverable. It might be worth exiting early if the name doesn't have exactly 3 dashes or if it is 63 characters long (max length for a name). for example, if I name my clusters like

rke2-test

, we would extract the

--cluster-id

as

rke2

, which may lead to collisions when provisioning an

rke2-test2

. So in summary, deriving the cluster name is fine, but it might be a good idea to be a bit more restrictive with doing so. • While not strictly needed, I'd recommend cleaning up everything in the event of an error during creation. The

amazonec2

driver within rancher/machine is a good example: https://github.com/rancher/machine/blob/master/drivers/amazonec2/amazonec2.go#L779-L792, if any error is encountered during the creation, just cleanup the vm. This has the benefit of ensuring no resources are ever leaked, and should also speed things along by having the cleanup performed during creation (a delete job may still be created, but should just exit immediately as there is nothing to clean).

thanks for the feedback, @modern-barista-59239!