From 54a6a69f32e3fa23187ba772c98cac84e65c29a5 Mon Sep 17 00:00:00 2001 From: Michael Irwin Date: Fri, 21 Jan 2022 09:46:41 -0500 Subject: [PATCH] Add ability to specify AWS default instance profile Signed-off-by: Michael Irwin Resolves #909 --- charts/karpenter/README.md | 1 + .../templates/controller/deployment.yaml | 4 ++ .../templates/webhook/deployment.yaml | 4 ++ charts/karpenter/values.yaml | 3 ++ .../aws/apis/v1alpha1/provider.go | 4 +- .../aws/apis/v1alpha1/provider_validation.go | 8 ---- pkg/cloudprovider/aws/launchtemplate.go | 18 ++++++++- pkg/cloudprovider/aws/suite_test.go | 39 ++++++++++++++++--- pkg/utils/options/options.go | 20 +++++----- .../content/en/preview/AWS/provisioning.md | 6 +-- .../getting-started-with-terraform/_index.md | 6 ++- .../en/preview/getting-started/_index.md | 2 +- 12 files changed, 85 insertions(+), 30 deletions(-) diff --git a/charts/karpenter/README.md b/charts/karpenter/README.md index 219b03ca8d02..84a2c124fc92 100644 --- a/charts/karpenter/README.md +++ b/charts/karpenter/README.md @@ -25,6 +25,7 @@ You can follow the detailed installation instruction [here](https://karpenter.sh | Key | Type | Default | Description | |-----|------|---------|-------------| | additionalLabels | object | `{}` | Additional labels to add into metadata | +| aws.defaultInstanceProfile | string | `""` | The default instance profile to use when launching nodes on AWS | | controller.affinity | object | `{}` | Affinity rules for scheduling | | controller.clusterEndpoint | string | `""` | Cluster endpoint | | controller.clusterName | string | `""` | Cluster name | diff --git a/charts/karpenter/templates/controller/deployment.yaml b/charts/karpenter/templates/controller/deployment.yaml index 69c47e1543b7..19c0e239706e 100644 --- a/charts/karpenter/templates/controller/deployment.yaml +++ b/charts/karpenter/templates/controller/deployment.yaml @@ -63,6 +63,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace + {{- if .Values.aws.defaultInstanceProfile }} + - name: AWS_DEFAULT_INSTANCE_PROFILE + value: {{ .Values.aws.defaultInstanceProfile }} + {{- end }} {{- with .Values.controller.env }} {{- toYaml . | nindent 12 }} {{- end }} diff --git a/charts/karpenter/templates/webhook/deployment.yaml b/charts/karpenter/templates/webhook/deployment.yaml index 0173838efc7d..9f86f66f5da9 100644 --- a/charts/karpenter/templates/webhook/deployment.yaml +++ b/charts/karpenter/templates/webhook/deployment.yaml @@ -62,6 +62,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace + {{- if .Values.aws.defaultInstanceProfile }} + - name: AWS_DEFAULT_INSTANCE_PROFILE + value: {{ .Values.aws.defaultInstanceProfile }} + {{- end }} {{- with .Values.webhook.env }} {{- toYaml . | nindent 12 }} {{- end }} diff --git a/charts/karpenter/values.yaml b/charts/karpenter/values.yaml index 099d940795ef..d05748955558 100644 --- a/charts/karpenter/values.yaml +++ b/charts/karpenter/values.yaml @@ -55,3 +55,6 @@ webhook: cpu: 100m memory: 50Mi replicas: 1 +aws: + # -- The default instance profile to use when launching nodes on AWS + defaultInstanceProfile: "" \ No newline at end of file diff --git a/pkg/cloudprovider/aws/apis/v1alpha1/provider.go b/pkg/cloudprovider/aws/apis/v1alpha1/provider.go index 716db0b778b8..3dd3c6c2e216 100644 --- a/pkg/cloudprovider/aws/apis/v1alpha1/provider.go +++ b/pkg/cloudprovider/aws/apis/v1alpha1/provider.go @@ -43,8 +43,8 @@ type AWS struct { // TypeMeta includes version and kind of the extensions, inferred if not provided. // +optional metav1.TypeMeta `json:",inline"` - // InstanceProfile is the AWS identity that instances use. - // +required + // InstanceProfile to use for provisioned nodes, overriding the default profile. + // +optional InstanceProfile string `json:"instanceProfile"` // LaunchTemplate for the node. If not specified, a launch template will be generated. // +optional diff --git a/pkg/cloudprovider/aws/apis/v1alpha1/provider_validation.go b/pkg/cloudprovider/aws/apis/v1alpha1/provider_validation.go index 10701ee328c0..edf4e9e3134b 100644 --- a/pkg/cloudprovider/aws/apis/v1alpha1/provider_validation.go +++ b/pkg/cloudprovider/aws/apis/v1alpha1/provider_validation.go @@ -28,7 +28,6 @@ func (a *AWS) Validate() (errs *apis.FieldError) { func (a *AWS) validate() (errs *apis.FieldError) { return errs.Also( - a.validateInstanceProfile(), a.validateLaunchTemplate(), a.validateSubnets(), a.validateSecurityGroups(), @@ -37,13 +36,6 @@ func (a *AWS) validate() (errs *apis.FieldError) { ) } -func (a *AWS) validateInstanceProfile() (errs *apis.FieldError) { - if a.InstanceProfile == "" { - errs = errs.Also(apis.ErrMissingField("instanceProfile")) - } - return errs -} - func (a *AWS) validateLaunchTemplate() (errs *apis.FieldError) { // nothing to validate at the moment return errs diff --git a/pkg/cloudprovider/aws/launchtemplate.go b/pkg/cloudprovider/aws/launchtemplate.go index 5322e10887a4..36fc28364540 100644 --- a/pkg/cloudprovider/aws/launchtemplate.go +++ b/pkg/cloudprovider/aws/launchtemplate.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "encoding/base64" + "errors" "fmt" "sort" "strings" @@ -89,6 +90,10 @@ func (p *LaunchTemplateProvider) Get(ctx context.Context, constraints *v1alpha1. if constraints.LaunchTemplate != nil { return map[string][]cloudprovider.InstanceType{ptr.StringValue(constraints.LaunchTemplate): instanceTypes}, nil } + instanceProfile, err := p.getInstanceProfile(ctx, constraints) + if err != nil { + return nil, err + } // Get constrained security groups securityGroupsIds, err := p.securityGroupProvider.Get(ctx, constraints) if err != nil { @@ -111,7 +116,7 @@ func (p *LaunchTemplateProvider) Get(ctx context.Context, constraints *v1alpha1. launchTemplate, err := p.ensureLaunchTemplate(ctx, &launchTemplateOptions{ UserData: userData, ClusterName: injection.GetOptions(ctx).ClusterName, - InstanceProfile: constraints.InstanceProfile, + InstanceProfile: instanceProfile, AMIID: amiID, SecurityGroupsIds: securityGroupsIds, Tags: constraints.Tags, @@ -312,6 +317,17 @@ func (p *LaunchTemplateProvider) getNodeTaintArgs(constraints *v1alpha1.Constrai return nodeTaintsArgs } +func (p *LaunchTemplateProvider) getInstanceProfile(ctx context.Context, constraints *v1alpha1.Constraints) (string, error) { + if constraints.InstanceProfile != "" { + return constraints.InstanceProfile, nil + } + defaultProfile := injection.GetOptions(ctx).AWSDefaultInstanceProfile + if defaultProfile == "" { + return "", errors.New("neither spec.provider.instanceProfile nor --aws-default-instance-profile is specified") + } + return defaultProfile, nil +} + func (p *LaunchTemplateProvider) GetCABundle(ctx context.Context) (*string, error) { // Discover CA Bundle from the REST client. We could alternatively // have used the simpler client-go InClusterConfig() method. diff --git a/pkg/cloudprovider/aws/suite_test.go b/pkg/cloudprovider/aws/suite_test.go index b3279995fcf5..80cb32f4868b 100644 --- a/pkg/cloudprovider/aws/suite_test.go +++ b/pkg/cloudprovider/aws/suite_test.go @@ -69,10 +69,11 @@ func TestAPIs(t *testing.T) { var _ = BeforeSuite(func() { env = test.NewEnvironment(ctx, func(e *test.Environment) { opts = options.Options{ - ClusterName: "test-cluster", - ClusterEndpoint: "https://test-cluster", - AWSNodeNameConvention: string(options.IPName), - AWSENILimitedPodDensity: true, + ClusterName: "test-cluster", + ClusterEndpoint: "https://test-cluster", + AWSNodeNameConvention: string(options.IPName), + AWSENILimitedPodDensity: true, + AWSDefaultInstanceProfile: "test-instance-profile", } Expect(opts.Validate()).To(Succeed(), "Failed to validate options") ctx = injection.WithOptions(ctx, opts) @@ -122,7 +123,6 @@ var _ = Describe("Allocation", func() { BeforeEach(func() { provider = &v1alpha1.AWS{ - InstanceProfile: "test-instance-profile", SubnetSelector: map[string]string{"foo": "bar"}, SecurityGroupSelector: map[string]string{"foo": "bar"}, } @@ -506,6 +506,27 @@ var _ = Describe("Allocation", func() { Expect(string(userData)).To(ContainSubstring("--dns-cluster-ip '10.0.10.100'")) }) }) + Context("Instance Profile", func() { + It("should use the default instance profile if none specified on the Provisioner", func() { + provisioner.Spec.KubeletConfiguration.ClusterDNS = []string{"10.0.10.100"} + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, ProvisionerWithProvider(provisioner, provider), test.UnschedulablePod())[0] + ExpectScheduled(ctx, env.Client, pod) + Expect(fakeEC2API.CalledWithCreateLaunchTemplateInput.Cardinality()).To(Equal(1)) + input := fakeEC2API.CalledWithCreateLaunchTemplateInput.Pop().(*ec2.CreateLaunchTemplateInput) + Expect(*input.LaunchTemplateData.IamInstanceProfile.Name).To(Equal("test-instance-profile")) + }) + It("should use the instance profile on the Provisioner when specified", func() { + provider = &v1alpha1.AWS{InstanceProfile: "overridden-profile"} + ProvisionerWithProvider(&v1alpha5.Provisioner{ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}}, provider) + provisioner.SetDefaults(ctx) + + pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, ProvisionerWithProvider(provisioner, provider), test.UnschedulablePod())[0] + ExpectScheduled(ctx, env.Client, pod) + Expect(fakeEC2API.CalledWithCreateLaunchTemplateInput.Cardinality()).To(Equal(1)) + input := fakeEC2API.CalledWithCreateLaunchTemplateInput.Pop().(*ec2.CreateLaunchTemplateInput) + Expect(*input.LaunchTemplateData.IamInstanceProfile.Name).To(Equal("overridden-profile")) + }) + }) }) Context("Metadata Options", func() { It("should default metadata options on generated launch template", func() { @@ -539,6 +560,14 @@ var _ = Describe("Allocation", func() { }) }) Context("Defaulting", func() { + // Intent here is that if updates occur on the controller, the Provisioner doesn't need to be recreated + It("should not set the InstanceProfile with the default if none provided in Provisioner", func() { + provisioner.SetDefaults(ctx) + constraints, err := v1alpha1.Deserialize(&provisioner.Spec.Constraints) + Expect(err).ToNot(HaveOccurred()) + Expect(constraints.InstanceProfile).To(Equal("")) + }) + It("should default requirements", func() { provisioner.SetDefaults(ctx) Expect(provisioner.Spec.Requirements.CapacityTypes().UnsortedList()).To(ConsistOf(v1alpha1.CapacityTypeOnDemand)) diff --git a/pkg/utils/options/options.go b/pkg/utils/options/options.go index 992da685ee76..b6a90bbca1e4 100644 --- a/pkg/utils/options/options.go +++ b/pkg/utils/options/options.go @@ -41,6 +41,7 @@ func MustParse() Options { flag.IntVar(&opts.KubeClientBurst, "kube-client-burst", env.WithDefaultInt("KUBE_CLIENT_BURST", 300), "The maximum allowed burst of queries to the kube-apiserver") flag.StringVar(&opts.AWSNodeNameConvention, "aws-node-name-convention", env.WithDefaultString("AWS_NODE_NAME_CONVENTION", string(IPName)), "The node naming convention used by the AWS cloud provider. DEPRECATION WARNING: this field may be deprecated at any time") flag.BoolVar(&opts.AWSENILimitedPodDensity, "aws-eni-limited-pod-density", env.WithDefaultBool("AWS_ENI_LIMITED_POD_DENSITY", true), "Indicates whether new nodes should use ENI-based pod density") + flag.StringVar(&opts.AWSDefaultInstanceProfile, "aws-default-instance-profile", env.WithDefaultString("AWS_DEFAULT_INSTANCE_PROFILE", ""), "The default instance profile to use when provisioning nodes in AWS") flag.Parse() if err := opts.Validate(); err != nil { panic(err) @@ -50,15 +51,16 @@ func MustParse() Options { // Options for running this binary type Options struct { - ClusterName string - ClusterEndpoint string - MetricsPort int - HealthProbePort int - WebhookPort int - KubeClientQPS int - KubeClientBurst int - AWSNodeNameConvention string - AWSENILimitedPodDensity bool + ClusterName string + ClusterEndpoint string + MetricsPort int + HealthProbePort int + WebhookPort int + KubeClientQPS int + KubeClientBurst int + AWSNodeNameConvention string + AWSENILimitedPodDensity bool + AWSDefaultInstanceProfile string } func (o Options) Validate() (err error) { diff --git a/website/content/en/preview/AWS/provisioning.md b/website/content/en/preview/AWS/provisioning.md index 6310073ab04c..144a91f144b0 100644 --- a/website/content/en/preview/AWS/provisioning.md +++ b/website/content/en/preview/AWS/provisioning.md @@ -11,9 +11,9 @@ This section covers parameters of the AWS Cloud Provider. [Review these fields in the code.](https://github.com/awslabs/karpenter/blob/main/pkg/cloudprovider/aws/apis/v1alpha1/provider.go#L33) ### InstanceProfile -An `InstanceProfile` is a way to pass a single IAM role to an EC2 instance. - -It is required, and specified by name. A suitable `KarpenterNodeRole` is created in the getting started guide. +An `InstanceProfile` is a way to pass a single IAM role to an EC2 instance. Karpenter will not create one automatically. +A default profile may be specified on the controller, allowing it to be omitted here. If not specified as either a default +or on the controller, node provisioning will fail. ``` spec: diff --git a/website/content/en/preview/getting-started-with-terraform/_index.md b/website/content/en/preview/getting-started-with-terraform/_index.md index e3a2f9e7ea8f..77d3cf73c7e5 100644 --- a/website/content/en/preview/getting-started-with-terraform/_index.md +++ b/website/content/en/preview/getting-started-with-terraform/_index.md @@ -260,6 +260,11 @@ resource "helm_release" "karpenter" { name = "controller.clusterEndpoint" value = module.eks.cluster_endpoint } + + set { + name = "aws.defaultInstanceProfile" + value = aws_iam_instance_profile.karpenter.name + } } ``` @@ -310,7 +315,6 @@ spec: resources: cpu: 1000 provider: - instanceProfile: KarpenterNodeInstanceProfile-${CLUSTER_NAME} subnetSelector: karpenter.sh/discovery: ${CLUSTER_NAME} securityGroupSelector: diff --git a/website/content/en/preview/getting-started/_index.md b/website/content/en/preview/getting-started/_index.md index 564e9258684f..2afecf18d0f1 100644 --- a/website/content/en/preview/getting-started/_index.md +++ b/website/content/en/preview/getting-started/_index.md @@ -145,6 +145,7 @@ helm upgrade --install karpenter karpenter/karpenter --namespace karpenter \ --create-namespace --set serviceAccount.create=false --version {{< param "latest_release_version" >}} \ --set controller.clusterName=${CLUSTER_NAME} \ --set controller.clusterEndpoint=$(aws eks describe-cluster --name ${CLUSTER_NAME} --query "cluster.endpoint" --output json) \ + --set aws.defaultInstanceProfile=KarpenterNodeInstanceProfile-${CLUSTER_NAME} \ --wait # for the defaulting webhook to install before creating a Provisioner ``` @@ -226,7 +227,6 @@ spec: karpenter.sh/discovery: ${CLUSTER_NAME} securityGroupSelector: karpenter.sh/discovery: ${CLUSTER_NAME} - instanceProfile: KarpenterNodeInstanceProfile-${CLUSTER_NAME} ttlSecondsAfterEmpty: 30 EOF ```