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

Make docs for new flags more clear #1620

Merged
merged 1 commit into from
Aug 13, 2021
Merged

Conversation

rancher-max
Copy link
Contributor

@rancher-max rancher-max commented Aug 12, 2021

Types of Changes

docs for #1463

Verification

N/A

Linked Issues

#1525

Further Comments

More clear since config.yaml is our recommended approach of installing and specifying the flag multiple times there only uses the last value specified.

@rancher-max rancher-max requested a review from a team as a code owner August 12, 2021 19:06
@rancher-max
Copy link
Contributor Author

rancher-max commented Aug 12, 2021

Had to push an update to the documented memory values, otherwise we hit this error:

Aug 12 18:24:07 maxt2 rke2[43738]: time="2021-08-12T18:24:07Z" level=error msg="error parsing memory request for static pod kube-apiserver: quantities must match the regular expression '^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$'"
Aug 12 18:24:07 maxt2 rke2[43738]: time="2021-08-12T18:24:07Z" level=error msg="error parsing memory limit for static pod kube-apiserver: quantities must match the regular expression '^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$'"

@brandond
Copy link
Member

@rancher-max
Copy link
Contributor Author

Yeah it was really the 'B' that was breaking it. Setting it with Mi works fine as well as it can be parsed by the given regex. The upstream docs can be a little confusing due to statements like this:

... Each Container has a limit of 0.5 cpu and 128MiB of memory ...

But overall it's pretty clear there what to use, just making this update for users who don't want to go look upstream.

@brandond
Copy link
Member

You're right, that's probably worth opening an issue in the upstream docs. They shouldn't use units in the text that the thing they're documenting doesn't support.

@vadorovsky vadorovsky merged commit ddec2fe into rancher:master Aug 13, 2021
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.

3 participants