mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Sekretenko <asekrete...@mesosphere.io>
Subject Re: Review Request 72158: Fixed systemd socket activation on old systemd versions.
Date Mon, 24 Feb 2020 13:49:36 GMT

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



Do we have a MESOS bug for this? Looks like we should...


src/linux/systemd.cpp
Line 400 (original), 400 (patched)
<https://reviews.apache.org/r/72158/#comment307847>

    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.


- Andrei Sekretenko


On Feb. 21, 2020, 12:30 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72158/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2020, 12:30 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Greg Mann, and Qian Zhang.
> 
> 
> 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.cpp 9897473115ac0f9809734c109ba412eefd32e59e 
> 
> 
> Diff: https://reviews.apache.org/r/72158/diff/1/
> 
> 
> Testing
> -------
> 
> internal CI (including CoreOS)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


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