nanovms / ops

ops - build and run nanos unikernels
https://ops.city
MIT License
1.27k stars 132 forks source link

Set VPC on instance creation using VPC name #1584

Closed hnord-vdx closed 7 months ago

hnord-vdx commented 7 months ago

Adding the VPCUUID field allows you to set a VPC of the droplet in the config.json. This way you can set VPC at instance creation without having to do a snapshot and then adding it to non-default VPC

hnord-vdx commented 7 months ago

@eyberg I see you are importing the google/uuid package perhaps we could add a check (uuid.Parse(vpcName)) if the name is a UUID and then skip the whole GetVPC round trip? It would look something like this:

// if users set VPC uuid instead of name it avoids lookup
vpcUUID := config.CloudConfig.VPC
if _, err := uuid.Parse(vpcUUID); err != nil {
    vpc, err := do.GetVPC(ctx, config.CloudConfig.Zone, vpcUUID)
    if err != nil {
        return err
    }
    if vpc != nil {
        vpcUUID = vpc.ID
    }
}

Also, currently my implementation matches the LAST vpc based on the name. Maybe we want to match first VPC and warn using Debugf if there are other matches?

eyberg commented 7 months ago

i'd assume the vpc name is unique and so you really only need the first match? also i'm not crazy about passing in the id optionally but it appears we already do that in aws https://github.com/nanovms/ops/blob/master/provider/aws/aws_network.go#L163 so i'm ok w/allowing that if that's easier

hnord-vdx commented 7 months ago

Yeah, you are right. I just double checked vpc names have to be unique within an account so the zone check is not needed.

eyberg commented 7 months ago

thanks!