Skip to content
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.

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

Allowing settings of a default instance profile #914

Merged
merged 1 commit into from
Jan 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions charts/karpenter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
4 changes: 4 additions & 0 deletions charts/karpenter/templates/controller/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
4 changes: 4 additions & 0 deletions charts/karpenter/templates/webhook/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
3 changes: 3 additions & 0 deletions charts/karpenter/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,6 @@ webhook:
cpu: 100m
memory: 50Mi
replicas: 1
aws:
# -- The default instance profile to use when launching nodes on AWS
defaultInstanceProfile: ""
4 changes: 2 additions & 2 deletions pkg/cloudprovider/aws/apis/v1alpha1/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions pkg/cloudprovider/aws/apis/v1alpha1/provider_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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
Expand Down
18 changes: 17 additions & 1 deletion pkg/cloudprovider/aws/launchtemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"bytes"
"context"
"encoding/base64"
"errors"
"fmt"
"sort"
"strings"
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
mikesir87 marked this conversation as resolved.
Show resolved Hide resolved
AMIID: amiID,
SecurityGroupsIds: securityGroupsIds,
Tags: constraints.Tags,
Expand Down Expand Up @@ -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.
Expand Down
39 changes: 34 additions & 5 deletions pkg/cloudprovider/aws/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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"},
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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))
Expand Down
20 changes: 11 additions & 9 deletions pkg/utils/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions website/content/en/preview/AWS/provisioning.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
```

Expand Down Expand Up @@ -310,7 +315,6 @@ spec:
resources:
cpu: 1000
provider:
instanceProfile: KarpenterNodeInstanceProfile-${CLUSTER_NAME}
subnetSelector:
karpenter.sh/discovery: ${CLUSTER_NAME}
securityGroupSelector:
Expand Down
2 changes: 1 addition & 1 deletion website/content/en/preview/getting-started/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```

Expand Down Expand Up @@ -226,7 +227,6 @@ spec:
karpenter.sh/discovery: ${CLUSTER_NAME}
securityGroupSelector:
karpenter.sh/discovery: ${CLUSTER_NAME}
instanceProfile: KarpenterNodeInstanceProfile-${CLUSTER_NAME}
ttlSecondsAfterEmpty: 30
EOF
```
Expand Down