Skip to content

Commit

Permalink
Change name to EC2NodeClass and remove some fast-follow features
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathan-innis committed Sep 14, 2023
1 parent 163b70c commit 4b8edd2
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 19 deletions.
15 changes: 7 additions & 8 deletions designs/v1beta1-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ To workaround the limitation of round-trippability, we are proposing a rename of

#### Aggressive Scale-Down

1. Spin up a new `v1beta1/NodePool` and `v1beta1/NodeClass` resource matching their `v1alpha5` counterparts
1. Spin up a new `v1beta1/NodePool` and `v1beta1/EC2NodeClass` resource matching their `v1alpha5` counterparts
2. Delete the `v1alpha5/Provisioner` and `v1alpha5/AWSNodeTemplate` with [cascading foreground deletion](https://kubernetes.io/docs/concepts/architecture/garbage-collection/#foreground-deletion) to delete the nodes and have Karpenter roll the nodes to `v1beta1`

#### Slow Scale-Down

1. Spin up a new `v1beta1/NodePool` and `v1beta1/NodeClass` resource matching their `v1alpha5` counterparts
1. Spin up a new `v1beta1/NodePool` and `v1beta1/EC2NodeClass` resource matching their `v1alpha5` counterparts
2. Set the `spec.limits` on your `v1alpha5/Provisioner` resource to `cpu=0` to stop provisioning
3. Manually delete Nodes one-by-one to have Karpenter roll the nodes over to `v1beta1`

Expand All @@ -36,7 +36,7 @@ To help clearly define where configuration should live within Karpenter’s API,
1. `NodePool`
1. Neutral Node configuration-based fields that affect the **compatibility between Nodes and Pods during scheduling** (e.g. requirements, taints, labels)
2. Neutral behavior-based fields for configuring Karpenter’s scheduling and deprovisioning decision-making
2. `NodeClass`
2. `EC2NodeClass`
1. Cloudprovider-specific Node configuration-based fields that affect launch and bootstrap process for that Node including: configuring startup scripts, volume mappings, metadata settings, etc.
2. Cloudprovider-specific behavior-based fields for configuring Karpenter’s scheduling and deprovisioning decision-making (e.g. interruption-based disruption, allocation strategy)
3. `NodeClaim`
Expand All @@ -59,7 +59,7 @@ spec:
spec:
nodeClass:
name: default
kind: NodeClass
kind: EC2NodeClass
apiVersion: compute.k8s.aws/v1beta1
taints:
- key: example.com/special-taint
Expand Down Expand Up @@ -119,11 +119,11 @@ status:
ephemeral-storage: "100Gi"
```

### `compute.k8s.aws/NodeClass`
### `compute.k8s.aws/EC2NodeClass`

```
apiVersion: compute.k8s.aws/v1beta1
kind: NodeClass
kind: EC2NodeClass
metadata:
name: default
spec:
Expand All @@ -134,7 +134,6 @@ spec:
- id: abc-123
- name: foo
owner: amazon
- ssm: "/my/custom/parameter"
subnetSelectorTerms:
- tags:
compute.k8s.aws/discovery: cluster-name
Expand Down Expand Up @@ -236,7 +235,7 @@ metadata:
spec:
nodeClass:
name: default
kind: NodeClass
kind: EC2NodeClass
apiVersion: compute.k8s.aws/v1beta1
taints:
- key: example.com/special-taint
Expand Down
20 changes: 9 additions & 11 deletions designs/v1beta1-full-changelist.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ As part of the bump to v1beta1, to allow the v1alpha5 APIs to exist alongside th
2. Kind Renames
1. `Provisioner``NodePool`
2. `Machine` -> `NodeClaim`
3. `AWSNodeTemplate``NodeClass`
3. `AWSNodeTemplate``EC2NodeClass`

We see the renames as opportunities to better align our API groups and kinds with upstream concepts as well as reducing confusion between other Kubernetes API concepts. Specifically, the word `Provisioner` (on its own) has become overloaded in Kubernetes, [particularly in the area of storage provisioning](https://kubernetes.io/docs/concepts/storage/storage-classes/#the-storageclass-resource). We want to get completely away from this naming, while also prefixing all of our kinds that apply to nodes with `Node` for better alignment and clarity across the project.

This gives the following naming to API types within the Karpenter project

1. `karpenter.sh/NodePool`
2. `karpenter.sh/NodeClaim`
3. `compute.k8s.aws/NodeClass`
3. `compute.k8s.aws/EC2NodeClass`

### Remove Validation/Mutating Webhooks in favor of CEL (Common Expression Language)

Expand Down Expand Up @@ -103,9 +103,9 @@ Karpenter will change the `ttlSecondsUntilExipred` field to `expireAfter` to ali

We’ve recommended that customers leverage spec.providerRef in favor of spec.provider since Q2 2022. Documentation for this feature has been removed since Q3 2022. We will take the opportunity to remove the feature entirely to minimize code bugs/complexity and user confusion.

### `NodeClass` Changes
### `EC2NodeClass` Changes

#### Update `nodeClass.spec.amiSelector`
#### Update `EC2NodeClass.spec.amiSelector`

The alpha API `amiSelector` has two primary limitations that restrict user’s ability to specify the AMIs that they want Karpenter to use:

Expand All @@ -118,14 +118,12 @@ amiSelectorTerms:
- name: foo
id: abc-123
owner: amazon
ssm: "/al2/alias"
tags:
key: value
# Selector Terms are ORed
- name: foo
id: abc-123
owner: self
ssm: "/al2/alias"
tags:
key: value
```
Expand Down Expand Up @@ -169,7 +167,7 @@ Direct launch template support is problematic for many reasons, outlined in the

#### `nodeTemplate.spec.instanceProfile``nodeClass.spec.role`

Currently, Karpenter uses an `instanceProfile` in the `AWSNodeTemplate` that is referenced to determine the profile that the EC2 node should launch with. Instance profiles are IAM entities that are specific to EC2 and do not have a lot of detail built around them (including console support); users are generally more familiar with the concept of IAM roles. As a result, we can support a `role` in the new `NodeClass` and allow Karpenter to provision the instance profile `ad-hoc` with the `role` specified attached to it.
Currently, Karpenter uses an `instanceProfile` in the `AWSNodeTemplate` that is referenced to determine the profile that the EC2 node should launch with. Instance profiles are IAM entities that are specific to EC2 and do not have a lot of detail built around them (including console support); users are generally more familiar with the concept of IAM roles. As a result, we can support a `role` in the new `EC2NodeClass` and allow Karpenter to provision the instance profile `ad-hoc` with the `role` specified attached to it.

#### Remove tag-based AMI Requirements

Expand All @@ -192,16 +190,16 @@ This functionality of Karpenter hasn’t been surfaced widely at this point in t

#### Deprecate `defaultInstanceProfile` in `karpenter-global-settings`

InstanceProfile, SubnetSelector, and SecurityGroup are all required information to launch nodes. Currently InstanceProfile is set in default settings, but subnetSelector and securityGroupSelector aren't. This is awkward and [doesn't provide a consistent experience for users](https://github.com/aws/karpenter/issues/2973). We should align all of our configuration at the `NodeClass` and `Provisioner` -level for users to streamline their experience.
InstanceProfile, SubnetSelector, and SecurityGroup are all required information to launch nodes. Currently InstanceProfile is set in default settings, but subnetSelector and securityGroupSelector aren't. This is awkward and [doesn't provide a consistent experience for users](https://github.com/aws/karpenter/issues/2973). We should align all of our configuration at the `EC2NodeClass` and `Provisioner` -level for users to streamline their experience.

#### Deprecate `tags` from `karpenter-global-settings` in favor of `nodeClass.tags`
#### Deprecate `tags` from `karpenter-global-settings` in favor of `nodeClass.spec.tags`

Having `tags` inside of the `karpenter-global-settings` makes it difficult to detect drift when these tag values are changed. Since the primary reason this field exists inside the `karpenter-global-settings` is for ease-of-use, and there is a simple workaround for customers (setting consistent tags inside each `NodeClass`), it makes natural sense to remove this from the `karpenter-global-settings` .
Having `tags` inside of the `karpenter-global-settings` makes it difficult to detect drift when these tag values are changed. Since the primary reason this field exists inside the `karpenter-global-settings` is for ease-of-use, and there is a simple workaround for customers (setting consistent tags inside each `EC2NodeClass`), it makes natural sense to remove this from the `karpenter-global-settings` .

#### Remove `aws.enablePodENI` from `karpenter-global-settings`

This value has no meaning anymore now that our initialization logic does not rely on it. This can be pulled out of the `karpenter-global-settings` without causing impact to users.

#### Deprecate `aws.enableENILimitedPodDensity` in `karpenter-global-settings`

Setting static pod density is available through the `nodeClass.kubelet.maxPods` so there is no need for this setting to be configured at a global level anymore.
Setting static pod density is available through the `EC2NodeClass.kubelet.maxPods` so there is no need for this setting to be configured at a global level anymore.

0 comments on commit 4b8edd2

Please sign in to comment.