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

Integrate Linux systemd-notify with Quarkus #29107

Closed
Eng-Fouad opened this issue Nov 7, 2022 · 17 comments · Fixed by quarkiverse/quarkiverse-devops#107
Closed

Integrate Linux systemd-notify with Quarkus #29107

Eng-Fouad opened this issue Nov 7, 2022 · 17 comments · Fixed by quarkiverse/quarkiverse-devops#107
Labels
area/core kind/extension-proposal Discuss and Propose new extensions

Comments

@Eng-Fouad
Copy link
Contributor

Eng-Fouad commented Nov 7, 2022

Description

Systemd is a popular service manager on Linux OS that manages starting/stopping services.

When a service unit file is configured to be of type notify:

[Service]
Type=notify

it is expected that the service sends a notification message via sd_notify(3) to notify service manager about start-up completion and other service status changes.

Implementation ideas

quarkus-systemd-notify: This project demonstrates integrating systemd-notify with Quarkus.

It uses JNI to access systemd-notify system call.

Alternatively, use ProcessBuilder to call systemd-notify.

@Eng-Fouad Eng-Fouad added the kind/enhancement New feature or request label Nov 7, 2022
@grossws
Copy link
Contributor

grossws commented Nov 8, 2022

JNI could be quite painful. Have you tried jna or jnr-ffi instead of raw jni? On the one hand it's another dependency (with 700k to 2M footprint) but on the other hand it doesn't bring pain with glibc versioning, libsystemd0 versioning etc

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 8, 2022

@geoand
Copy link
Contributor

geoand commented Nov 8, 2022

Definitely interesting.

I would definitely prefer the solution proposed by @dmlloyd using the ProcessBuilder (that you have linked to in the description)

@Eng-Fouad
Copy link
Contributor Author

Eng-Fouad commented Nov 16, 2022

This is an example of using ProcessBuilder:

import io.quarkus.logging.Log;
import java.util.Locale;

public class SystemdNotifier {
    
    /**
     * Notify the service manager about start-up completion and other service status changes.
     * 
     * @param state contain a newline-separated list of variable assignments
     */
    public static void notify(String state) {
        if (System.getenv("NOTIFY_SOCKET") != null) {
            try {
                var process = new ProcessBuilder("systemd-notify", state).redirectErrorStream(true).start();
                int exitCode = process.waitFor();
                if (exitCode != 0) {
                    Log.warn("systemd-notify returned non-zero exit code: %d".formatted(exitCode));
                    try (var r = new BufferedReader(new InputStreamReader(process.getInputStream()))) {
                        r.lines().forEach(l -> Log.warn("systemd-notify: %s".formatted(l)));
                    }
                }
            } catch (Exception e) {
                Log.error("Failed to call systemd-notify", e);
            }
        }
    }
}

However, it doesn't work unless we add NotifyAccess=all to the service unit file:

[Service]
Type=notify
NotifyAccess=all

The following warning appears in logs if we don't specify NotifyAccess=all:

systemd[1]: quarkus.service: Got notification message from PID 2735, but reception only permitted for main PID 2730


cc @Sanne, @aloubyansky, @gsmet, @radcortez, @stuartwdouglas @geoand @dmlloyd @grossws

@Eng-Fouad
Copy link
Contributor Author

Which class in Quarkus codebase can we include SystemdNotifier.notify()?

io.quarkus.runner.ApplicationImpl's doStart() and doStop()?

Also, I think it is good to have a corresponding config to enable/disable this feature. Something like:

quarkus.linux.systemd.notify.enabled = true

@dmlloyd
Copy link
Member

dmlloyd commented Nov 19, 2022

However, it doesn't work unless we add NotifyAccess=all to the service unit file:

This is a good point and would be a tradeoff that perhaps some would not wish to make.

There is a third option. Since Java 16, Java has had support for UNIX type sockets. The sd_notify function simply sends a string on a datagram socket, so it should be possible to do the same thing from Java. Here's a totally untested example:

package io.quarkus.systemd;

import java.io.Closeable;
import java.io.IOException;
import java.io.Serial;
import java.net.DatagramPacket;
import java.net.DatagramSocket;
import java.net.UnixDomainSocketAddress;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;

/**
 * A simple {@code systemd} notifier.
 */
public final class SystemDNotify implements Closeable {
    private final DatagramSocket sock;

    private SystemDNotify(DatagramSocket sock) {
        this.sock = sock;
    }

    /**
     * Open a connection to the {@code systemd} notifier mechanism.
     *
     * @return the systemd notifier (not {@code null})
     * @throws NoSystemDException if no {@code systemd} information was found
     * @throws IOException if some problem occurred while trying to talk to {@code systemd}
     */
    public static SystemDNotify open() throws NoSystemDException, IOException {
        DatagramSocket sock = new DatagramSocket();
        try {
            // note: it might actually be `$NOTIFY_SOCKET`; if this doesn't work try that
            String env = System.getenv("NOTIFY_SOCKET");
            if (env == null || env.isBlank()) {
                // no systemd; caller can catch this one safely
                throw new NoSystemDException();
            }
            sock.connect(UnixDomainSocketAddress.of(Path.of(env)));
            return new SystemDNotify(sock);
        } catch (Throwable t) {
            // doesn't throw! weird
            sock.close();
            throw t;
        }
    }

    public void ready() throws IOException {
        send("READY=1");
    }

    public void ready(String status) throws IOException {
        send("READY=1\nSTATUS=" + firstLineOf(status));
    }

    public void reloading() throws IOException {
        send("RELOADING=1");
    }

    public void reloading(String status) throws IOException {
        send("RELOADING=1\nSTATUS=" + firstLineOf(status));
    }

    public void stopping() throws IOException {
        send("STOPPING=1");
    }

    public void stopping(String status) throws IOException {
        send("STOPPING=1\nSTATUS=" + firstLineOf(status));
    }

    public void status(String status) throws IOException {
        send("STATUS=" + firstLineOf(status));
    }

    public void watchdog() throws IOException {
        send("WATCHDOG=1");
    }

    @Override
    public void close() {
        sock.close();
    }

    private String firstLineOf(String str) {
        int len = str.length();
        char ch;
        for (int i = 0; i < len; i ++) {
            ch = str.charAt(i);
            if (Character.isISOControl(ch)) {
                return str.substring(0, i);
            }
        }
        // well-behaved string
        return str;
    }

    private void send(String string) throws IOException {
        byte[] bytes = string.getBytes(StandardCharsets.UTF_8);
        sock.send(new DatagramPacket(bytes, 0, bytes.length));
    }

    public static final class NoSystemDException extends IOException {
        @Serial
        private static final long serialVersionUID = 3041841060447675133L;

        /**
         * Constructs a new {@code NoSystemDException} instance.  The message is left blank ({@code null}), and no
         * cause is specified.
         */
        public NoSystemDException() {
        }
    }
}

I think this would suffice, though it's possible that the initial message might need MAINPID=1234 as well (the actual PID can be gotten from ProcessHandle.current().pid() if needed). Also the docs seem a bit ambiguous about the env var holding the socket path.

Which class in Quarkus codebase can we include SystemdNotifier.notify()?

io.quarkus.runner.ApplicationImpl's doStart() and doStop()?

No, IMO it should be a build step in an extension (but I'm not 100% sure which build item would correspond to "ready to process requests"; someone around here should know though...).

Also, I think it is good to have a corresponding config to enable/disable this feature. Something like:

quarkus.linux.systemd.notify.enabled = true

If it's an extension, then the presence of the extension would activate the behavior and its absence would deactivate it. Also, if using the example code above, then the lack of systemd could be captured and logged, allowing execution to continue anyway.

@Eng-Fouad
Copy link
Contributor Author

@gsmet Any plan to implement this feature? Thanks.

@gsmet
Copy link
Member

gsmet commented Nov 25, 2022

I agree with @dmlloyd that it should live in an extension.

The build step should probably consume ServiceStartBuildItem.

@gsmet gsmet added kind/extension-proposal Discuss and Propose new extensions and removed kind/enhancement New feature or request labels Nov 25, 2022
@gsmet
Copy link
Member

gsmet commented Nov 25, 2022

If someone is interested in developing and maintaining it, it would be a nice candidate for the Quarkiverse.

@Eng-Fouad
Copy link
Contributor Author

If someone is interested in developing and maintaining it, it would be a nice candidate for the Quarkiverse.

@gsmet I am interested in developing and maintaining this extension.

@Eng-Fouad
Copy link
Contributor Author

However, it doesn't work unless we add NotifyAccess=all to the service unit file:

This is a good point and would be a tradeoff that perhaps some would not wish to make.

There is a third option. Since Java 16, Java has had support for UNIX type sockets. The sd_notify function simply sends a string on a datagram socket, so it should be possible to do the same thing from Java. Here's a totally untested example:

package io.quarkus.systemd;

import java.io.Closeable;
import java.io.IOException;
import java.io.Serial;
import java.net.DatagramPacket;
import java.net.DatagramSocket;
import java.net.UnixDomainSocketAddress;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;

/**
 * A simple {@code systemd} notifier.
 */
public final class SystemDNotify implements Closeable {
    private final DatagramSocket sock;

    private SystemDNotify(DatagramSocket sock) {
        this.sock = sock;
    }

    /**
     * Open a connection to the {@code systemd} notifier mechanism.
     *
     * @return the systemd notifier (not {@code null})
     * @throws NoSystemDException if no {@code systemd} information was found
     * @throws IOException if some problem occurred while trying to talk to {@code systemd}
     */
    public static SystemDNotify open() throws NoSystemDException, IOException {
        DatagramSocket sock = new DatagramSocket();
        try {
            // note: it might actually be `$NOTIFY_SOCKET`; if this doesn't work try that
            String env = System.getenv("NOTIFY_SOCKET");
            if (env == null || env.isBlank()) {
                // no systemd; caller can catch this one safely
                throw new NoSystemDException();
            }
            sock.connect(UnixDomainSocketAddress.of(Path.of(env)));
            return new SystemDNotify(sock);
        } catch (Throwable t) {
            // doesn't throw! weird
            sock.close();
            throw t;
        }
    }

    public void ready() throws IOException {
        send("READY=1");
    }

    public void ready(String status) throws IOException {
        send("READY=1\nSTATUS=" + firstLineOf(status));
    }

    public void reloading() throws IOException {
        send("RELOADING=1");
    }

    public void reloading(String status) throws IOException {
        send("RELOADING=1\nSTATUS=" + firstLineOf(status));
    }

    public void stopping() throws IOException {
        send("STOPPING=1");
    }

    public void stopping(String status) throws IOException {
        send("STOPPING=1\nSTATUS=" + firstLineOf(status));
    }

    public void status(String status) throws IOException {
        send("STATUS=" + firstLineOf(status));
    }

    public void watchdog() throws IOException {
        send("WATCHDOG=1");
    }

    @Override
    public void close() {
        sock.close();
    }

    private String firstLineOf(String str) {
        int len = str.length();
        char ch;
        for (int i = 0; i < len; i ++) {
            ch = str.charAt(i);
            if (Character.isISOControl(ch)) {
                return str.substring(0, i);
            }
        }
        // well-behaved string
        return str;
    }

    private void send(String string) throws IOException {
        byte[] bytes = string.getBytes(StandardCharsets.UTF_8);
        sock.send(new DatagramPacket(bytes, 0, bytes.length));
    }

    public static final class NoSystemDException extends IOException {
        @Serial
        private static final long serialVersionUID = 3041841060447675133L;

        /**
         * Constructs a new {@code NoSystemDException} instance.  The message is left blank ({@code null}), and no
         * cause is specified.
         */
        public NoSystemDException() {
        }
    }
}

I think this would suffice, though it's possible that the initial message might need MAINPID=1234 as well (the actual PID can be gotten from ProcessHandle.current().pid() if needed). Also the docs seem a bit ambiguous about the env var holding the socket path.

Which class in Quarkus codebase can we include SystemdNotifier.notify()?
io.quarkus.runner.ApplicationImpl's doStart() and doStop()?

No, IMO it should be a build step in an extension (but I'm not 100% sure which build item would correspond to "ready to process requests"; someone around here should know though...).

Also, I think it is good to have a corresponding config to enable/disable this feature. Something like:

quarkus.linux.systemd.notify.enabled = true

If it's an extension, then the presence of the extension would activate the behavior and its absence would deactivate it. Also, if using the example code above, then the lack of systemd could be captured and logged, allowing execution to continue anyway.

@dmlloyd

Unfortunately, Unix-Domain Datagram is not supported in Java.

https://bugs.openjdk.org/browse/JDK-8265226

@dmlloyd
Copy link
Member

dmlloyd commented Nov 28, 2022

Ah, this is unfortunate. In that case I've run out of ideas; personally I think the more permissive service file is better than using JNI but other than that I don't have any other suggestions.

@gsmet
Copy link
Member

gsmet commented Nov 29, 2022

@Eng-Fouad if you still think an extension makes sense, please ask @gastaldi to set up the Quarkiverse repository. Thanks.

@Eng-Fouad
Copy link
Contributor Author

@gsmet I believe we still can implement it using an extension.
@gastaldi Could you please setup a Quarkiverse repository for this extension (quarkus-systemd-notify)? Thanks.

@gastaldi
Copy link
Contributor

gastaldi commented Nov 30, 2022

Unfortunately, Unix-Domain Datagram is not supported in Java.

https://bugs.openjdk.org/browse/JDK-8265226

Just curious: the issue says that it was fixed in JDK 17. Does the proposed solution by @dmlloyd works in JDK 17+?

Update: nevermind, just saw that the fix was a Javadoc improvement.

@Eng-Fouad
Copy link
Contributor Author

Eng-Fouad commented Nov 30, 2022

New issue is created in OpenJDK tracker to implement Unix-Domain datagram:

https://bugs.openjdk.org/browse/JDK-8297837

@Eng-Fouad
Copy link
Contributor Author

quarkus-systemd-notify v0.1.0 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core kind/extension-proposal Discuss and Propose new extensions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants