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

fix binpacking log to show proper node quantity and pods #825

Merged
merged 4 commits into from
Nov 23, 2021

Conversation

bwagner5
Copy link
Contributor

1. Issue, if available:
N/A

2. Description of changes:

  • The packing log that should tell how many nodes it packed to and how many pods it packed was incorrectly logging a single node packing even when node quantity was > 1.

Example bad log:

2021-11-09T17:48:28.330Z	INFO	controller.allocation.provisioner/default	Computed packing for 127 pod(s) with instance type option(s) [c6i.32xlarge m6i.32xlarge]	{"commit": "82226ca"}
2021-11-09T17:48:28.542Z	INFO	controller.allocation.provisioner/default	Computed packing for 71 pod(s) with instance type option(s) [c5.18xlarge c5d.18xlarge c5n.18xlarge c6i.24xlarge c5d.24xlarge c5a.24xlarge c5.24xlarge c5ad.24xlarge c6i.32xlarge m5d.24xlarge m6i.24xlarge m5.24xlarge m5a.24xlarge m5ad.24xlarge m5dn.24xlarge m5n.24xlarge m6i.32xlarge r5ad.24xlarge r5b.24xlarge r5a.24xlarge]	{"commit": "82226ca"}
2021-11-09T17:48:31.692Z	INFO	controller.allocation.provisioner/default	Launched instance: i-0f8265d219c280e4d, hostname: ip-10-0-238-146.us-east-2.compute.internal, type: c6i.32xlarge, zone: us-east-2c, capacityType: on-demand	{"commit": "82226ca"}
2021-11-09T17:48:31.692Z	INFO	controller.allocation.provisioner/default	Launched instance: i-04fde67af0c30f65a, hostname: ip-10-0-211-218.us-east-2.compute.internal, type: c6i.32xlarge, zone: us-east-2c, capacityType: on-demand	{"commit": "82226ca"}
2021-11-09T17:48:31.692Z	INFO	controller.allocation.provisioner/default	Launched instance: i-0a59c9e463c985d7a, hostname: ip-10-0-201-31.us-east-2.compute.internal, type: c6i.32xlarge, zone: us-east-2c, capacityType: on-demand	{"commit": "82226ca"}
2021-11-09T17:48:31.692Z	INFO	controller.allocation.provisioner/default	Launched instance: i-0c7acd653104b0566, hostname: ip-10-0-249-63.us-east-2.compute.internal, type: c6i.32xlarge, zone: us-east-2c, capacityType: on-demand	{"commit": "82226ca"}
2021-11-09T17:48:31.692Z	INFO	controller.allocation.provisioner/default	Launched instance: i-0da0793d3902502a2, hostname: ip-10-0-229-44.us-east-2.compute.internal, type: c6i.32xlarge, zone: us-east-2c, capacityType: on-demand	{"commit": "82226ca"}
2021-11-09T17:48:31.692Z	INFO	controller.allocation.provisioner/default	Launched instance: i-0933578060fb13476, hostname: ip-10-0-247-173.us-east-2.compute.internal, type: c6i.32xlarge, zone: us-east-2c, capacityType: on-demand	{"commit": "82226ca"}

Example Fixed Log:

karpenter-controller-65567bbc4d-mxwk2 manager 2021-11-19T21:20:20.915Z	INFO	controller.provisioning	Computed packing of 15 nodes for 1905 pod(s) with instance type option(s) [c6i.32xlarge m6i.32xlarge]	{"commit": "b4a52e2", "provisioner": "default"}
karpenter-controller-65567bbc4d-mxwk2 manager 2021-11-19T21:20:20.915Z	INFO	controller.provisioning	Computed packing of 1 nodes for 95 pod(s) with instance type option(s) [c6i.24xlarge c5a.24xlarge c5ad.24xlarge c5d.24xlarge c5.24xlarge c6i.32xlarge m5.24xlarge m5d.24xlarge m5a.24xlarge m5dn.24xlarge m5n.24xlarge m5ad.24xlarge m6i.24xlarge m6i.32xlarge r5n.24xlarge r5b.24xlarge r5.24xlarge r5ad.24xlarge r5a.24xlarge r5d.24xlarge]	{"commit": "b4a52e2", "provisioner": "default"}

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 Nov 19, 2021

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

🔨 Explore the source changes: 7fb9081

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

ellistarn
ellistarn previously approved these changes Nov 22, 2021
Copy link
Contributor

@ellistarn ellistarn left a comment

Choose a reason for hiding this comment

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

One minor. Nice fix!

pkg/controllers/provisioning/binpacking/packer.go Outdated Show resolved Hide resolved
@ellistarn ellistarn merged commit 568e82e into aws:main Nov 23, 2021
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.

3 participants