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.

Sign up for GitHub

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

Jump to bottom

AWS Cloudprovider now supports architecture flexibility in a single fleet request #711

Merged
merged 1 commit into from
Sep 29, 2021
Merged

AWS Cloudprovider now supports architecture flexibility in a single fleet request #711

merged 1 commit into from
Sep 29, 2021

Conversation

ellistarn
Copy link
Contributor

@ellistarn ellistarn commented Sep 28, 2021 •
edited

1. Issue, if available:
#703

2. Description of changes:
Now separates instance types by SSM parameter and passes separate LaunchConfigs to the Fleet API.

karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:17.055Z	INFO	controller.allocation.provisioner/default	Found 16 provisionable pods	{"commit": "551f2bf"}
 karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:18.336Z	DEBUG	controller.allocation.provisioner/default	Discovered 309 EC2 instance types	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:18.345Z	INFO	controller.allocation.provisioner/default	Computed packing for 15 pod(s) with instance type option(s) [m5.2xlarge]	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:18.359Z	INFO	controller.allocation.provisioner/default	Computed packing for 1 pod(s) with instance type option(s) [c6g.medium m5.2xlarge]	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:18.659Z	DEBUG	controller.allocation.provisioner/default	Discovered subnets: [subnet-04ffb1a3c5e6c19a3 subnet-0082841075ccc05d4 subnet-0550f22b5de615eac subnet-0f3fa09b6ca9de74a subnet-05ad119d43b8c07b3 subnet-043ae197020acadfb]	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:18.882Z	DEBUG	controller.allocation.provisioner/default	Discovered security groups: [sg-000d68e67c3fe53d8 sg-0c8c73058f4cbaaaa]	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:18.884Z	DEBUG	controller.allocation.provisioner/default	Discovered kubernetes version 1.20	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:18.937Z	DEBUG	controller.allocation.provisioner/default	Discovered ami ami-0494b922d4a70c8d6 for query /aws/service/eks/optimized-ami/1.20/amazon-linux-2/recommended/image_id	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:18.938Z	DEBUG	controller.allocation.provisioner/default	Discovered caBundle, length 1025	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:18.980Z	DEBUG	controller.allocation.provisioner/default	Discovered launch template Karpenter-etarn-10670878007216278433	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:22.176Z	INFO	controller.allocation.provisioner/default	Launched instance: i-0f76daeedd03b7aea, type: m5.2xlarge, zone: us-west-2b, hostname: ip-192-168-29-92.us-west-2.compute.internal	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:22.229Z	INFO	controller.allocation.provisioner/default	Bound 15 pod(s) to node ip-192-168-29-92.us-west-2.compute.internal	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:22.255Z	DEBUG	controller.allocation.provisioner/default	Discovered ami ami-029c456cf0e4bd912 for query /aws/service/eks/optimized-ami/1.20/amazon-linux-2-arm64/recommended/image_id	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:22.256Z	DEBUG	controller.allocation.provisioner/default	Discovered caBundle, length 1025	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:22.294Z	DEBUG	controller.allocation.provisioner/default	Discovered launch template Karpenter-etarn-14912319109985293255	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:22.297Z	DEBUG	controller.allocation.provisioner/default	Discovered caBundle, length 1025	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:24.239Z	INFO	controller.allocation.provisioner/default	Launched instance: i-0d841115239d55209, type: c6g.medium, zone: us-west-2a, hostname: ip-192-168-91-244.us-west-2.compute.internal	{"commit": "551f2bf"}
karpenter-controller-6c847fcb55-9cf2r manager 2021-09-29T01:05:24.256Z	INFO	controller.allocation.provisioner/default	Bound 1 pod(s) to node ip-192-168-91-244.us-west-2.compute.internal	{"commit": "551f2bf"}

3. Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: link to issue
  • No

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

@netlify
Copy link

netlify bot commented Sep 28, 2021 •
edited

✔️ Deploy Preview for karpenter-docs-prod canceled.

🔨 Explore the source changes: aa9ec5f

🔍 Inspect the deploy log: https://app.netlify.com/sites/karpenter-docs-prod/deploys/6154b8fd0970ca00077db884

@ellistarn ellistarn changed the title AWS Cloudprovider now supports architecture flexibility in a single fleet request [needs rebase #705] AWS Cloudprovider now supports architecture flexibility in a single fleet request Sep 29, 2021
@ellistarn ellistarn changed the title [needs rebase #705] AWS Cloudprovider now supports architecture flexibility in a single fleet request AWS Cloudprovider now supports architecture flexibility in a single fleet request Sep 29, 2021
bwagner5
bwagner5 reviewed Sep 29, 2021
View reviewed changes
pkg/cloudprovider/aws/instancetype.go Outdated
}
}
return architectures
return aws.StringValue(i.ProcessorInfo.SupportedArchitectures[0]) // Unrecognized, but used for error printing
Copy link
Contributor

Choose a reason for hiding this comment

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

should you just print both then if it's just used for error printing (join) ? Might be confusing if for some reason the first arch is i386

bwagner5
bwagner5 reviewed Sep 29, 2021
View reviewed changes
pkg/cloudprovider/aws/launchtemplate.go
func (p *LaunchTemplateProvider) Get(ctx context.Context, constraints *v1alpha1.Constraints, instanceTypes []cloudprovider.InstanceType) (string, error) {
// 1. If the customer specified a launch template then just use it
func (p *LaunchTemplateProvider) Get(ctx context.Context, constraints *v1alpha1.Constraints, instanceTypes []cloudprovider.InstanceType) (map[string][]cloudprovider.InstanceType, error) {
// If Launch Template is directly specified then just use it
if constraints.LaunchTemplate != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

thoughts on making this a list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you mean?

bwagner5
bwagner5 previously approved these changes Sep 29, 2021
View reviewed changes
Copy link
Contributor

@bwagner5 bwagner5 left a comment

Choose a reason for hiding this comment

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

Nice!

bwagner5
bwagner5 approved these changes Sep 29, 2021
View reviewed changes
Copy link
Contributor

@bwagner5 bwagner5 left a comment

Choose a reason for hiding this comment

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

lgtm

@ellistarn ellistarn merged commit 14dde17 into aws:main Sep 29, 2021
@ellistarn ellistarn deleted the archfix branch September 29, 2021 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwagner5 bwagner5 bwagner5 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ellistarn @bwagner5