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

GKE Autopilot Tutorial: Resource requirement update for userservice #1054

Merged
merged 5 commits into from
Nov 7, 2022

Conversation

oginskis
Copy link
Contributor

The Autopilot sets limits to request values (reference: https://cloud.google.com/kubernetes-engine/docs/concepts/autopilot-resource-requests#resource-limits) therefore the userservice gets only up to 200 milicores of CPU at maximum on the Autopilot. The userservice generates JWT tokens which are CPU-consuming and currently it takes several seconds for a user to log in.

I propose to set the CPU requests to 500m to improve the UX.

@bourgeoisor

@oginskis oginskis requested a review from a team as a code owner October 25, 2022 16:18
@bourgeoisor
Copy link
Member

Hi @oginskis! Two quick notes:

  1. The kubernetes-manifests/ directory is auto-generated with new releases and should not be manually modified. Instead, could you make your changes in dev-kubernetes-manifests/? Those changes are then picked up during the next release.

  2. I tested these changes in a GKE Standard default configuration and wasn't able to deploy successfully (not enough vCPU to deploy). Bank of Anthos currently can be deployed in a default GKE Standard cluster without any changes or scaling, and this needs to stay true for the many quickstart flows it supports. I have tried the following which did work, though: bumping the vCPU by 50% from 200m to 300m, as well as doubling memory from 64Mi to 128Mi. Would that work for you?

Feel free to make those two changes and I'll re-verify this PR

cpu: 200m
memory: 64Mi
cpu: 300m
memory: 128Mi
Copy link
Collaborator

@NimJay NimJay Nov 7, 2022

Choose a reason for hiding this comment

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

@bourgeoisor first off, thank you for thoroughly testing this pull-request.

Question: Have I interpreted your suggestions correctly?

Copy link
Member

Choose a reason for hiding this comment

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

Yep, thank you very much, Nim!

Copy link
Member

@bourgeoisor bourgeoisor left a comment

Choose a reason for hiding this comment

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

LGTM

@bourgeoisor bourgeoisor merged commit 34163ca into GoogleCloudPlatform:main Nov 7, 2022
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