Skip to content
This repository was archived by the owner on Jun 29, 2022. It is now read-only.

Add memory requests for nodes and controlplane workloads - #49

Open
Mateusz Gozdek (invidian) wants to merge 2 commits into
masterfrom
invidian/memory-requests
Open

Mateusz Gozdek (invidian) wants to merge 2 commits into
masterfrom
invidian/memory-requests

Conversation

@invidian

@invidian Mateusz Gozdek (invidian) commented Feb 25, 2020

Copy link
Copy Markdown
Member

This commit adds memory requests for all nodes and controlplane
workloads. The reason behind it is to better show user available
resources on both worker and controller nodes e.g. when doing 'kubectl
describe node'. This is important while one scales the controlplane
deployments and may prevent node eviction.

The measurment was done on freshly created cluster, with
prometheus-operator and metrics-server deployed,
on controller node and on worker node, so the numbers might be lower
than the numbers on long-running cluster, but they give at least some
initial visibility.

The values were measured using 'systemd-cgtop -m -1 / --depth=1', not
using 'free', as 'systemd-cgtop' also includes page cache usage, the
same way 'kubelet' is measuring the memory usage.

Before the measurment, following command has been executed:
'sync; echo 1 | sudo tee /proc/sys/vm/drop_caches; sleep 10'
To make sure only active memory has been captured.

system.slice uses ~250Mi, init.scope uses ~200Mi, which sums up to
roughly 500Mi, which is needed for system.

Kubelet in /docker slice was using ~100Mi. etcd in /docker slice was
using ~200Mi, so workers has 100Mi reserved for 'kube' and controllers
has 300Mi.

Memory usage for self-hosted components has been measured using the
following command: 'kubectl top pods --sort-by=memory | sort -h -k3 -r'.

Then, the read values were rounded up a bit.

Signed-off-by: Mateusz Gozdek mateusz@kinvolk.io

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think adding these fields makes sense, but not sure which was the reasoning to pick them. Can you please share the reasoning (and add it to the commit)?

I can't really review if the numbers make sense without knowing how they were calculated :)

--register-with-taints=$${NODE_TAINTS} \
--volume-plugin-dir=/var/lib/kubelet/volumeplugins
--volume-plugin-dir=/var/lib/kubelet/volumeplugins \
--kube-reserved=memory=500Mi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Where does this value come from?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The values are a snapshot of kubectl top pods --sort-by=memory | sort -h -k3 -r after deploying a cluster from example configuration. They should perhaps be configurable, as depending on the cluster size, they will be changing.

Those values are minimal, just to have something in place.

Should I add such comment in commit message?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, please do.

I'd like to get some values from production clusters maybe too... But we can update later, maybe?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wow, kubelet+docker was using 500mb?

Note ssh, etc. should go under --system-reserved, say the docs

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah right. I'll use system-reserved then.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done. PTAL.

@invidian

Copy link
Copy Markdown
Member Author

I think adding these fields makes sense, but not sure which was the reasoning to pick them. Can you please share the reasoning (and add it to the commit)?

I can't really review if the numbers make sense without knowing how they were calculated :)

Right, sorry for not providing that straight away. I've added methods of measuring etc to the commit message now. Please take a look.

This commit adds memory requests for all nodes and controlplane
workloads. The reason behind it is to better show user available
resources on both worker and controller nodes e.g. when doing 'kubectl
describe node'. This is important while one scales the controlplane
deployments and may prevent node eviction.

The measurment was done on freshly created cluster, with
prometheus-operator and metrics-server deployed,
on controller node and on worker node, so the numbers might be lower
than the numbers on long-running cluster, but they give at least some
initial visibility.

The values were measured using 'systemd-cgtop -m -1 / --depth=1', not
using 'free', as 'systemd-cgtop' also includes page cache usage, the
same way 'kubelet' is measuring the memory usage.

Before the measurment, following command has been executed:
'sync; echo 1 | sudo tee /proc/sys/vm/drop_caches; sleep 10'
To make sure only active memory has been captured.

system.slice uses ~250Mi, init.scope uses ~200Mi, which sums up to
roughly 500Mi, which is needed for system.

Kubelet in /docker slice was using ~100Mi. etcd in /docker slice was
using ~200Mi, so workers has 100Mi reserved for 'kube' and controllers
has 300Mi.

Memory usage for self-hosted components has been measured using the
following command: 'kubectl top pods --sort-by=memory | sort -h -k3 -r'.

Then, the read values were rounded up a bit.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
To avoid duplicating the template logic and to make the configuration
more readable, as having quote (") right before the template logic is
very confusing.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
@vbatts

Copy link
Copy Markdown

This seems like something that would be done inside slices of cgroups on the host, to ensure the system can still operate. I guess it seems arbitrary and inflexible to declare numbers like this. Right?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants