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

[preview] Decrease preview env density #4750

Merged
merged 1 commit into from
Jul 8, 2021

Conversation

geropl
Copy link
Member

@geropl geropl commented Jul 8, 2021

No description provided.

@geropl geropl requested a review from a team as a code owner July 8, 2021 11:36
@geropl geropl requested review from aledbf and csweichel and removed request for a team July 8, 2021 11:36
@princerachit
Copy link
Contributor

Should we have an equivalent change in the other file/(s) too?

@geropl
Copy link
Member Author

geropl commented Jul 8, 2021

Should we have an equivalent change in the other file/(s) too?

Are we deploying multiple preview environments to them? If yes: maybe. But let's start with the problems we know about.

resources:
default:
# as opposed to 200Mi, the default
memory: 500Mi
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 500Mi? Is there some kind of calculation behind this akin to #4700 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

No calculation as I assumed assignment of static pods to be too dynamic to reason about.

Let's try. To come as close as possible to the theoretical limit of 33 preview envs are goals are:

  • don't consume too much RAM to drive node count up artificially
  • ensure we never reach "max nr of pods" with static deployments

Thus we make them big enough so that say 100 pods fill up the whole node (we ignore the DaemonSets here because they are quite small).
=> 32Gi / 100 ~ 328Mi
If we choose 350Mi we're coming close enough it seems.

WDYT? @csweichel

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me. Let's do this

@princerachit
Copy link
Contributor

Should we have an equivalent change in the other file/(s) too?

Are we deploying multiple preview environments to them? If yes: maybe. But let's start with the problems we know about.

Yes we are deploying multiple preview env to k3s ws cluster

@geropl
Copy link
Member Author

geropl commented Jul 8, 2021

Yes we are deploying multiple preview env to k3s ws cluster

But we're not having trouble with it atm, plus not all preview envs deploy to that cluster I recon. Let's wait until we have problems, we can quickly come back to this, then.

@geropl geropl force-pushed the gpl/previews-decrease-density branch from efa62f8 to 70dab6f Compare July 8, 2021 13:08
@geropl geropl force-pushed the gpl/previews-decrease-density branch from 70dab6f to 5fa2cb1 Compare July 8, 2021 13:10
@csweichel csweichel merged commit ca8d44a into main Jul 8, 2021
@csweichel csweichel deleted the gpl/previews-decrease-density branch July 8, 2021 14:54
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.

None yet

3 participants