Skip to content

Commit 8e3a4e7

Browse files
sapphirewsapphirew
authored
fix: use default security group for eks 1.33+ when EFA is enabled (#8509)
1 parent cff1216 commit 8e3a4e7

File tree

16 files changed

+1060
-31
lines changed

16 files changed

+1060
-31
lines changed

‎pkg/apis/eksctl.io/v1alpha5/types.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ const (
5050
Version1_33 = "1.33"
5151
DockershimDeprecationVersion = Version1_24
5252
AmazonLinux2EOLVersion = Version1_33
53+
// EFABuiltInSupportVersion defines the minimum Kubernetes version that supports built-in EFA
54+
EFABuiltInSupportVersion = Version1_33
5355
//TODO: Remove this and replace with output from DescribeClusterVersions endpoint
5456
// DefaultVersion (default)
5557
DefaultVersion = Version1_32

‎pkg/cfn/builder/managed_launch_template.go‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
gfnt "github.com/weaveworks/eksctl/pkg/goformation/cloudformation/types"
1010

1111
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
12+
"github.com/weaveworks/eksctl/pkg/utils/efa"
1213
)
1314

1415
func (m *ManagedNodeGroupResourceSet) makeLaunchTemplateData(ctx context.Context) (*gfnec2.LaunchTemplate_LaunchTemplateData, error) {
@@ -75,9 +76,21 @@ func (m *ManagedNodeGroupResourceSet) makeLaunchTemplateData(ctx context.Context
7576
if api.IsEnabled(mng.EFAEnabled) {
7677
// we don't want to touch the network interfaces at all if we have a
7778
// managed nodegroup, unless EFA is enabled
78-
desc := "worker nodes in group " + m.nodeGroup.Name
79-
efaSG := m.addEFASecurityGroup(m.vpcImporter.VPC(), m.clusterConfig.Metadata.Name, desc)
80-
securityGroupIDs = append(securityGroupIDs, efaSG)
79+
config := efa.SecurityGroupConfig{
80+
ClusterVersion: m.clusterConfig.Metadata.Version,
81+
ClusterName: m.clusterConfig.Metadata.Name,
82+
NodeGroupName: m.nodeGroup.Name,
83+
VPCID: m.vpcImporter.VPC(),
84+
Description: "worker nodes in group " + m.nodeGroup.Name,
85+
}
86+
87+
efaSG, err := efa.ProcessSecurityGroup(config, m.addEFASecurityGroup)
88+
if err != nil {
89+
return nil, err
90+
} else if efaSG != nil {
91+
securityGroupIDs = append(securityGroupIDs, efaSG)
92+
}
93+
8194
if err := buildNetworkInterfaces(ctx, launchTemplateData, mng.InstanceTypeList(), true, securityGroupIDs, m.ec2API); err != nil {
8295
return nil, fmt.Errorf("couldn't build network interfaces for launch template data: %w", err)
8396
}

‎pkg/cfn/builder/managed_launch_template_test.go‎

Lines changed: 249 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,255 @@ API_SERVER_URL=https://test.com
457457
resourcesFilename: "launch_template_with_capacity_reservation_preference.json",
458458
}),
459459
)
460+
461+
Context("version-aware EFA security group creation", func() {
462+
var (
463+
clusterConfig *api.ClusterConfig
464+
provider *mockprovider.MockProvider
465+
vpcImporter *vpcfakes.FakeImporter
466+
bootstrapper *fakes.FakeBootstrapper
467+
)
468+
469+
BeforeEach(func() {
470+
clusterConfig = api.NewClusterConfig()
471+
clusterConfig.Metadata.Name = "efa-test"
472+
clusterConfig.Metadata.Region = "us-west-2"
473+
provider = mockprovider.NewMockProvider()
474+
vpcImporter = new(vpcfakes.FakeImporter)
475+
bootstrapper = new(fakes.FakeBootstrapper)
476+
477+
vpcImporter.VPCReturns(gfnt.NewString("vpc-12345"))
478+
vpcImporter.SecurityGroupsReturns(gfnt.Slice{gfnt.NewString("sg-cluster")})
479+
bootstrapper.UserDataReturns("", nil)
480+
481+
mockSubnetsAndAZInstanceSupport(clusterConfig, provider,
482+
[]string{"us-west-2a"},
483+
[]string{}, // local zones
484+
[]ec2types.InstanceType{
485+
ec2types.InstanceTypeC5n18xlarge,
486+
})
487+
488+
provider.MockEC2().On("DescribeInstanceTypes",
489+
mock.Anything,
490+
&ec2.DescribeInstanceTypesInput{
491+
InstanceTypes: []ec2types.InstanceType{ec2types.InstanceTypeC5n18xlarge},
492+
},
493+
).Return(
494+
&ec2.DescribeInstanceTypesOutput{
495+
InstanceTypes: []ec2types.InstanceTypeInfo{
496+
{
497+
InstanceType: ec2types.InstanceTypeC5n18xlarge,
498+
NetworkInfo: &ec2types.NetworkInfo{
499+
EfaSupported: aws.Bool(true),
500+
MaximumNetworkCards: aws.Int32(4),
501+
},
502+
},
503+
},
504+
}, nil,
505+
)
506+
})
507+
508+
Context("Kubernetes 1.33+ with built-in EFA support", func() {
509+
BeforeEach(func() {
510+
clusterConfig.Metadata.Version = "1.33"
511+
})
512+
513+
It("does not create custom EFA security groups", func() {
514+
ng := &api.ManagedNodeGroup{
515+
NodeGroupBase: &api.NodeGroupBase{
516+
Name: "efa-ng-1.33",
517+
InstanceType: "c5n.18xlarge",
518+
EFAEnabled: aws.Bool(true),
519+
},
520+
}
521+
522+
err := api.SetManagedNodeGroupDefaults(ng, clusterConfig.Metadata, false)
523+
Expect(err).NotTo(HaveOccurred())
524+
525+
stack := builder.NewManagedNodeGroup(provider.MockEC2(), clusterConfig, ng, builder.NewLaunchTemplateFetcher(provider.MockEC2()), bootstrapper, false, vpcImporter)
526+
err = stack.AddAllResources(context.Background())
527+
Expect(err).NotTo(HaveOccurred())
528+
529+
bytes, err := stack.RenderJSON()
530+
Expect(err).NotTo(HaveOccurred())
531+
532+
template, err := goformation.ParseJSON(bytes)
533+
Expect(err).NotTo(HaveOccurred())
534+
535+
// Should not have custom EFA security group resources
536+
Expect(template.Resources).NotTo(HaveKey("EFASG"))
537+
Expect(template.Resources).NotTo(HaveKey("EFAEgressSelf"))
538+
Expect(template.Resources).NotTo(HaveKey("EFAIngressSelf"))
539+
})
540+
})
541+
542+
Context("Kubernetes 1.34 with built-in EFA support", func() {
543+
BeforeEach(func() {
544+
clusterConfig.Metadata.Version = "1.34"
545+
})
546+
547+
It("does not create custom EFA security groups", func() {
548+
ng := &api.ManagedNodeGroup{
549+
NodeGroupBase: &api.NodeGroupBase{
550+
Name: "efa-ng-1.34",
551+
InstanceType: "c5n.18xlarge",
552+
EFAEnabled: aws.Bool(true),
553+
},
554+
}
555+
556+
err := api.SetManagedNodeGroupDefaults(ng, clusterConfig.Metadata, false)
557+
Expect(err).NotTo(HaveOccurred())
558+
559+
stack := builder.NewManagedNodeGroup(provider.MockEC2(), clusterConfig, ng, builder.NewLaunchTemplateFetcher(provider.MockEC2()), bootstrapper, false, vpcImporter)
560+
err = stack.AddAllResources(context.Background())
561+
Expect(err).NotTo(HaveOccurred())
562+
563+
bytes, err := stack.RenderJSON()
564+
Expect(err).NotTo(HaveOccurred())
565+
566+
template, err := goformation.ParseJSON(bytes)
567+
Expect(err).NotTo(HaveOccurred())
568+
569+
// Should not have custom EFA security group resources
570+
Expect(template.Resources).NotTo(HaveKey("EFASG"))
571+
Expect(template.Resources).NotTo(HaveKey("EFAEgressSelf"))
572+
Expect(template.Resources).NotTo(HaveKey("EFAIngressSelf"))
573+
})
574+
})
575+
576+
Context("Kubernetes 1.32 without built-in EFA support", func() {
577+
BeforeEach(func() {
578+
clusterConfig.Metadata.Version = "1.32"
579+
})
580+
581+
It("creates custom EFA security groups", func() {
582+
ng := &api.ManagedNodeGroup{
583+
NodeGroupBase: &api.NodeGroupBase{
584+
Name: "efa-ng-1.32",
585+
InstanceType: "c5n.18xlarge",
586+
EFAEnabled: aws.Bool(true),
587+
},
588+
}
589+
590+
err := api.SetManagedNodeGroupDefaults(ng, clusterConfig.Metadata, false)
591+
Expect(err).NotTo(HaveOccurred())
592+
593+
stack := builder.NewManagedNodeGroup(provider.MockEC2(), clusterConfig, ng, builder.NewLaunchTemplateFetcher(provider.MockEC2()), bootstrapper, false, vpcImporter)
594+
err = stack.AddAllResources(context.Background())
595+
Expect(err).NotTo(HaveOccurred())
596+
597+
bytes, err := stack.RenderJSON()
598+
Expect(err).NotTo(HaveOccurred())
599+
600+
template, err := goformation.ParseJSON(bytes)
601+
Expect(err).NotTo(HaveOccurred())
602+
603+
// Should have custom EFA security group resources
604+
Expect(template.Resources).To(HaveKey("EFASG"))
605+
Expect(template.Resources).To(HaveKey("EFAEgressSelf"))
606+
Expect(template.Resources).To(HaveKey("EFAIngressSelf"))
607+
})
608+
})
609+
610+
Context("Kubernetes 1.31 without built-in EFA support", func() {
611+
BeforeEach(func() {
612+
clusterConfig.Metadata.Version = "1.31"
613+
})
614+
615+
It("creates custom EFA security groups", func() {
616+
ng := &api.ManagedNodeGroup{
617+
NodeGroupBase: &api.NodeGroupBase{
618+
Name: "efa-ng-1.31",
619+
InstanceType: "c5n.18xlarge",
620+
EFAEnabled: aws.Bool(true),
621+
},
622+
}
623+
624+
err := api.SetManagedNodeGroupDefaults(ng, clusterConfig.Metadata, false)
625+
Expect(err).NotTo(HaveOccurred())
626+
627+
stack := builder.NewManagedNodeGroup(provider.MockEC2(), clusterConfig, ng, builder.NewLaunchTemplateFetcher(provider.MockEC2()), bootstrapper, false, vpcImporter)
628+
err = stack.AddAllResources(context.Background())
629+
Expect(err).NotTo(HaveOccurred())
630+
631+
bytes, err := stack.RenderJSON()
632+
Expect(err).NotTo(HaveOccurred())
633+
634+
template, err := goformation.ParseJSON(bytes)
635+
Expect(err).NotTo(HaveOccurred())
636+
637+
// Should have custom EFA security group resources
638+
Expect(template.Resources).To(HaveKey("EFASG"))
639+
Expect(template.Resources).To(HaveKey("EFAEgressSelf"))
640+
Expect(template.Resources).To(HaveKey("EFAIngressSelf"))
641+
})
642+
})
643+
644+
Context("invalid Kubernetes version", func() {
645+
BeforeEach(func() {
646+
clusterConfig.Metadata.Version = "invalid.version"
647+
})
648+
649+
It("falls back to creating custom EFA security groups", func() {
650+
ng := &api.ManagedNodeGroup{
651+
NodeGroupBase: &api.NodeGroupBase{
652+
Name: "efa-ng-invalid",
653+
InstanceType: "c5n.18xlarge",
654+
EFAEnabled: aws.Bool(true),
655+
},
656+
}
657+
658+
err := api.SetManagedNodeGroupDefaults(ng, clusterConfig.Metadata, false)
659+
Expect(err).NotTo(HaveOccurred())
660+
661+
stack := builder.NewManagedNodeGroup(provider.MockEC2(), clusterConfig, ng, builder.NewLaunchTemplateFetcher(provider.MockEC2()), bootstrapper, false, vpcImporter)
662+
err = stack.AddAllResources(context.Background())
663+
Expect(err).NotTo(HaveOccurred())
664+
665+
bytes, err := stack.RenderJSON()
666+
Expect(err).NotTo(HaveOccurred())
667+
668+
template, err := goformation.ParseJSON(bytes)
669+
Expect(err).NotTo(HaveOccurred())
670+
671+
// Should fall back to creating custom EFA security group resources
672+
Expect(template.Resources).To(HaveKey("EFASG"))
673+
Expect(template.Resources).To(HaveKey("EFAEgressSelf"))
674+
Expect(template.Resources).To(HaveKey("EFAIngressSelf"))
675+
})
676+
})
677+
678+
Context("EFA disabled", func() {
679+
It("does not create EFA security groups regardless of version", func() {
680+
clusterConfig.Metadata.Version = "1.33"
681+
ng := &api.ManagedNodeGroup{
682+
NodeGroupBase: &api.NodeGroupBase{
683+
Name: "no-efa-ng",
684+
InstanceType: "c5n.18xlarge",
685+
EFAEnabled: aws.Bool(false),
686+
},
687+
}
688+
689+
err := api.SetManagedNodeGroupDefaults(ng, clusterConfig.Metadata, false)
690+
Expect(err).NotTo(HaveOccurred())
691+
692+
stack := builder.NewManagedNodeGroup(provider.MockEC2(), clusterConfig, ng, builder.NewLaunchTemplateFetcher(provider.MockEC2()), bootstrapper, false, vpcImporter)
693+
err = stack.AddAllResources(context.Background())
694+
Expect(err).NotTo(HaveOccurred())
695+
696+
bytes, err := stack.RenderJSON()
697+
Expect(err).NotTo(HaveOccurred())
698+
699+
template, err := goformation.ParseJSON(bytes)
700+
Expect(err).NotTo(HaveOccurred())
701+
702+
// Should not have EFA security group resources
703+
Expect(template.Resources).NotTo(HaveKey("EFASG"))
704+
Expect(template.Resources).NotTo(HaveKey("EFAEgressSelf"))
705+
Expect(template.Resources).NotTo(HaveKey("EFAIngressSelf"))
706+
})
707+
})
708+
})
460709
})
461710

462711
func mockLaunchTemplate(matcher func(*ec2.DescribeLaunchTemplateVersionsInput) bool, lt *ec2types.ResponseLaunchTemplateData) func(provider *mockprovider.MockProvider) {

0 commit comments

Comments
 (0)