jupyterhub / zero-to-jupyterhub-k8s

Helm Chart & Documentation for deploying JupyterHub on Kubernetes
https://zero-to-jupyterhub.readthedocs.io
Other
1.55k stars 795 forks source link

Inconsistent memory req/lim between single-userserver and user-placeholder #3539

Open dev-dsp opened 1 month ago

dev-dsp commented 1 month ago

Bug description

My configuration

singleuser:
  memory:
    limit: 1800M
    guarantee: 1800M
scheduling:
  userPlaceholder:
    enabled: true
    replicas: 4

Results in

$ kubectl describe pod jupyter-...
Containers:
  notebook:
    Limits:
      cpu:     2
      memory:  1887436800
    Requests:
      cpu:     500m
      memory:  1887436800

$ kubectl describe pod user-placeholder-...
Containers:
  pause:
    Limits:
      cpu:     2
      memory:  1800M
    Requests:
      cpu:        500m
      memory:     1800M

So 1887436800 bytes obviously is 1800Mi(B), which is 1887,4368 M(B), so not what I've requested. 1800M is a correct notation for MB, and should result in 1800000000 bytes (~1717 MiB)

For my HW this means that VM can fit 3 user-servers and 4 user-placeholders :shrug:

Configuration

Z2JH 3.2.1

dev-dsp commented 1 month ago

Actually it's even stranger now that I see the calculated value in user-server fields, while it would make more sense to find it in those of user-placeholder. Somehow user-placeholder has correct user-server values from my values.yaml, while user-server has something calculated.

dev-dsp commented 1 month ago

I've tried replacing requests/limits with 1717Mi, now I receive

    traitlets.traitlets.TraitError: 1717Mi is not a valid memory specification. Must be an int or a string with suffix K, M, G, T

And BackOff-ing hub :shrug:

consideRatio commented 1 month ago

Hehe sorry for the trouble with this, a workaround is to specify 123456... bytes directly.

The issue is that JupyterHub's k8s unaware config is used AND k8s native specification, and the translation cause issues. In the jupyterhub software, or traitlets config system software, the M means Mi, and in the k8s specification it means M and not Mi.

So, to avoid mismatch, use the lowest possible specification - the number of bytes as a plain integer.

dev-dsp commented 1 month ago

Yep, just fixed with bytes. But I believe this should be fixed in chart somehow?..

consideRatio commented 1 month ago

Yes! A fix is most welcome btw