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

[WONTFIX]: Platform Engineer can limit memory used during staging using stagingRequirements.memoryMB #2654

Closed
tcdowney opened this issue Jun 30, 2023 · 2 comments
Assignees
Labels
wontfix This will not be worked on

Comments

@tcdowney
Copy link
Member

Blockers/Dependencies

No response

Background

As a Platform Engineer
I want to be able to set a memory limit for staging apps
So that I have more predictable utilization of my platform

Acceptance Criteria

GIVEN I have configured the stagingRequirements.memoryMB Helm value
WHEN I cf push an app
THEN I see that there is a memory resource limit on the kpack Image / Pod performing the build corresponding with the value I set

Dev Notes

  • NOTE this property already exists in the Helm chart, it is just not being used. We should be able to specify it as a resource limit here.
  • We want to set this as a limit instead of a request for now since a request may limit Pod placement too much. Eventually we'd want to support specifying the request on a per CFApp/CFBuild level since apps will have varying requirements here.
  • Ephemeral disk story: stagingRequirements.diskMB
@tcdowney
Copy link
Member Author

I left a comment with some thoughts on implementation the disk story that also apply to this one:
#2651 (comment)

@davewalter davewalter added this to the v0.8 milestone Jul 11, 2023
acosta11 added a commit that referenced this issue Jul 11, 2023
[#2654]

Reorganized the staging resource configuration under its own struct to
better capture the intent of the fields.

Co-authored-by: Dave Walter <[email protected]>
acosta11 added a commit that referenced this issue Jul 12, 2023
[#2654]

Reorganized the staging resource configuration under its own struct to
better capture the intent of the fields.

Co-authored-by: Dave Walter <[email protected]>
@tcdowney tcdowney removed this from the v0.8 milestone Jul 14, 2023
@tcdowney tcdowney added the wontfix This will not be worked on label Jul 14, 2023
@tcdowney tcdowney changed the title [Feature]: Platform Engineer can limit memory used during staging using stagingRequirements.memoryMB ~[Feature]: Platform Engineer can limit memory used during staging using stagingRequirements.memoryMB~ Jul 14, 2023
@tcdowney tcdowney changed the title ~[Feature]: Platform Engineer can limit memory used during staging using stagingRequirements.memoryMB~ ~~[Feature]: Platform Engineer can limit memory used during staging using stagingRequirements.memoryMB~~ Jul 14, 2023
@tcdowney tcdowney changed the title ~~[Feature]: Platform Engineer can limit memory used during staging using stagingRequirements.memoryMB~~ [WONTFIX]: Platform Engineer can limit memory used during staging using stagingRequirements.memoryMB Jul 14, 2023
@tcdowney
Copy link
Member Author

This was effectively undone by #2685.

This feature was originally oriented around setting limits. After some discussion on a Slack thread and a Github issue we decided to undo that. The original CF installation properties were interpreted as a minimum resource request versus a hard limit so we made this value optional (no requests) and allow operators to specify it if they want to set that minimum.

CF for VMs uses a quota system to set operator-specified limits -- so that might be something worth exploring if we want to do this in the future. Given that we don't want to do this right now and that the implementation/UX should likely look very different, I am going to close this issue as "wontfix".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
Archived in project
Development

No branches or pull requests

3 participants