Skip to content

kube: fix quantityToInt64 dropping scale for fractional BinarySI#28969

Merged
danishprakash merged 1 commit into
podman-container-tools:mainfrom
guillermodotn:main
Jun 19, 2026
Merged

kube: fix quantityToInt64 dropping scale for fractional BinarySI#28969
danishprakash merged 1 commit into
podman-container-tools:mainfrom
guillermodotn:main

Conversation

@guillermodotn

Copy link
Copy Markdown
Contributor

Replace AsDec().Unscaled() with Quantity.Value() in quantityToInt64(), which properly handles the decimal scale. Remove the now-unused error returns from quantityToInt64 and setupContainerResources. Add unit tests for fractional BinarySI inputs.

Fixes: #28789

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

  • Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
    commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
    the sign-off email address. See CONTRIBUTING.md
    for more information.
  • Referenced issues using Fixes: #00000 in commit message (if applicable)
  • Tests have been added/updated (or no tests are needed)
  • Documentation has been updated (or no documentation changes are needed)
  • All commits pass make validatepr (format/lint checks)
  • Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

Fixed `podman kube play` setting incorrect memory limits for fractional BinarySI quantities (e.g. `1.5Gi`).

@TomSweeneyRedHat

Copy link
Copy Markdown
Contributor

From a quick look, the changes look good. But it looks like you didn't sign your PR, and we can't merge until you do. git commit --amend -s should take care of it, with a fetch, rebase, and push. Thanks!

@guillermodotn guillermodotn force-pushed the main branch 2 times, most recently from ceca262 to de33b8a Compare June 19, 2026 03:55

@danishprakash danishprakash left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall looks good but could you also add an integration test? thanks

Signed-off-by: guillermodotn <guillerm0.n@outlook.es>
@guillermodotn

Copy link
Copy Markdown
Contributor Author

Overall looks good but could you also add an integration test? thanks

Added an integration test in test/e2e/play_kube_test.go. That said, I think the unit test alone is sufficient here. The bug is entirely in quantityToInt64(), a function that the unit test covers with the exact inputs that trigger the issue.

@Luap99 Luap99 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@danishprakash danishprakash enabled auto-merge June 19, 2026 16:05

@danishprakash danishprakash left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM (have rerun what seemed to be unrelated failures)

@danishprakash danishprakash merged commit 249df98 into podman-container-tools:main Jun 19, 2026
224 of 232 checks passed
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.

podman kube play mis-scales fractional BinarySI memory limits (e.g. 1.5Gi) by 10⁹ in quantityToInt64

4 participants