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

Feat: Add cpu entitlement to app process table #2840

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

ctlong
Copy link
Member

@ctlong ctlong commented Apr 1, 2024

Where this PR should be backported?

  • main - all changes should by default start here
  • v8
  • v7

Description of the Change

Extend the v8 cf CLI to support showing the new CPU Entitlement metric as a non-breaking change, with the cpu entitlement column appearing before the details column:

     state     since                  cpu    memory        disk           logging        cpu entitlement   details
#0   running   2024-02-10T00:30:38Z   0.2%   47.7M of 1G   129.6M of 1G   0/s of 16K/s   5.0%
#1   running   2024-02-12T18:17:49Z   0.1%   47.3M of 1G   129.6M of 1G   0/s of 16K/s   2.5%

If the cpu entitlement stats are not available (e.g. deployment does not support it, app is stopped, etc.) then the cf CLI should show an empty value for the process column.

Why Is This PR Valuable?

Extend the v8 cf CLI to support showing the new CPU Entitlement metric, where applicable.

Applicable Issues

How Urgent Is The Change?

Slightly less than urgent.

Other Relevant Parties

@rroberts2222

Shows the new CPU Entitlement metric in the app process table, under the
heading `cpu entitlement`. If CPU Entitlement metrics are not available
(e.g. deployment does not support it, app is stopped, etc.) for a
process instance, then that row will show an empty value.

Signed-off-by: Rebecca Roberts <rebecca.roberts@broadcom.com>
@ctlong ctlong marked this pull request as ready for review April 2, 2024 18:06
@ctlong
Copy link
Member Author

ctlong commented Apr 2, 2024

We're not sure why this change is failing unit tests in macOS... it seems unrelated to our changes. When we run unit tests on the main branch on my local mac, we see these tests flake one out of every 7 runs or so.

@ctlong
Copy link
Member Author

ctlong commented Apr 2, 2024

Using the following command, I was able to run unit tests for this branch on my local mac 23 times in a row without failure:

CF_HOME=$PWD/fixtures CF_USERNAME="" CF_PASSWORD="" ginkgo -r -randomize-all -require-suite -randomize-suites -skip-package integration,cf/ssh,plugin,cf/actors/plugin,cf/commands/plugin,cf/actors/plugin,util/randomword -until-it-fails

@moleske
Copy link
Member

moleske commented Apr 3, 2024

I like clicking rerun so I've unstuck the pr checks (for now)

Please also open a pr against the main branch as any changes in the v8 branch we'll want in main as well
edit - as main is theoretically a new major version, feel free to do great breaking changes there (well at least I say do it, not sure how other cli approvers feel)

@ctlong
Copy link
Member Author

ctlong commented Apr 3, 2024

We've another change in progress for main right now.

Where this change represents the "alternative option" described in #2812, for cf CLI v9 we're aiming for the proposed solution in that issue, which we prefer 😄

@a-b a-b merged commit 5c2a1f4 into cloudfoundry:v8 Apr 30, 2024
11 of 13 checks passed
@ctlong ctlong deleted the feat/cpu-entitlement branch April 30, 2024 22:33
vchrisb pushed a commit to vchrisb/cli that referenced this pull request May 3, 2024
Shows the new CPU Entitlement metric in the app process table, under the
heading `cpu entitlement`. If CPU Entitlement metrics are not available
(e.g. deployment does not support it, app is stopped, etc.) for a
process instance, then that row will show an empty value.

Signed-off-by: Rebecca Roberts <rebecca.roberts@broadcom.com>
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

4 participants