Skip to content

Expose flags for max-pods-calculator to use a configurable max pods ceiling#759

Closed
a7i wants to merge 1 commit intoawslabs:masterfrom
a7i:max-pods-ceiling
Closed

Expose flags for max-pods-calculator to use a configurable max pods ceiling#759
a7i wants to merge 1 commit intoawslabs:masterfrom
a7i:max-pods-ceiling

Conversation

@a7i
Copy link
Copy Markdown

@a7i a7i commented Sep 15, 2021

Issue #, if available:

Description of changes:
Provide the ability to pass in the ceiling for max-pods instead of relying on the default values for low/high cpu count.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@a7i
Copy link
Copy Markdown
Author

a7i commented Nov 9, 2021

@overdrive3000 this is a pretty small change, can this be reviewed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Apologies for not looking at this PR earlier.

Can we use a combination of this PR to achieve this? You can do the min_number logic on whatever invokes this script ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No worries @suket22

I'm not sure what SHOW_MAX_ALLOWED would do here

@o6uoq
Copy link
Copy Markdown

o6uoq commented Jun 16, 2022

+1 for having a ceiling but also the option to, within the script, to provide an override.

@o6uoq o6uoq mentioned this pull request Jun 16, 2022
@a7i a7i force-pushed the max-pods-ceiling branch from 5761c9a to 3915829 Compare July 19, 2022 15:36
@a7i a7i requested a review from suket22 July 19, 2022 15:36
@a7i
Copy link
Copy Markdown
Author

a7i commented Oct 20, 2023

@cartermckinnon would you please provide a reason why this was closed and why it's not a good change?

@cartermckinnon
Copy link
Copy Markdown
Contributor

I'm just going through and closing stale PR's, this one needed a rebase. I'm not really against the change if you're still interested, but there's been no other engagement and I assume you've solved this on your end by now?

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.

4 participants