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

feat: Add tenancy option dedicated on LaunchTemplate #6360

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

DingGGu
Copy link

@DingGGu DingGGu commented Jun 14, 2024

Fixes #4633

Description
Support tenancy: dedicated options for mitigate compliance issues for some reason.

How was this change tested?

I confirmed that the dedicated instance was launched fine in my own eks cluster.

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

netlify bot commented Jun 14, 2024

Deploy Preview for karpenter-docs-prod ready!

Name Link
🔨 Latest commit ae90694
🔍 Latest deploy log https://app.netlify.com/sites/karpenter-docs-prod/deploys/666c08ba7de682000839bb67
😎 Deploy Preview https://deploy-preview-6360--karpenter-docs-prod.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@DingGGu DingGGu marked this pull request as ready for review June 14, 2024 07:02
@DingGGu DingGGu requested a review from a team as a code owner June 14, 2024 07:02
@DingGGu DingGGu requested a review from engedaam June 14, 2024 07:02
// https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-launchtemplate-placement.html
// +kubebuilder:validation:Enum:={default,dedicated}
// +optional
Tenancy *string `json:"tenancy,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need default or can we just assume that not specifying this option assumes that you are using "shared"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not specified, tenancy is "default (shared)".
If wish, We can also specify tenancy: "default" in yaml.

@@ -113,6 +113,16 @@ type EC2NodeClassSpec struct {
// +kubebuilder:default={"httpEndpoint":"enabled","httpProtocolIPv6":"disabled","httpPutResponseHopLimit":2,"httpTokens":"required"}
// +optional
MetadataOptions *MetadataOptions `json:"metadataOptions,omitempty"`
// Tenancy of the instance. An instance with a tenancy of dedicated runs on single-tenant hardware.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you able to workaround this limitation by having the VPC tenancy be dedicated? Or do you need a mix of dedicated and non-dedicated instances in the VPC?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically, dedicated tenancy has less available instance capacity in a region than default tenancy. This is a case where default and tenancy was mixed and used within a VPC.

@@ -433,6 +433,20 @@ spec:
rule: self.all(k, k !='karpenter.sh/nodeclaim')
- message: tag contains a restricted tag matching karpenter.k8s.aws/ec2nodeclass
rule: self.all(k, k !='karpenter.k8s.aws/ec2nodeclass')
tenancy:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your thoughts about doing something smarter here? Can Karpenter be aware of your dedicated instances and just use them? What happens if you run out of dedicated instances for the EC2NodeClass to launch?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is placement.tenancy information in the response of ec2 launch instance.
https://docs.aws.amazon.com/ko_kr/AWSEC2/latest/APIReference/API_Placement.html

Using this, We can enter tenancy information in nodeclass status, but I don't know if any other method can be implemented other than logging that the dedicated instance of the currently requested Instance requirement is Insufficient.

@DingGGu
Copy link
Author

DingGGu commented Jun 26, 2024

What we need to consider is whether to set the tenancy setting as a requirement (for example, karpenter.k8s.aws/instance-category) from the crd of karpenter NodeClass.

Copy link
Contributor

This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity.

@DingGGu
Copy link
Author

DingGGu commented Jul 15, 2024

It would be good to consider before releasing the CRD of karpenter v1.0. @jonathan-innis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose Tenancy field in AwsNodeTemplate API
2 participants