Skip to content

Commit

Permalink
bug: placement group should not be added for reservation
Browse files Browse the repository at this point in the history
Reservations are special cases that often can be created
with placement already in mind, or have instances in different
availability zones (or far enough away) so adding a placement
group automatically will prevent provision of a large set of
resources. Changing the default behavior to always require the
user to specify a placement group for EFA is overkill, but
a good balance is, in the case EFA is enabled and there is no
placement group, when there is a reservation do not add the
group automatically, but issue a warning to the user they can
choose to respond to. TLDR: when a user deploys a cluster via
a reservation they are responsible for adding the placement
group.

Signed-off-by: vsoch <[email protected]>
  • Loading branch information
vsoch committed Feb 14, 2025
1 parent 854da87 commit 947f898
Showing 1 changed file with 14 additions and 5 deletions.
19 changes: 14 additions & 5 deletions pkg/cfn/builder/managed_launch_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/awslabs/goformation/v4/cloudformation/cloudformation"
gfnec2 "github.com/awslabs/goformation/v4/cloudformation/ec2"
gfnt "github.com/awslabs/goformation/v4/cloudformation/types"
"github.com/kris-nova/logger"

api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
)
Expand Down Expand Up @@ -82,11 +83,19 @@ func (m *ManagedNodeGroupResourceSet) makeLaunchTemplateData(ctx context.Context
return nil, fmt.Errorf("couldn't build network interfaces for launch template data: %w", err)
}
if mng.Placement == nil {
groupName := m.newResource("NodeGroupPlacementGroup", &gfnec2.PlacementGroup{
Strategy: gfnt.NewString("cluster"),
})
launchTemplateData.Placement = &gfnec2.LaunchTemplate_Placement{
GroupName: groupName,

// Reservations are often explicitly created with placement in mind, and adding
// a group can lead to an error if it cannot reach all reserved instances.
// For this niche case, we leave it up to the user to create the group.
if mng.CapacityReservation != nil {
logger.Warning("EFA requested without placement group. If your reservation warrants one, please add it")
} else {
groupName := m.newResource("NodeGroupPlacementGroup", &gfnec2.PlacementGroup{
Strategy: gfnt.NewString("cluster"),
})
launchTemplateData.Placement = &gfnec2.LaunchTemplate_Placement{
GroupName: groupName,
}
}
}
} else {
Expand Down

0 comments on commit 947f898

Please sign in to comment.