Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docker: add option to force hostname for linstor #172

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

WanzenBug
Copy link
Member

Add a new environment variable that, if set, forces the LINSTOR components to run with a specific hostname.

This may be useful in an environment where we don't have complete control over the names of the created containers, such as kubernetes. While LINSTOR is capable of dealing with changing hostnames, it does lead to weird issues later:

  • LINSTOR names connections based on the peers uname. If that uname is changed, LINSTOR tries to update the resource, but renaming the connection does not always work.
  • For TLS to work, we use the peer name as the name to validate in the certificate. However, if the name is not under our control, it is hard to generate a certificate that is valid for the names.
  • Some external components such as monitoring and so on also rely on the peer name to generate useful output.

So in order to have a stable hostname for LINSTOR and DRBD, we add an option to start LINSTOR in a new UTS namespace with the forced hostname set.

We use a new UTS namespace because if we would just force the hostname to be set in all cases, we potentially override the "root" namespace if the container is started in the host network. We do not want to make unneeded changes to the host, so using a new UTS namespace is the simplest solution.

Using unshare with a new UTS namespace also means that the created namespace is automatically removed once the LINSTOR processes exits.

We also update the await-election tool: this now uses execve() to run the command when not running the leader election. This means that in case a satellite is started, the entry.sh script will become PID 1 in the container. This in turn makes the unshare()-d process PID 1, which is nice, as now docker|kubectl exec'ing into the container will also put us into the same namespace, so all drbdadm commands will work as expected.

Copy link
Member

@rck rck left a comment

Choose a reason for hiding this comment

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

unshare needs certain privs, how does that match running the LINSTOR controller unprivileged? do we require root? e.g., the "official" LINBIT containers use a less privileged user to run the container. I know this is a separate entry.sh, just making sure this got thought of.

@WanzenBug
Copy link
Member Author

unshare needs certain privs, how does that match running the LINSTOR controller unprivileged?

It doesn't really. I mostly added the option for the controller for symmetry, but I could also remove it: it only really makes sense for Satellites, which already need to run as privileged containers, so unshare is fine.

@rck
Copy link
Member

rck commented Feb 15, 2024

It doesn't really. I mostly added the option for the controller for symmetry, but I could also remove it: it only really makes sense for Satellites, which already need to run as privileged containers, so unshare is fine.

totally agree, only makes sense for the satellite and there it is fine. Then I suggest you remove it from the controller call, just we don't raise some "expectations"

Add a new environment variable that, if set, forces the LINSTOR components
to run with a specific hostname.

This may be useful in an environment where we don't have complete control over
the names of the created containers, such as kubernetes. While LINSTOR is
capable of dealing with changing hostnames, it does lead to weird issues later:

* LINSTOR names connections based on the peers uname. If that uname is changed,
  LINSTOR tries to update the resource, but renaming the connection does not
  always work.
* For TLS to work, we use the peer name as the name to validate in the
  certificate. However, if the name is not under our control, it is hard to
  generate a certificate that is valid for the names.
* Some external components such as monitoring and so on also rely on the
  peer name to generate useful output.

So in order to have a stable hostname for LINSTOR and DRBD, we add an option to
start LINSTOR in a new UTS namespace with the forced hostname set.

We use a new UTS namespace because if we would just force the hostname to be
set in all cases, we potentially override the "root" namespace if the
container is started in the host network. We do not want to make unneeded
changes to the host, so using a new UTS namespace is the simplest solution.

Using unshare with a new UTS namespace also means that the created namespace
is automatically removed once the LINSTOR processes exits.

We also update the await-election tool: this now uses execve() to run the
command when not running the leader election. This means that in case a
satellite is started, the entry.sh script will become PID 1 in the container.
This in turn makes the unshare()-d process PID 1, which is nice, as now
docker|kubectl exec'ing into the container will also put us into the same
namespace, so all drbdadm commands will work as expected.

Signed-off-by: Moritz Wanzenböck <[email protected]>
Copy link
Member

@rck rck left a comment

Choose a reason for hiding this comment

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

LGTM

@JoelColledge JoelColledge merged commit 6bc1ca9 into master Feb 16, 2024
2 checks passed
@JoelColledge JoelColledge deleted the force-node-name branch February 16, 2024 07:55
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.

3 participants