From 1d566b9b0ad04d041e58e781c6485dcc67aeb58f Mon Sep 17 00:00:00 2001 From: Mahmoud Yaser Date: Tue, 13 Feb 2024 17:52:12 +0100 Subject: [PATCH] feat: Add AssociatePublicIpAddress to EC2NodeClass (#5437) Signed-off-by: Mahmoud Gaballah Co-authored-by: Jason Deal --- .../karpenter.k8s.aws_ec2nodeclasses.yaml | 4 ++ pkg/apis/v1beta1/ec2nodeclass.go | 3 ++ pkg/apis/v1beta1/zz_generated.deepcopy.go | 5 ++ .../launchtemplate/launchtemplate.go | 12 ++--- pkg/providers/launchtemplate/suite_test.go | 47 +++++++++++++++- .../integration/network_interface_test.go | 54 +++++++++++++++++++ .../en/preview/concepts/nodeclasses.md | 17 +++++- 7 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 test/suites/integration/network_interface_test.go diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index 5c9ab1ac51f3..8d1535ae6455 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -99,6 +99,10 @@ spec: of other fields in amiSelectorTerms' rule: '!self.all(x, has(x.id) && (has(x.tags) || has(x.name) || has(x.owner)))' + associatePublicIPAddress: + description: AssociatePublicIPAddress controls if public IP addresses + are assigned to instances that are launched with the nodeclass. + type: boolean blockDeviceMappings: description: BlockDeviceMappings to be applied to provisioned nodes. items: diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go index 2590d39e2273..9729450351f5 100644 --- a/pkg/apis/v1beta1/ec2nodeclass.go +++ b/pkg/apis/v1beta1/ec2nodeclass.go @@ -41,6 +41,9 @@ type EC2NodeClassSpec struct { // +kubebuilder:validation:MaxItems:=30 // +required SecurityGroupSelectorTerms []SecurityGroupSelectorTerm `json:"securityGroupSelectorTerms" hash:"ignore"` + // AssociatePublicIPAddress controls if public IP addresses are assigned to instances that are launched with the nodeclass. + // +optional + AssociatePublicIPAddress *bool `json:"associatePublicIPAddress,omitempty"` // AMISelectorTerms is a list of or ami selector terms. The terms are ORed. // +kubebuilder:validation:XValidation:message="expected at least one, got none, ['tags', 'id', 'name']",rule="self.all(x, has(x.tags) || has(x.id) || has(x.name))" // +kubebuilder:validation:XValidation:message="'id' is mutually exclusive, cannot be set with a combination of other fields in amiSelectorTerms",rule="!self.all(x, has(x.id) && (has(x.tags) || has(x.name) || has(x.owner)))" diff --git a/pkg/apis/v1beta1/zz_generated.deepcopy.go b/pkg/apis/v1beta1/zz_generated.deepcopy.go index 430fb6bb0840..8c7b0176f3cb 100644 --- a/pkg/apis/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/v1beta1/zz_generated.deepcopy.go @@ -223,6 +223,11 @@ func (in *EC2NodeClassSpec) DeepCopyInto(out *EC2NodeClassSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.AssociatePublicIPAddress != nil { + in, out := &in.AssociatePublicIPAddress, &out.AssociatePublicIPAddress + *out = new(bool) + **out = **in + } if in.AMISelectorTerms != nil { in, out := &in.AMISelectorTerms, &out.AMISelectorTerms *out = make([]AMISelectorTerm, len(*in)) diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index ea01721b0dad..c91f2bca3e09 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -183,11 +183,14 @@ func (p *Provider) createAMIOptions(ctx context.Context, nodeClass *v1beta1.EC2N KubeDNSIP: p.KubeDNSIP, NodeClassName: nodeClass.Name, } - if ok, err := p.subnetProvider.CheckAnyPublicIPAssociations(ctx, nodeClass); err != nil { + if nodeClass.Spec.AssociatePublicIPAddress != nil { + options.AssociatePublicIPAddress = nodeClass.Spec.AssociatePublicIPAddress + } else if ok, err := p.subnetProvider.CheckAnyPublicIPAssociations(ctx, nodeClass); err != nil { return nil, err } else if !ok { + // when `AssociatePublicIPAddress` is not specified in the `EC2NodeClass` spec, // If all referenced subnets do not assign public IPv4 addresses to EC2 instances therein, we explicitly set - // AssociatePublicIpAddress to 'false' in the Launch Template, generated based on this configuration struct. + // AssociatePublicIPAddress to 'false' in the Launch Template, generated based on this configuration struct. // This is done to help comply with AWS account policies that require explicitly setting of that field to 'false'. // https://github.com/aws/karpenter-provider-aws/issues/3815 options.AssociatePublicIPAddress = aws.Bool(false) @@ -292,11 +295,6 @@ func (p *Provider) generateNetworkInterfaces(options *amifamily.LaunchTemplate) }) } - // If all referenced subnets do not assign public IPv4 addresses to EC2 instances therein, we explicitly set - // AssociatePublicIpAddress to 'false' in the Launch Template, generated based on this configuration struct. - // This is done to help comply with AWS account policies that require explicitly setting that field to 'false'. - // This is ignored for EFA instances since it can't be specified if you launch with multiple network interfaces. - // https://github.com/aws/karpenter-provider-aws/issues/3815 if options.AssociatePublicIPAddress != nil { return []*ec2.LaunchTemplateInstanceNetworkInterfaceSpecificationRequest{ { diff --git a/pkg/providers/launchtemplate/suite_test.go b/pkg/providers/launchtemplate/suite_test.go index 3bff820d1a45..aaf548a780ac 100644 --- a/pkg/providers/launchtemplate/suite_test.go +++ b/pkg/providers/launchtemplate/suite_test.go @@ -1561,7 +1561,7 @@ var _ = Describe("LaunchTemplates", func() { }) }) Context("Subnet-based Launch Template Configration", func() { - It("should explicitly set 'AssignPublicIPv4' to false in the Launch Template", func() { + It("should explicitly set 'AssociatePublicIPAddress' to false in the Launch Template", func() { nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ {Tags: map[string]string{"Name": "test-subnet-1"}}, {Tags: map[string]string{"Name": "test-subnet-3"}}, @@ -1574,7 +1574,50 @@ var _ = Describe("LaunchTemplates", func() { Expect(*input.LaunchTemplateData.NetworkInterfaces[0].AssociatePublicIpAddress).To(BeFalse()) }) - It("should not explicitly set 'AssignPublicIPv4' when the subnets are configured to assign public IPv4 addresses", func() { + It("should overwrite 'AssociatePublicIPAddress' to true when specified by user in the EC2NodeClass", func() { + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + {Tags: map[string]string{"Name": "test-subnet-1"}}, + {Tags: map[string]string{"Name": "test-subnet-3"}}, + } + nodeClass.Spec.AssociatePublicIPAddress = lo.ToPtr(true) + + ExpectApplied(ctx, env.Client, nodePool, nodeClass) + pod := coretest.UnschedulablePod() + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) + ExpectScheduled(ctx, env.Client, pod) + input := awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.Pop() + Expect(*input.LaunchTemplateData.NetworkInterfaces[0].AssociatePublicIpAddress).To(BeTrue()) + }) + + It("should set 'AssociatePublicIPAddress' to false when specified by user in the EC2NodeClass", func() { + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + {Tags: map[string]string{"Name": "test-subnet-1"}}, + {Tags: map[string]string{"Name": "test-subnet-3"}}, + } + nodeClass.Spec.AssociatePublicIPAddress = lo.ToPtr(false) + + ExpectApplied(ctx, env.Client, nodePool, nodeClass) + pod := coretest.UnschedulablePod() + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) + ExpectScheduled(ctx, env.Client, pod) + input := awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.Pop() + Expect(*input.LaunchTemplateData.NetworkInterfaces[0].AssociatePublicIpAddress).To(BeFalse()) + }) + + It("should set 'AssociatePublicIPAddress' to false when not specified by the user in the EC2NodeClass using private subnet", func() { + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + {Tags: map[string]string{"Name": "test-subnet-1"}}, + {Tags: map[string]string{"Name": "test-subnet-3"}}, + } + + ExpectApplied(ctx, env.Client, nodePool, nodeClass) + pod := coretest.UnschedulablePod() + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) + ExpectScheduled(ctx, env.Client, pod) + input := awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.Pop() + Expect(*input.LaunchTemplateData.NetworkInterfaces[0].AssociatePublicIpAddress).To(BeFalse()) + }) + It("should not explicitly set 'AssociatePublicIPAddress' when the subnets are configured to assign public IPv4 addresses", func() { nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{{Tags: map[string]string{"Name": "test-subnet-2"}}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod() diff --git a/test/suites/integration/network_interface_test.go b/test/suites/integration/network_interface_test.go new file mode 100644 index 000000000000..8a5976fed031 --- /dev/null +++ b/test/suites/integration/network_interface_test.go @@ -0,0 +1,54 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integration_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/samber/lo" + "sigs.k8s.io/karpenter/pkg/test" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" +) + +var _ = Describe("NetworkInterfaces", func() { + DescribeTable( + "should correctly configure public IP assignment on instances", + func(associatePublicIPAddress *bool) { + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{{ + Tags: map[string]string{ + "Name": "*Private*", + "karpenter.sh/discovery": env.ClusterName, + }, + }} + nodeClass.Spec.AssociatePublicIPAddress = associatePublicIPAddress + + pod := test.Pod() + env.ExpectCreated(pod, nodeClass, nodePool) + env.EventuallyExpectHealthy(pod) + env.ExpectCreatedNodeCount("==", 1) + instance := env.GetInstance(pod.Spec.NodeName) + + if lo.FromPtr(associatePublicIPAddress) { + Expect(instance.PublicIpAddress).ToNot(BeNil()) + } else { + Expect(instance.PublicIpAddress).To(BeNil()) + } + }, + // Only tests private subnets since nodes w/o a public IP address in a public subnet will be unable to join the cluster + Entry("AssociatePublicIPAddress not specified by the user while using a private subnet", nil), + Entry("AssociatePublicIPAddress set true by user while using a private subnet", lo.ToPtr(true)), + ) +}) diff --git a/website/content/en/preview/concepts/nodeclasses.md b/website/content/en/preview/concepts/nodeclasses.md index a7fefbfeb1ee..77b650219475 100644 --- a/website/content/en/preview/concepts/nodeclasses.md +++ b/website/content/en/preview/concepts/nodeclasses.md @@ -1,4 +1,4 @@ ---- + --- title: "NodeClasses" linkTitle: "NodeClasses" weight: 2 @@ -113,6 +113,10 @@ spec: # Optional, configures detailed monitoring for the instance detailedMonitoring: true + + # Optional, configures if the instance should be launched with an associated public IP address. + # If not specified, the default value depends on the subnet's public IP auto-assign setting. + associatePublicIPAddress: true status: # Resolved subnets subnets: @@ -306,6 +310,7 @@ spec: - id: "subnet-0471ca205b8a129ae" ``` + ## spec.securityGroupSelectorTerms Security Group Selector Terms allow you to specify selection logic for all security groups that will be attached to an instance launched from the `EC2NodeClass`. The security group of an instance is comparable to a set of firewall rules. @@ -860,6 +865,16 @@ spec: detailedMonitoring: true ``` +## spec.associatePublicIPAddress + +A boolean field that controls whether instances created by Karpenter for this EC2NodeClass will have an associated public IP address. This overrides the `MapPublicIpOnLaunch` setting applied to the subnet the node is launched in. If this field is not set, the `MapPublicIpOnLaunch` field will be respected. + +{{% alert title="Note" color="warning" %}} +If a `NodeClaim` requests `vpc.amazonaws.com/efa` resources, the `associatePublicIPAddress` field is ignored. +A public IP address may only be associated with a node at launch if a single network interface is configured. +This is inherently incompatible with instances configured for EFA workloads since Karpenter will configure an EFA for each network card on the instance. +{{% /alert %}} + ## status.subnets [`status.subnets`]({{< ref "#statussubnets" >}}) contains the resolved `id` and `zone` of the subnets that were selected by the [`spec.subnetSelectorTerms`]({{< ref "#specsubnetselectorterms" >}}) for the node class. The subnets will be sorted by the available IP address count in decreasing order.