-
Notifications
You must be signed in to change notification settings - Fork 579
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
✨ Add support to specify dedicated host ID and Affinity #5113
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,18 @@ const ( | |
IgnitionStorageTypeOptionUnencryptedUserData = IgnitionStorageTypeOption("UnencryptedUserData") | ||
) | ||
|
||
// Tenancy defines the different tenancy options for EC2 instance. | ||
type Tenancy string | ||
|
||
const ( | ||
// TenancyDefault means that the EC2 instance will be launched into a shared host. | ||
TenancyDefault = Tenancy("default") | ||
// TenancyDedicated means that AWS will create and allocate a physical host for the EC2 instance (1 instance per host). | ||
TenancyDedicated = Tenancy("dedicated") | ||
// TenancyHost means that the EC2 instance will be launched into one of the user's pre-allocated physical hosts (multiple instances per host). | ||
TenancyHost = Tenancy("host") | ||
) | ||
|
||
// AWSMachineSpec defines the desired state of an Amazon EC2 instance. | ||
type AWSMachineSpec struct { | ||
// ProviderID is the unique identifier as specified by the cloud provider. | ||
|
@@ -186,9 +198,18 @@ type AWSMachineSpec struct { | |
PlacementGroupPartition int64 `json:"placementGroupPartition,omitempty"` | ||
|
||
// Tenancy indicates if instance should run on shared or single-tenant hardware. | ||
// If host tenancy is used: | ||
// - if there are no Dedicated Hosts with auto-placement enabled that match the instance type configuration, then it is required to set '.spec.hostPlacment.hostID' or no host would be picked. | ||
// - if '.spec.hostPlacment.hostID' is set, the instance will be launched into the specified Host, disregarding any auto-placement settings. | ||
// see https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/dedicated-hosts-understanding.html#dedicated-hosts-auto-placement | ||
// | ||
// +optional | ||
// +kubebuilder:validation:Enum:=default;dedicated;host | ||
Tenancy string `json:"tenancy,omitempty"` | ||
Tenancy Tenancy `json:"tenancy,omitempty"` | ||
|
||
// HostPlacement denotes the placement settings for the instance when tenancy=host. | ||
// +optional | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be PlacementHost consistently with placementGroup and placementGroupPartition? Can we please open ticket to capture putting all placements choices in a common struct for future api bump? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about just |
||
HostPlacement *HostPlacement `json:"hostPlacment,omitempty"` | ||
|
||
// PrivateDNSName is the options for the instance hostname. | ||
// +optional | ||
|
@@ -199,6 +220,19 @@ type AWSMachineSpec struct { | |
CapacityReservationID *string `json:"capacityReservationId,omitempty"` | ||
} | ||
|
||
// HostPlacement denotes the placement settings for the instance when tenancy=host. | ||
type HostPlacement struct { | ||
// HostID pins the instance to a sepcific Dedicated Host. | ||
// +optional | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't a hostID required if I set HostPlacement? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? you might just want to set the affinity. |
||
HostID *string `json:"hostID,omitempty"` | ||
|
||
// Affinity indicates the affinity setting between the instance and a Dedicated Host. | ||
// When affinity is set to host, an instance launched onto a specific host always restarts on the same host if stopped. This applies to both targeted and untargeted launches. | ||
// +optional | ||
// +kubebuilder:validation:Enum:=default;host | ||
Affinity *string `json:"affinity,omitempty"` | ||
} | ||
|
||
// CloudInit defines options related to the bootstrapping systems where | ||
// CloudInit is used. | ||
type CloudInit struct { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
By reading this API is not clear to me which input should I set for tenancy host with autoplacement, can we document that here?
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.
added more documentation, let me know if it makes sense.
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.
thanks! "If host tenancy is used and auto-placement is disabled for the Dedicated Host, It is required to set '.spec.hostPlacment.hostID'" though the requirement is actually not enforced. Can we document what happens if not set in that case, e.g. it's required or no host would be picked. Can we also document all matrix combinations this API enables: If both hostID and autoplacement are set, the hostID would take precedence? If the hostID doesn't exist would it fallback to autoplacement?
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.
we can't enforce it, because we don't know if auto-placement is enabled or not on the dedicated Host, since its created directly by the user out of band.
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.
updated.