-
Notifications
You must be signed in to change notification settings - Fork 76
Don't create PV AMIs in regions that don't support them #810
Conversation
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.
Overall looks okay to me; had a couple readability comments.
The multiple disconnected pv-support checks seems a little messy, but I don't have any better suggestion.
platform/api/aws/images.go
Outdated
@@ -439,6 +454,14 @@ func (a *API) CopyImage(sourceImageID string, regions []string) (map[string]stri | |||
return nil, err | |||
} | |||
|
|||
if *image.VirtualizationType == "paravirtual" { | |||
for _, region := range regions { | |||
if _, ok := akis[region]; !ok { |
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.
Might as well reuse the RegionSupportsPV
helper for readability here.
platform/api/aws/images.go
Outdated
@@ -439,6 +454,14 @@ func (a *API) CopyImage(sourceImageID string, regions []string) (map[string]stri | |||
return nil, err | |||
} | |||
|
|||
if *image.VirtualizationType == "paravirtual" { |
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.
ec2.VirtualizationTypeParavirtual
looks like the correct const to use here.
cmd/plume/prerelease.go
Outdated
destRegions = append(destRegions, region) | ||
if !pv || aws.RegionSupportsPV(region) { | ||
destRegions = append(destRegions, region) | ||
} |
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.
Perhaps we should have an "else if pv" that logs to indicate each region pv is being skipped for?
We were creating PV images with no AKI ID.
CreatePVImage would previously create an AMI with no AKI ID, and CopyImage would create an image that never exited pending state.
Assume that the BucketRegion for each AWS partition supports PV.
Neither supports PV.
Updated. |
Add
eu-west-3
andcn-northwest-1
, which don't, to plume. Add PV AKI ID forcn-north-1
, which does.