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

[CI:DOCS] man pages: replace -c with --cpu-shares #14915

Merged
merged 2 commits into from
Jul 13, 2022

Conversation

eriksjolund
Copy link
Contributor

These three man pages have some similar text.

For example, consider a system with more than three cores. If you start one
container **{C0}** with **--cpu-shares=512** running one process, and another container
**{C1}** with **--cpu-shares=1024** running two processes, this can result in the following
division of CPU shares:
| PID | container | CPU | CPU share |
| ---- | ----------- | ------- | ------------ |
| 100 | {C0} | 0 | 100% of CPU0 |
| 101 | {C1} | 1 | 100% of CPU1 |
| 102 | {C1} | 2 | 100% of CPU2 |

For example, consider a system with more than three cores. If you start one
container **{C0}** with **-c=512** running one process, and another container
**{C1}** with **-c=1024** running two processes, this can result in the following
division of CPU shares:
PID container CPU CPU share
100 {C0} 0 100% of CPU0
101 {C1} 1 100% of CPU1
102 {C1} 2 100% of CPU2

For example, consider a system with more than three cores.
container **{C0}** is started with **-c=512** running one process, and another container
**{C1}** with **-c=1024** running two processes, this can result in the following
division of CPU shares:
PID container CPU CPU share
100 {C0} 0 100% of CPU0
101 {C1} 1 100% of CPU1
102 {C1} 2 100% of CPU2

I assume podman-run.1.md is the correct one (i.e. using --cpu-shares= instead of -c=)

Signed-off-by: Erik Sjölund [email protected]

Does this PR introduce a user-facing change?

None

@rhatdan
Copy link
Member

rhatdan commented Jul 12, 2022

-c and --cpu-shares are the same, although having these use --cpu-shares is easier for humans to understand.
LGTM

@rhatdan
Copy link
Member

rhatdan commented Jul 12, 2022

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 12, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eriksjolund, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 12, 2022
@eriksjolund
Copy link
Contributor Author

eriksjolund commented Jul 12, 2022

It seems the man pages don't mention -c

#### **--cpu-shares**=*shares*

#### **--cpu-shares**=*shares*

#### **--cpu-shares**=*shares*

Should I add that?

@eriksjolund
Copy link
Contributor Author

-c is not mentioned here either

$ podman create --help| grep cpu-sh
      --cpu-shares uint                          CPU shares (relative weight)
$ podman run --help| grep cpu-sh
      --cpu-shares uint                          CPU shares (relative weight)
$ podman container clone --help| grep cpu-sh
      --cpu-shares uint                       CPU shares (relative weight)
$ 

@rhatdan
Copy link
Member

rhatdan commented Jul 12, 2022

Indeed you are right. Looks like you have found a compatibility issue.

$ podman create --help | grep -- --cpu-shares
      --cpu-shares uint                          CPU shares (relative weight)
$ docker create --help | grep -- --cpu-shares
  -c, --cpu-shares int                 CPU shares (relative weight)

@TomSweeneyRedHat
Copy link
Member

@edsantiago sorry, I wasn't clear. I wasn't looking to get them all removed at this point, a fun weekend project for me. What I was wondering if there's a new " you " added in PR for the man pages, just the ones that live in https://github.com/containers/podman/tree/main/docs/source/markdown, then the CI would throw a warning or perhaps fail.

Pronouns in readme's, tutorials, and other docs are fine. We just want to keep the man pages crisp and sans pronouns, or at least " you ".

@edsantiago
Copy link
Member

That would require running git diff, which is a big can of worms. I'm sorry.

@TomSweeneyRedHat
Copy link
Member

@edsantiago no worries at all, I was hoping it might be quick and easy. Thanks!

@eriksjolund eriksjolund force-pushed the rename_c_to_cpu_shares branch 3 times, most recently from 9e61ee3 to 9ea32f2 Compare July 13, 2022 10:04
@rhatdan
Copy link
Member

rhatdan commented Jul 13, 2022

@eriksjolund You have whitespace issues, and then we can merge.

@eriksjolund eriksjolund force-pushed the rename_c_to_cpu_shares branch from 9ea32f2 to 1392922 Compare July 13, 2022 14:07
@eriksjolund
Copy link
Contributor Author

whitespace now removed

@rhatdan
Copy link
Member

rhatdan commented Jul 13, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2022
@openshift-ci openshift-ci bot merged commit 5f8d08d into containers:main Jul 13, 2022
@eriksjolund eriksjolund deleted the rename_c_to_cpu_shares branch July 27, 2022 04:14
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants