[DPE-4859] Apply sysctl from the charm #369

phvalguima · 2024-07-16T15:29:52Z

Production deployments will be using VMs instead of containers. In this case, we do not need to delegate the sysctl setup to the user but could rather do it on the charm.

Containers, on the other hand, cannot set all the types of sysctl:

root@juju-f3f50a-0:~# sysctl -w vm.max_map_count=262144
sysctl: permission denied on key "vm.max_map_count", ignoring
root@juju-f3f50a-0:~# sysctl -w vm.swappiness=0
sysctl: permission denied on key "vm.swappiness", ignoring
root@juju-f3f50a-0:~# sysctl -w net.ipv4.tcp_retries2=5
net.ipv4.tcp_retries2 = 5
root@juju-f3f50a-0:~# sysctl -w fs.file-max=1048576
sysctl: permission denied on key "fs.file-max", ignoring

This PR adds the sysctl lib and uses it to set all the parameters if we are running directly on the host, or only net.ipv4.tcp_retries2 otherwise.

Closes #359

README.md

tox.ini

delgod · 2024-07-17T10:14:46Z

lib/charms/opensearch/v0/opensearch_distro.py

+            "vm.swappiness": "0",
+            "net.ipv4.tcp_retries2": "5",
+        }
+        if OpenSearchDistribution.running_as_lxc():


I think it is better to check sysctl anyway, it doesn't matter on which platform.

So, I cannot apply all the options above for LXC; but I can if we have a VM or host.

I think it is better to check the sysctl on all platforms after setting. It doesn't matter what platform or exit codes are received.

@delgod, so to clarify, it is better to keep the original behavior, where we check the settings and then, block if the user did not apply it?

@phvalguima I think the following flow can work in our case:

check the current values of each sysctl setting (regardless of the platform)

make the distinction between:

the settings that need to be equal to a value (tcp_retries2, or swappiness)

the settings that need to be greater than a value (max_map_count >= 262144 , file-max)

we should only set these if that's the case, otherwise we are decreasing the quality of the settings already existing in the environment

This should result in a set of invalid settings.

attempt to set these settings (all the invalid set or one at a time), and re-check the new values, so you can accurately add each in the missing_requirements if any:

I prefer the missing_requirements we had, as the message is more tailored towards the values that need to be set, not just that the setting could not be set

Mehdi-Bendriss

Thanks Pedro - I have a few comments

lib/charms/opensearch/v0/opensearch_distro.py

Mehdi-Bendriss · 2024-07-17T13:53:46Z

README.md

-Configure the system settings required by [OpenSearch](https://opensearch.org/docs/latest/install-and-configure/install-opensearch/index/),
-we'll do that by creating and setting a [`cloudinit-userdata.yaml` file](https://juju.is/docs/olm/juju-model-config) on the model. 
-As well as setting some kernel settings on the host machine.
-```
-cat <<EOF > cloudinit-userdata.yaml
-cloudinit-userdata: |
-  postruncmd:
-    - [ 'echo', 'vm.max_map_count=262144', '>>', '/etc/sysctl.conf' ]
-    - [ 'echo', 'vm.swappiness=0', '>>', '/etc/sysctl.conf' ]
-    - [ 'echo', 'net.ipv4.tcp_retries2=5', '>>', '/etc/sysctl.conf' ]
-    - [ 'echo', 'fs.file-max=1048576', '>>', '/etc/sysctl.conf' ]
-    - [ 'sysctl', '-p' ]
-EOF


Why did we remove this? This will still set userdata on EC2 instances - where swappiness etc can be set directly on the VM. (on LXD it will only set tcp_retries)

Why would we need it then?

Running within LXC

We cannot set vm.max_map_count, vm.swappiness and fs.file-max as they must be set on the underlying host. So, the cloud-init is useless there.

Running within VMs

We will set all these values via charm.

So, in which situation I need the cloud-init back?

I am keeping the commands to set the configs in sysctl for the LXC case.

on LXD:

tcp_retries can be set using userdata through the model.
The rest will not be set, but will not fail.

on EC2 / VM / Baremetal:

all sysctl can be applied using userdata through the model

users may choose to set better values than the ones we suggest, e.g higher value of max_map_count than our minimum set by the charm.

On the code:

we ensure that the values are correct, (they could have been changed manually in machines), if they're not - we set them

sysctl calls can fail for whatever reason, having them on the userdata already is 1 additional layer of safety.

Mehdi-Bendriss · 2024-07-17T13:54:01Z

README.md

 fs.file-max=1048576
 EOT

 sudo sysctl -p
-
-juju model-config --file=./cloudinit-userdata.yaml


Same comment as above

lib/charms/opensearch/v0/opensearch_distro.py

Mehdi-Bendriss · 2024-07-17T14:52:43Z

lib/charms/opensearch/v0/opensearch_distro.py

+            sysctl.Config(name).configure(config)
+        except sysctl.CommandError as e:
+            logger.error(f"Failed to apply sysctl config: {e}")
+            missing_requirements.append(str(e))


If we go with a bulk set, This will result in the BlockedStatus message, as follows:

Unable to set params: vm.swappiness, vm.max_map_count.

In which case we don't need missing_requirements

Mehdi-Bendriss · 2024-07-18T08:13:04Z

lib/charms/opensearch/v0/opensearch_distro.py

+            "vm.swappiness": "0",
+            "net.ipv4.tcp_retries2": "5",
+        }
+        if OpenSearchDistribution.running_as_lxc():


@phvalguima I think the following flow can work in our case:

check the current values of each sysctl setting (regardless of the platform)

make the distinction between:

the settings that need to be equal to a value (tcp_retries2, or swappiness)

the settings that need to be greater than a value (max_map_count >= 262144 , file-max)

we should only set these if that's the case, otherwise we are decreasing the quality of the settings already existing in the environment

This should result in a set of invalid settings.

attempt to set these settings (all the invalid set or one at a time), and re-check the new values, so you can accurately add each in the missing_requirements if any:

I prefer the missing_requirements we had, as the message is more tailored towards the values that need to be set, not just that the setting could not be set

lib/charms/opensearch/v0/opensearch_distro.py

phvalguima added 3 commits July 16, 2024 17:22

Add support for sysctl commands

c90a574

Update README

ac9c35f

Fix README text

45aa06f

phvalguima requested review from Mehdi-Bendriss and delgod July 16, 2024 15:29

phvalguima added 5 commits July 16, 2024 17:35

Move to sysctl -n to list param values

d94c470

Add missing self.unit_name

8b10601

Update the sysctl on tox

45131ef

Add test to check new sysctl lib

5346109

Simplify sysctl

4a2bab3

delgod requested changes Jul 17, 2024

View reviewed changes

Update opensearch_distro.py

277e090

Mehdi-Bendriss reviewed Jul 18, 2024

View reviewed changes

phvalguima added 2 commits July 26, 2024 16:12

Update sysctl following feedback

7902d20

Remove value left over by cherry-pick

2419464

Mehdi-Bendriss closed this Aug 13, 2024

Mehdi-Bendriss deleted the DPE-4859-apply-sysctl branch August 13, 2024 22:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DPE-4859] Apply sysctl from the charm #369

[DPE-4859] Apply sysctl from the charm #369

phvalguima commented Jul 16, 2024

delgod Jul 17, 2024

phvalguima Jul 17, 2024

delgod Jul 17, 2024

phvalguima Jul 17, 2024

Mehdi-Bendriss Jul 18, 2024

Mehdi-Bendriss left a comment

Mehdi-Bendriss Jul 17, 2024 •

edited

Loading

phvalguima Jul 18, 2024

phvalguima Jul 18, 2024

Mehdi-Bendriss Jul 18, 2024

Mehdi-Bendriss Jul 17, 2024

Mehdi-Bendriss Jul 17, 2024

Mehdi-Bendriss Jul 18, 2024

[DPE-4859] Apply sysctl from the charm #369

[DPE-4859] Apply sysctl from the charm #369

Conversation

phvalguima commented Jul 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mehdi-Bendriss left a comment

Choose a reason for hiding this comment

Mehdi-Bendriss Jul 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Running within LXC

Running within VMs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mehdi-Bendriss Jul 17, 2024 •

edited

Loading