-
Notifications
You must be signed in to change notification settings - Fork 9.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
d/ec2_instance_type #13124
d/ec2_instance_type #13124
Conversation
ba8bb11
to
776c16b
Compare
Closes #10989. |
@lde Thank you for your effort on this! At first glance, this looks great and I'd like to help get this moving. Would you be willing to rebase? |
8fe7d71
to
b126f97
Compare
Hi @YakDriver I've rebased my pr, it should be ready to merge. |
Hi @YakDriver , we need this feature in one of our terraform modules, when can we expect this to be merged in? |
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.
This is looking great! Thank you for your work. It needs a few fixes. Let me know if you're willing to make the changes.
@lde I just wanted to check in with you and see if you are willing to make the changes. If not, we can take it over or someone from the community can. We want to give you the first chance to finish if you're able. 👍 |
0364090
to
d3016aa
Compare
…tance type Co-authored-by: Dirk Avery <31492422+YakDriver@users.noreply.github.com>
d3016aa
to
f821eda
Compare
Hi, @YakDriver But i've an issue on terrafmt check. terrafmt diff --check --verbose --fmtcompat aws/data_source_aws_ec2_instance_type_test.go |
Hi @YakDriver, I've splited tests and added somes missing attributes. |
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.
This is very close. Just a couple of small items and we should be ready to merge!
|
||
~> **NOTE:** Not all attributes are set for every instance type. | ||
|
||
* `accelerators` Describes the Inference accelerators for the instance type. |
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.
I appreciate these being in alphabetical order! It makes the docs more readable!
` | ||
|
||
const testAccDataSourceEc2InstanceTypeAccelerator = ` | ||
|
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.
The blank line is probably triggering the linter.
d.Set("instance_type", v.InstanceType) | ||
d.Set("current_generation", v.CurrentGeneration) | ||
d.Set("free_tier_eligible", v.FreeTierEligible) | ||
d.Set("supported_usages_classes", v.SupportedUsageClasses) | ||
d.Set("supported_root_device_types", v.SupportedRootDeviceTypes) | ||
d.Set("supported_virtualization_types", v.SupportedVirtualizationTypes) | ||
d.Set("bare_metal", v.BareMetal) | ||
d.Set("hypervisor", v.Hypervisor) | ||
d.Set("supported_architectures", v.ProcessorInfo.SupportedArchitectures) | ||
d.Set("sustained_clock_speed", v.ProcessorInfo.SustainedClockSpeedInGhz) | ||
d.Set("default_vcpus", v.VCpuInfo.DefaultVCpus) | ||
d.Set("default_cores", v.VCpuInfo.DefaultCores) | ||
d.Set("default_threads_per_core", v.VCpuInfo.DefaultThreadsPerCore) | ||
d.Set("valid_threads_per_core", v.VCpuInfo.ValidThreadsPerCore) | ||
d.Set("valid_cores", v.VCpuInfo.ValidCores) | ||
d.Set("memory_size", v.MemoryInfo.SizeInMiB) | ||
d.Set("instance_storage_supported", v.InstanceStorageSupported) | ||
if v.InstanceStorageInfo != nil { | ||
d.Set("total_instance_storage", v.InstanceStorageInfo.TotalSizeInGB) | ||
if v.InstanceStorageInfo.Disks != nil { | ||
diskList := make([]interface{}, len(v.InstanceStorageInfo.Disks)) | ||
for i, dk := range v.InstanceStorageInfo.Disks { | ||
disk := map[string]interface{}{ | ||
"size": aws.Int64Value(dk.SizeInGB), | ||
"count": aws.Int64Value(dk.Count), | ||
"type": aws.StringValue(dk.Type), | ||
} | ||
diskList[i] = disk | ||
} | ||
d.Set("instance_disks", diskList) | ||
} | ||
} | ||
d.Set("ebs_optimized_support", v.EbsInfo.EbsOptimizedSupport) | ||
if v.EbsInfo.EbsOptimizedInfo != nil { | ||
d.Set("ebs_performance_baseline_bandwidth", v.EbsInfo.EbsOptimizedInfo.BaselineBandwidthInMbps) | ||
d.Set("ebs_performance_baseline_throughput", v.EbsInfo.EbsOptimizedInfo.BaselineThroughputInMBps) | ||
d.Set("ebs_performance_baseline_iops", v.EbsInfo.EbsOptimizedInfo.BaselineIops) | ||
d.Set("ebs_performance_maximum_bandwidth", v.EbsInfo.EbsOptimizedInfo.MaximumBandwidthInMbps) | ||
d.Set("ebs_performance_maximum_throughput", v.EbsInfo.EbsOptimizedInfo.MaximumThroughputInMBps) | ||
d.Set("ebs_performance_maximum_iops", v.EbsInfo.EbsOptimizedInfo.MaximumIops) | ||
} | ||
d.Set("ebs_encryption_support", v.EbsInfo.EncryptionSupport) | ||
d.Set("ebs_nvme_support", v.EbsInfo.NvmeSupport) | ||
d.Set("network_performance", v.NetworkInfo.NetworkPerformance) | ||
d.Set("maximum_network_interfaces", v.NetworkInfo.MaximumNetworkInterfaces) | ||
d.Set("maximum_ipv4_addresses_per_interface", v.NetworkInfo.Ipv4AddressesPerInterface) | ||
d.Set("maximum_ipv6_addresses_per_interface", v.NetworkInfo.Ipv6AddressesPerInterface) | ||
d.Set("ipv6_supported", v.NetworkInfo.Ipv6Supported) | ||
d.Set("ena_support", v.NetworkInfo.EnaSupport) | ||
d.Set("efa_supported", v.NetworkInfo.EfaSupported) | ||
if v.GpuInfo != nil { | ||
gpuList := make([]interface{}, len(v.GpuInfo.Gpus)) | ||
for i, gp := range v.GpuInfo.Gpus { | ||
gpu := map[string]interface{}{ | ||
"manufacturer": aws.StringValue(gp.Manufacturer), | ||
"name": aws.StringValue(gp.Name), | ||
"count": aws.Int64Value(gp.Count), | ||
"memory_size": aws.Int64Value(gp.MemoryInfo.SizeInMiB), | ||
} | ||
gpuList[i] = gpu | ||
} | ||
d.Set("gpus", gpuList) | ||
d.Set("total_gpu_memory", v.GpuInfo.TotalGpuMemoryInMiB) | ||
} | ||
if v.FpgaInfo != nil { | ||
fpgaList := make([]interface{}, len(v.FpgaInfo.Fpgas)) | ||
for i, fpg := range v.FpgaInfo.Fpgas { | ||
fpga := map[string]interface{}{ | ||
"manufacturer": aws.StringValue(fpg.Manufacturer), | ||
"name": aws.StringValue(fpg.Name), | ||
"count": aws.Int64Value(fpg.Count), | ||
"memory_size": aws.Int64Value(fpg.MemoryInfo.SizeInMiB), | ||
} | ||
fpgaList[i] = fpga | ||
} | ||
d.Set("fpgas", fpgaList) | ||
d.Set("total_fpga_memory", v.FpgaInfo.TotalFpgaMemoryInMiB) | ||
} | ||
d.Set("supported_placement_strategies", v.PlacementGroupInfo.SupportedStrategies) | ||
if v.InferenceAcceleratorInfo != nil { | ||
acceleratorList := make([]interface{}, len(v.InferenceAcceleratorInfo.Accelerators)) | ||
for i, accl := range v.InferenceAcceleratorInfo.Accelerators { | ||
accelerator := map[string]interface{}{ | ||
"manufacturer": aws.StringValue(accl.Manufacturer), | ||
"name": aws.StringValue(accl.Name), | ||
"count": aws.Int64Value(accl.Count), | ||
} | ||
acceleratorList[i] = accelerator | ||
} | ||
d.Set("accelerators", acceleratorList) | ||
} | ||
d.Set("hibernation_supported", v.HibernationSupported) | ||
d.Set("burstable_performance_supported", v.BurstablePerformanceSupported) | ||
d.Set("dedicated_hosts_supported", v.DedicatedHostsSupported) | ||
d.Set("auto_recovery_supported", v.AutoRecoverySupported) |
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.
Nit: Please put all the attributes in alphabetical order. It makes maintenance easier later!
Hi, |
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.
Once we make the linter happy, we are ready to go.
Co-authored-by: Dirk Avery <31492422+YakDriver@users.noreply.github.com>
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.
I apologize. I found some additional problems running the tests across partitions.
Hooray, linter seems to be happy ! |
@lde If you have a chance to fix these last few items in the next hour or so, we can get this in in time for 3.10.0. |
Co-authored-by: Dirk Avery <31492422+YakDriver@users.noreply.github.com>
@YakDriver I've applied your tests updates. |
This looks great! Thank you for all your work on this. I think a lot of people will find this useful. Tests on GovCloud:
Tests on Commercial:
|
This has been released in version 3.10.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Community Note
Closes #10989
Release note for CHANGELOG:
Output from acceptance testing: