Skip to content

WIP: SPLAT-2615: Added support for legacy AWS dedicated hosts #2650

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.

Sign up for GitHub

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

Open
vr4manta wants to merge 1 commit into
base: master
Choose a base branch
from
Open

WIP: SPLAT-2615: Added support for legacy AWS dedicated hosts #2650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion machine/v1beta1/types_awsprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,15 +460,56 @@ const (
HostAffinityDedicatedHost HostAffinity = "DedicatedHost"
)

// AllocationStrategy selects how a dedicated host is provided to the system for assigning to the instance.
// +kubebuilder:validation:Enum:=Provided;Dynamic
type AllocationStrategy string

const (
// AllocationStrategyUserProvided specifies system is to assign a provided dedicated host to instances.
Copy link
Member

@saschagrunert saschagrunert Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

Suggested change
// AllocationStrategyUserProvided specifies system is to assign a provided dedicated host to instances.
// AllocationStrategyUserProvided specifies that the system should assign instances to a user-provided dedicated host.

AllocationStrategyUserProvided AllocationStrategy = "Provided"
Comment on lines +468 to +469
Copy link
Member

@saschagrunert saschagrunert Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a mismatch between the variable name and value. I guess we should either use

Suggested change
// AllocationStrategyUserProvided specifies system is to assign a provided dedicated host to instances.
AllocationStrategyUserProvided AllocationStrategy = "Provided"
// AllocationStrategyProvided specifies system is to assign a provided dedicated host to instances.
AllocationStrategyProvided AllocationStrategy = "Provided"

or

Suggested change
// AllocationStrategyUserProvided specifies system is to assign a provided dedicated host to instances.
AllocationStrategyUserProvided AllocationStrategy = "Provided"
// AllocationStrategyUserProvided specifies system is to assign a provided dedicated host to instances.
AllocationStrategyUserProvided AllocationStrategy = "UserProvided"


// AllocationStrategyDynamic specifies system to dynamically allocate a dedicated host to use for assignment to instances.
Copy link
Member

@saschagrunert saschagrunert Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

Suggested change
// AllocationStrategyDynamic specifies system to dynamically allocate a dedicated host to use for assignment to instances.
// AllocationStrategyDynamic specifies that the system should dynamically allocate a dedicated host for instances.

AllocationStrategyDynamic AllocationStrategy = "Dynamic"
)

// DedicatedHost represents the configuration for the usage of dedicated host.
// +kubebuilder:validation:XValidation:rule="self.allocationStrategy == 'Provided' ? has(self.id) : true",message="id is required when allocationStrategy is Provided, and forbidden otherwise"
// +kubebuilder:validation:XValidation:rule="has(self.id) ? self.allocationStrategy == 'Provided' : true",message="id is only allowed when allocationStrategy is Provided"
// +kubebuilder:validation:XValidation:rule="has(self.dynamicHostAllocation) ? self.allocationStrategy == 'Dynamic' : true",message="dynamicHostAllocation is only allowed when allocationStrategy is Dynamic"
type DedicatedHost struct {
// allocationStrategy specifies if the dedicated host will be provided by the admin through the id field or if the host will be dynamically allocated.
// Valid values are Provided and Dynamic.
// The current default value is "Provided".
// When AllocationStrategy is set to Provided, an ID of the dedicated host to assign must be provided.
// When AllocationStrategy is set to Dynamic, a dedicated host will be allocated and used to assign instances.
// When AllocationStrategy is set to Dynamic, and DynamicHostAllocation is provided, a dedicated host will be allocated and the tags in DynamicHostAllocation will be assigned to that host.
// +required
// +unionDiscriminator
// +kubebuilder:default=Provided
Copy link
Member

@saschagrunert saschagrunert Jan 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The +kubebuilder:default=Provided marker does not generate a default value in the OpenAPI schema because the field is marked as +required. The marker only adds documentation text, not an actual OpenAPI default.

So we could remove that line and change

	// The current default value is "Provided".

to

	// This field is required.

AllocationStrategy AllocationStrategy `json:"allocationStrategy,omitempty"`

// id identifies the AWS Dedicated Host on which the instance must run.
// The value must start with "h-" followed by either 8 or 17 lowercase hexadecimal characters (0-9 and a-f).
// The use of 8 lowercase hexadecimal characters is for older legacy hosts that may not have been migrated to newer format.
// Must be either 10 or 19 characters in length.
// +kubebuilder:validation:XValidation:rule="self.matches('^h-([0-9a-f]{8}|[0-9a-f]{17})$')",message="hostID must start with 'h-' followed by either 8 or 17 lowercase hexadecimal characters (0-9 and a-f)"
// +kubebuilder:validation:MinLength=10
// +kubebuilder:validation:MaxLength=19
// +required
// +optional
Comment on lines -472 to +498
Copy link
Member

@saschagrunert saschagrunert Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to document now what happens when ommited:

// This field is required when allocationStrategy is Provided, and forbidden when allocationStrategy is Dynamic.
// When omitted, allocationStrategy must be set to Dynamic to enable automatic host allocation.

// +unionMember
ID string `json:"id,omitempty"`
Copy link
Member

@saschagrunert saschagrunert Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ID is already part of 4.21, right? If so, then it's a breaking change:

4.21 will ship:

type DedicatedHost struct {
    // +required
    ID string `json:"id,omitempty"`
}

4.22 will ship:

type DedicatedHost struct {
    // +required
    AllocationStrategy AllocationStrategy `json:"allocationStrategy,omitempty"`

    // +optional (changed from +required)
    ID string `json:"id,omitempty"`

    // +optional (new field)
    DynamicHostAllocation *DynamicHostAllocationSpec `json:"dynamicHostAllocation,omitempty"`
}

Means that a valid 4.21 manifest will be invalid in 4.22.

dedicatedHost:
  id: "h-1234567890abcdef0"

Do we have a webhook or similar solution in the works for this?


// dynamicHostAllocation specifies tags to apply to a dynamically allocated dedicated host.
// This field is mutually exclusive with id and always allocates exactly one host.
Copy link
Member

@saschagrunert saschagrunert Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, we need to document missing behavior:

Suggested change
// This field is mutually exclusive with id and always allocates exactly one host.
// This field is only allowed when allocationStrategy is Dynamic, and is mutually exclusive with id.
// When specified, a dedicated host will be allocated with the provided tags applied.
// When omitted (and allocationStrategy is Dynamic), a dedicated host will be allocated without any additional tags.

// +optional
// +unionMember
DynamicHostAllocation *DynamicHostAllocationSpec `json:"dynamicHostAllocation,omitempty"`
}

// DynamicHostAllocationSpec defines the configuration for dynamic dedicated host allocation.
// This specification always allocates exactly one dedicated host per machine.
type DynamicHostAllocationSpec struct {
// tags to apply to the allocated dedicated host.
Copy link
Member

@saschagrunert saschagrunert Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And for this one as well, document missing behavior:

Suggested change
// tags to apply to the allocated dedicated host.
// tags specifies a set of key-value pairs to apply to the allocated dedicated host.
// When omitted or empty, no additional user-defined tags will be applied to the allocated host.

// +optional
Tags map[string]string `json:"tags,omitempty"`
}
30 changes: 29 additions & 1 deletion machine/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 13 additions & 2 deletions machine/v1beta1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 46 additions & 1 deletion openapi/generated_openapi/zz_generated.openapi.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading