mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Budnik <abud...@mesosphere.com>
Subject Re: Review Request 72158: Fixed systemd socket activation on old systemd versions.
Date Mon, 24 Feb 2020 16:41:31 GMT


> On Фев. 24, 2020, 1:49 п.п., Andrei Sekretenko wrote:
> > src/linux/systemd.cpp
> > Line 400 (original), 400 (patched)
> > <https://reviews.apache.org/r/72158/diff/1/?file=2211995#file2211995line400>
> >
> >     If this function starts to behave this way to work around older systemds, at
the bare minimum it should be renamed. Otherwise the code in `slave/main.cpp` becomes totally
misleading.
> >     
> >     Also, this workaround (and, honestly, the whole situation with pre-277 systemds)
looks rather fragile. 
> >     This makes me wonder if there is a simple reliable way to detect systemd version
(I'm looking at some other code in this file that seems to do this) and suppress this workaround
for newer versions (and log warning for older versions!). 
> >     If there is no simple way, this should probably be left as TODO...
> >     
> >     
> >     I see at least two options how to avoid misleading readers of `slave/main.cpp`.
> >     
> >     1) change signature to `listenFdsWithNames(const std::set<string>&
names)` so that the code in main.cpp looks like 
> >     ```
> >     // NOTE: pre-277 systemd versions do not support FileDescriptorName, thus we
also need to listen on descriptiors with name "unknown"
> >     Try<std::vector<int>> socketFds =
> >         systemd::socket_activation::listenFdsWithNames({name , "unknown"})
> >     ```
> >     
> >     2) Rename this function (with added `== "unknown"`) into `listenFds(const string&
name)` and, optionally, make `listenFds()` with no arguments static.
> >     This way, systemd version check, if we decide to add it, can be introduced into
this fucntion, instead of going somewhere outside.

Updated the function (choose option 1 from your suggestion).


- Andrei


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72158/#review219636
-----------------------------------------------------------


On Фев. 24, 2020, 4:39 п.п., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72158/
> -----------------------------------------------------------
> 
> (Updated Фев. 24, 2020, 4:39 п.п.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Greg Mann, and Qian Zhang.
> 
> 
> Bugs: MESOS-10098
>     https://issues.apache.org/jira/browse/MESOS-10098
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch fixes the bug when `listenFdsWithName` function returns
> an empty set of file descriptors on pre-227 systemd versions when
> `domain_socket_location` value is not equals to the "systemd:unknown".
> This happens when a user expects a newer version of systemd and
> specifies a "systemd/<value taken from FileDescriptorName>", but
> the actual systemd version does not support `FileDescriptorName`.
> In this case, `LISTEN_FDNAMES` env variable is not specified,
> so all socket FDs, which are specified by the `LISTEN_FDS`,
> must be used to locate the path to the domain socket.
> 
> 
> Diffs
> -----
> 
>   src/linux/systemd.hpp ba960009d4e47f253fccfcd0edeedf4e6cdc0dca 
>   src/linux/systemd.cpp 9897473115ac0f9809734c109ba412eefd32e59e 
>   src/slave/main.cpp c1e65519f4c684dcd608cbd1a67d7f5945161af4 
> 
> 
> Diff: https://reviews.apache.org/r/72158/diff/2/
> 
> 
> Testing
> -------
> 
> internal CI (including CoreOS)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message