add license-specifications and placement/host-resource-group-arn #109
Conversation
|
|
KhrisRichardson-BO
force-pushed
the
placement-host-resource-group-arn
branch
2 times, most recently
from
1ebed03
to
283317f
Compare
So the closer we are to the aws sdk schema and naming the better we generally think. In terms of backwards compatibility, a pattern we have often used when justified — and I think it would be justified here — was to have a deprecation period where both fields work in between. Let me know what you prefer 🙂 |
KhrisRichardson-BO
force-pushed
the
placement-host-resource-group-arn
branch
2 times, most recently
from
b0ed976
to
dcf0739
Compare
|
@azr I've updated the PR so that there's more coherence with the AWS SDK, including adding tenancy under placement with some deprecation warnings. I'm open to any suggestions. P.S. there was one small ancillary edit made to make golangci-lint happy. Thanks! |
KhrisRichardson-BO
force-pushed
the
placement-host-resource-group-arn
branch
from
dcf0739
to
dd3c4ea
Compare
| // Each `license_configuration_request` describes a license configuration, | ||
| // the properties of which are: | ||
| // | ||
| // - `license_configuration_arn` (string) - The Amazon Resource Name (ARN) | ||
| // of the license configuration. | ||
| // |
| // of the license configuration. | ||
| // | ||
| LicenseSpecifications []LicenseSpecification `mapstructure:"license_specifications" required:"false"` | ||
| // Describes the placement of an instance. |
There was a problem hiding this comment.
Should we describe the default behavior too here ?
We could say: When no placement is specified, shared tenancy will be selected. or something? Is that even correct ?
There was a problem hiding this comment.
@azr the docs don't really go into much detail about the default placement. My interpretation is that these are scheduling hints that EC2 uses to filter the host onto which the instance is scheduled, otherwise EC2 chooses at random using black box heuristics that are unknown to/unknowable by the end-user.
|
Thanks for addressing the docs changes, they looked great, I just have a tiny question/suggestion, and we should be good ! |
This PR adds support for scheduling instances to host resource groups, which likewise requires the declaration of a license configuration.
While the AWS SDK allows for a collection of license configurations, this PR flattens that to one in accordance with some of the other conventions observed. I'm open to changing that decision, updating the data model to more accurately reflect the collection of license configurations that comprise a license specification.
In all honesty I would prefer to group the
tenancyandhost_resource_group_arnsettings under aplacementgrouping, but I wasn't sure what the policy was around backwards compatibility or the general attitude around adherence to the AWS SDK schema vs. flattening to provoke the end user to use simple variable names that they can override viavarsarguments on the command line.Any input is welcome. Thanks