By default, `cryptsetup luksFormat` uses Argon2i as Password-Based Key
Derivation Function (PBKDF), which not only has a CPU cost, but also a memory
cost (to make brute-force attacks harder).
The memory cost is based on the available system memory by default, which in
the context of Ceph CSI can be a problem for two reasons:
1. Pods can have a memory limit (much lower that the memory available on the
node, usually) which isn't taken into account by `cryptsetup`, so it can get
OOM-killed when formating a new volume;
2. The amount of memory that was used during `cryptsetup luksFormat` will then
be needed for `cryptsetup luksOpen`, so if the volume was formated on a node
with a lot of memory, but then needs to be opened on a different node with
less memory, `cryptsetup` will get OOM-killed.
This commit sets the PBKDF memory limit to a fixed value to ensure consistent
memory usage regardless of the specifications of the nodes where the volume
happens to be formatted in the first place.
The limit is set to a relatively low value (32 MiB) so that the `csi-rbdplugin`
container in the `nodeplugin` pod doesn't require an extravagantly high memory
limit in order to format/open volumes (particularly with operations happening
in parallel), while at the same time not being so low as to render it
completely pointless.
Signed-off-by: Benoît Knecht <bknecht@protonmail.ch>
(cherry picked from commit 1852e977f8)
Currently the Ceph-CSI community is on the 'free' Slack instance at
https://cephcsi.slack.com. The Ceph project uses a Slack instance that
we can use for Ceph-CSI as well. In order to integrate more with other
Ceph projects, we should ideally be active on the same Slack instance.
For now, we have `#ceph-csi` as only channel on the
https://ceph-storage-slack.com, we can add more channels if needed.
See-also: https://ceph.io/en/community/connect/
Signed-off-by: Niels de Vos <ndevos@ibm.com>
(cherry picked from commit ea3cd2b5e4)
After the `ok-to-test` label was added, the commenter will remove the
label again. There is no need for Mergify to re-add the label while CI
jobs are still running.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
There is no need to run the `test-retest-action` GitHub Workflow if
there are no changes under the `actions/retest` directory.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Currently commitlint is only skipped for PR at the time dependabot
creates them. Once Mergify rebases them, commitlint is started anyway.
This causes failed CI runs, which then need to be ignored. It is cleaner
to not run commitlint on any PR that dependabot owns.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Sometimes Mergify removed the `ok-to-test` label before the Pull Request
Commentor action have been run. With the updated commentor action, the
`ok-to-test` label is removed after leaving comments. There is no need
for Mergify to remove the label anymore.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Once the comments have been added, the `ok-to-test` label can be
removed. This makes it possible to simplify the Mergify configuration.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
This commit makes use of crush location labels from node
labels to supply `crush_location` and `read_from_replica=localize`
options during rbd map cmd. Using these options, ceph
will be able to redirect reads to the closest OSD,
improving performance.
Signed-off-by: Rakshith R <rar@redhat.com>
The check for a rebase always hits, but the actions for that inspect the
event for a real rebase. Removing the `ok-to-test` label is not suitable
in the Mergify check, as the label action does not inspect the event.
This caused the `ok-to-test` label to be removed on every Mergify
validation of the PR.
Fixes: ba68ce6 (ci: drop `ok-to-test` label when a PR is rebased)
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Without this patch the READMEs for the Helm Charts do not provide any
documentation on how to upgrade to a newer version. There is at least
one known issue when updating to a newer versions that is unavoidable as
of writing. There is a workaround for the issue which should be
documented in the upgrade section.
This is a problem because currently the only way to find this workaround
is to go through closed GitHub issues. These might not be around at the
time someone needs this information. Furthermore the issue should be
communicated to the operator before it occurs.
This patch adds basic documentation for updating the Helm repository,
and upgrading the installed release of the Helm Chart. How values can be
set is not part of the documentation. If an operator used custom values,
e.g. for the secret, they probably already know how to deal with setting
values. However, the docs still remind the reader to take values into
account.
Reusing the installed values (`--reuse-values`) has lead to problems in
past, which is why it is explicitly discouraged. An example for this
would be the value `logLevel` which was changed to `sidecarLogLevel`.
Reusing values lead to `.Values.sidecarLogLevel` being empty and the
`csi-provisioner` not being started due to invalid value `-v=""`.
Comparing new values with set values is encouraged.
The workaround for issue #3397 from GitHub is being addressed in the
section Know Issues Upgrading.
Signed-off-by: Christian Kugler <syphdias+git@gmail.com>
