Skip to content

Commit

Permalink
[AWS] Fix: override disk when mounted on same device name (#97)
Browse files Browse the repository at this point in the history
  • Loading branch information
obierlaire authored Sep 20, 2023
1 parent f313cc2 commit 2621e25
Show file tree
Hide file tree
Showing 8 changed files with 218 additions and 35 deletions.
48 changes: 41 additions & 7 deletions internal/plan/mappings/aws/ec2_asg.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,35 +62,69 @@ compute_resource:
storage:
- type: list
item:
- paths: '${ami}.values.block_device_mappings[].ebs | select(length > 0)'
- paths: '${ami}.values.block_device_mappings[] | select(length > 0)'
properties:
size:
- paths: ".volume_size"
- paths: ".ebs.volume_size"
default: 8
unit: gb
type:
- paths: ".volume_type"
- paths: ".ebs.volume_type"
default: standard
reference:
general: disk_types
key:
- paths: ".device_name | cbf::extract_disk_key"
override_priority:
- default: 2
- paths:
- '${launch_configuration}.values.ebs_block_device[] | select(length > 0)'
- '${launch_configuration}.values.block_device_mappings[] | select(length > 0) | select(.virtual_name == null or (.virtual_name | startswith("ephemeral") | not)) | .ebs'
properties:
size:
- paths: ".volume_size"
- paths:
- ".volume_size"
unit: gb
- paths: ".snapshot_id"
- paths:
- ".snapshot_id"
reference:
paths: .prior_state.values.root_module.resources[] | select(.values.id == "${key}") | .values
property: ".volume_size"
- default: 8
unit: gb
type:
- paths: ".volume_type"
- paths:
- ".volume_type"
default: standard
reference:
general: disk_types
key:
- paths: ".device_name | cbf::extract_disk_key"
override_priority:
- default: 0
- paths:
- '${launch_configuration}.values.block_device_mappings[] | select(length > 0) | select(.virtual_name == null or (.virtual_name | startswith("ephemeral") | not)) | select(.ebs != null)'
properties:
size:
- paths:
- ".ebs[0].volume_size"
unit: gb
- paths:
- ".ebs[0].snapshot_id"
reference:
paths: .prior_state.values.root_module.resources[] | select(.values.id == "${key}") | .values
property: ".volume_size"
- default: 8
unit: gb
type:
- paths:
- ".ebs[0].volume_type"
default: standard
reference:
general: disk_types
key:
- paths: ".device_name | cbf::extract_disk_key"
override_priority:
- default: 1
- paths:
- '${launch_configuration}.values.ephemeral_block_device[] | select(length > 0)'
- '${launch_configuration}.values.block_device_mappings[] | select(length > 0) | select(.virtual_name != null and (.virtual_name | startswith("ephemeral")))'
Expand Down
50 changes: 42 additions & 8 deletions internal/plan/mappings/aws/ec2_instance.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,35 +65,69 @@ compute_resource:
- type: list
item:
- paths:
- '${ami}.values.block_device_mappings[] | select(length > 0) | .ebs'
- '${ami}.values.block_device_mappings[] | select(length > 0)'
properties:
size:
- paths: ".volume_size"
- paths: ".ebs.volume_size"
default: 8
unit: gb
type:
- paths: ".volume_type"
- paths: ".ebs.volume_type"
default: standard
reference:
general: disk_types
general: disk_types
key:
- paths: ".device_name | cbf::extract_disk_key"
override_priority:
- default: 2
- paths:
- '.values.ebs_block_device[] | select(length > 0)'
- '${launch_template}.values.block_device_mappings[] | select(length > 0) | select(.virtual_name == null or .virtual_name == "" or (.virtual_name | startswith("ephemeral") | not)) | .ebs'
properties:
size:
- paths: ".volume_size"
- paths:
- ".volume_size"
unit: gb
- paths: ".snapshot_id"
- paths:
- ".snapshot_id"
reference:
paths: .prior_state.values.root_module.resources[] | select(.values.id == "${key}") | .values
property: ".volume_size"
- default: 8
unit: gb
type:
- paths: ".volume_type"
- paths:
- ".volume_type"
default: standard
reference:
general: disk_types
key:
- paths: ".device_name | cbf::extract_disk_key"
override_priority:
- default : 0
- paths:
- '${launch_template}.values.block_device_mappings[] | select(length > 0) | select(.virtual_name == null or .virtual_name == "" or (.virtual_name | startswith("ephemeral") | not)) | select(.ebs != null)'
properties:
size:
- paths:
- ".ebs[0].volume_size"
unit: gb
- paths:
- ".ebs[0].snapshot_id"
reference:
paths: .prior_state.values.root_module.resources[] | select(.values.id == "${key}") | .values
property: ".volume_size"
- default: 8
unit: gb
type:
- paths:
- ".ebs[0].volume_type"
default: standard
reference:
general: disk_types
key:
- paths: ".device_name | cbf::extract_disk_key"
override_priority:
- default : 1
- paths:
- '.values.ephemeral_block_device[] | select(length > 0)'
- '${launch_template}.values.block_device_mappings[] | select(length > 0) | select(.virtual_name != null and (.virtual_name | startswith("ephemeral")))'
Expand Down
6 changes: 4 additions & 2 deletions internal/plan/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ import (
)

type storage struct {
SizeGb decimal.Decimal
IsSSD bool
SizeGb decimal.Decimal
IsSSD bool
OverridePriority int
Key string
}

func applyReference(valueFound string, propertyMapping *PropertyDefinition, context *tfContext) (interface{}, error) {
Expand Down
99 changes: 83 additions & 16 deletions internal/plan/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package plan
import (
"fmt"
"regexp"
"sort"
"strings"

"github.com/carboniferio/carbonifer/internal/providers"
Expand Down Expand Up @@ -305,20 +306,9 @@ func GetComputeResource(resourceI interface{}, resourceMapping *ResourceMapping,
return nil, errors.Wrapf(err, "Cannot get storages for %v", resourceAddress)
}

for i, storageI := range storages {
if storageI == nil {
continue
}
storage, err := getStorage(storageI.(map[string]interface{}))
if err != nil {
return nil, errors.Wrapf(err, "Cannot get storage[%v] for %v", i, resourceAddress)
}
size := storage.SizeGb
if storage.IsSSD {
computeResource.Specs.SsdStorage = computeResource.Specs.SsdStorage.Add(size)
} else {
computeResource.Specs.HddStorage = computeResource.Specs.HddStorage.Add(size)
}
err = processStorages(storages, &computeResource, context)
if err != nil {
return nil, errors.Wrapf(err, "Cannot process storages for %v", resourceAddress)
}

resourcesResult = append(resourcesResult, computeResource)
Expand Down Expand Up @@ -346,6 +336,68 @@ func getGPU(gpu map[string]interface{}) ([]string, error) {
return gpuTypes, nil
}

func processStorages(storagesI []interface{}, computeResource *resources.ComputeResource, context *tfContext) error {
storagesByKey := map[string][]*storage{}
for i, storageI := range storagesI {
if storageI == nil {
continue
}
storageItem, err := getStorage(storageI.(map[string]interface{}))
if err != nil {
return errors.Wrapf(err, "Cannot get storage[%v] for %v", i, context.ResourceAddress)
}
if storagesByKey[storageItem.Key] == nil {
storagesByKey[storageItem.Key] = []*storage{storageItem}
} else {
storagesByKey[storageItem.Key] = append(storagesByKey[storageItem.Key], storageItem)
}
}

// Print the map nicely
for key, storages := range storagesByKey {
log.Debugf("Storage Key: %v", key)
for _, storage := range storages {
log.Debugf(" %v\n", storage)
}
}
log.Debug("------")

storagesOverriden := []*storage{}

for key, storages := range storagesByKey {
// Take the storage with the lower priority in the list
if key != "" {
storages = sortStorages(storages)
storageItem := storages[0]
storagesOverriden = append(storagesOverriden, storageItem)
} else {
storagesOverriden = append(storagesOverriden, storages...)
}
}

for _, storageItem := range storagesOverriden {
size := storageItem.SizeGb
if storageItem.IsSSD {
computeResource.Specs.SsdStorage = computeResource.Specs.SsdStorage.Add(size)
} else {
computeResource.Specs.HddStorage = computeResource.Specs.HddStorage.Add(size)
}
}

return nil
}

func sortStorages(storages []*storage) []*storage {
if len(storages) == 0 {
return storages
}
// sort by override_priority, lowest priority first
sort.Slice(storages, func(i, j int) bool {
return storages[i].OverridePriority < storages[j].OverridePriority
})
return storages
}

func getStorage(storageMap map[string]interface{}) (*storage, error) {
storageSize, ok := storageMap["size"].(*valueWithUnit)
if !ok {
Expand Down Expand Up @@ -400,9 +452,24 @@ func getStorage(storageMap map[string]interface{}) (*storage, error) {
isSSD = true
}
}
overridePriority := 0
overridePriorityI := storageMap["override_priority"]
if overridePriorityI != nil {
overridePriority = overridePriorityI.(*valueWithUnit).Value.(int)
}

key := ""
keyI := storageMap["key"]
if keyI != nil {
keyString := keyI.(*valueWithUnit).Value.(string)
key = keyString
}

storage := storage{
SizeGb: storageSizeGb,
IsSSD: isSSD,
SizeGb: storageSizeGb,
IsSSD: isSSD,
OverridePriority: overridePriority,
Key: key,
}
return &storage, nil
}
Expand Down
2 changes: 1 addition & 1 deletion internal/plan/test/resources_aws_asg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestGetResource_AWSASG(t *testing.T) {
MemoryMb: int32(16384),

HddStorage: decimal.NewFromInt(300),
SsdStorage: decimal.NewFromInt(30 + 150),
SsdStorage: decimal.NewFromInt(150),
},
},
}
Expand Down
20 changes: 19 additions & 1 deletion internal/plan/test/resources_aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,26 @@ func TestGetResource_EC2(t *testing.T) {
VCPUs: int32(4),
MemoryMb: int32(16384),

HddStorage: decimal.Zero,
SsdStorage: decimal.NewFromInt(180),
},
},
"aws_instance.ec2_with_lt_disk_override": resources.ComputeResource{
Identification: &resources.ResourceIdentification{
Address: "aws_instance.ec2_with_lt_disk_override",
Name: "ec2_with_lt_disk_override",
ResourceType: "aws_instance",
Provider: providers.AWS,
Region: "eu-west-3",
Count: 1,
ReplicationFactor: 1,
},
Specs: &resources.ComputeResourceSpecs{
VCPUs: int32(4),
MemoryMb: int32(16384),

HddStorage: decimal.NewFromInt(300),
SsdStorage: decimal.NewFromInt(30 + 150),
SsdStorage: decimal.NewFromInt(150),
},
},
}
Expand Down
9 changes: 9 additions & 0 deletions internal/utils/jsonQuery.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ func (*moduleLoader) LoadModule(name string) (*gojq.Query, error) {
def all_select(a; b):
.planned_values | .. | objects | select(has("resources")) | .resources[] | select(.[a] == b);
def extract_disk_key:
if test("^/dev/sd[a-z]+") or test("^/dev/xvd[a-z]+") then
capture("^/dev/(?:sd|xvd)(?<letter>[a-z]+)").letter
elif test("^/dev/nvme[0-2]?[0-9]n") then
capture("^/dev/nvme(?<number>[0-2]?[0-9])n").number | tonumber | (96 + .) | [.] | implode
else
"Unknown format"
end;
`)
}
return nil, fmt.Errorf("module not found: %q", name)
Expand Down
19 changes: 19 additions & 0 deletions test/terraform/aws_ec2/ec2_launch_template.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,18 @@ resource "aws_launch_template" "my_launch_template" {
image_id = "${data.aws_ami.ubuntu.id}"
instance_type = "m5d.xlarge"

block_device_mappings {
device_name = "/dev/sdb"
virtual_name = "ephemeral0"
}

}

resource "aws_launch_template" "my_launch_template_disk_override" {
name_prefix = "my_launch_template"
image_id = "${data.aws_ami.ubuntu.id}"
instance_type = "m5d.xlarge"

block_device_mappings {
device_name = "/dev/sda1"

Expand All @@ -25,4 +37,11 @@ resource "aws_instance" "ec2_with_lt" {
id = "${aws_launch_template.my_launch_template.id}"
version = "$$Latest"
}
}

resource "aws_instance" "ec2_with_lt_disk_override" {
launch_template {
id = "${aws_launch_template.my_launch_template_disk_override.id}"
version = "$$Latest"
}
}

0 comments on commit 2621e25

Please sign in to comment.