Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: dedicated hosts selectors #4553
feat: dedicated hosts selectors #4553
Changes from 36 commits
7f08088
4bf9719
d91d401
57a7a27
0bda01c
77bb9d3
bf0ac90
c050b87
1e6f05d
1410dce
cd8035b
fbcbab9
a66c663
f7fff5a
44fdeef
4381e8d
7246d16
d514af5
08f41a1
cbf61a7
c5bc5ee
b2e8fa8
a3a7f5d
5b31e38
04b081a
7416e23
1e4ed56
16aa9c6
16ff1cf
42cbd6b
2069783
914e187
284b157
5975167
c70d149
b0ebfef
78efb9a
25d1e91
48a0262
03ba9be
57c79ee
f1e23d5
9e2e1bc
85f0860
9a23247
5e0c322
9fe0f2f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Digging a little deeper into the docs, trying to understand why the license configuration piece is needed. Can you achieve dedicated hosts by associating license configurations with the AMIs that you are planning to launch? From what I can tell, if the AMI has an associated license configuration, you may not have a need to specify the configuration directly.
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.
there's two options:
In some cases it makes sense to associate with AMIs, in others it does not, such as when the an AMI is used both with & without license rules or where doing so just adds unnecessary workflow steps.
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.
What's the use-case for an AMI being used without license rules? When you are bringing in software at runtime that will have a license attached to it? Why not bake it into the AMI in that case?
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.
so dedicated host resource groups are a peculiar thing. They are implemented within license manager, but they don't have to have anything to do with software licenses https://docs.aws.amazon.com/license-manager/latest/userguide/host-resource-groups.html
We use them to get a dynamically allocated pool of dedicated hosts that can be shared across accounts.
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.
Quick high-level question: Which of these things actually need to be terms? Is there a need for someone to specify multiple of any of these things and have all of the options be passed through or us to somehow order and pick one based on some heuristic?
If there's not a reason that we should support more than one resolution of these things per-EC2NodeClass, it may be worth having this be a pure selector, no sets of terms that are ORed
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'm curious what your thoughts are on this: If we're just using the name selector across all of these new entities in the EC2NodeClass, what's the value in resolving the ARN back into the status? For the entities that already use the name as an "id"-like thing, this is just the name with some additional "syntactic sugar" to add the account and region information